Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118515
MIME-Version: 1.0
References: <CAMs36Hdg7Ydx+B6b1fSGosds3mC2KppN7xbfOi3zwPW8cRncsQ@mail.gmail.com>
 <355260c3-880f-a497-698f-c12175192485@gmx.de>
In-Reply-To: <355260c3-880f-a497-698f-c12175192485@gmx.de>
Date: Fri, 26 Aug 2022 11:28:11 -0500
Message-ID: <CAESVnVpeOd==hz619F92embes4iE+2CaSVC17i9ozJcwDhk3Aw@mail.gmail.com>
To: "Christoph M. Becker" <cmbecker69@gmx.de>, Ben Ramsey <ramsey@php.net>, 
	Patrick ALLAERT <patrick.allaert@gmail.com>
Cc: Go Kudo <g-kudo@colopl.co.jp>, PHP internals <internals@lists.php.net>, 
	Gabriel Caruso <carusogabriel34@gmail.com>
Content-Type: multipart/alternative; boundary="0000000000000cea9605e727672b"
Subject: Re: ksort breaking change
From: pollita@php.net (Sara Golemon)

--0000000000000cea9605e727672b
Content-Type: text/plain; charset="UTF-8"

On Fri, Aug 26, 2022 at 7:19 AM Christoph M. Becker <cmbecker69@gmx.de>
wrote:
>
> On 26.08.2022 at 05:15, Go Kudo wrote:
>
> > In the actively supported version of PHP, `ksort()` has been modified to
> > include BC Break.
> >
> > https://github.com/php/php-src/issues/9296
> >
> > This may seem like an appropriate bug fix, but it is a clear BC Break. I
> > think this change should only be introduced in PHP 8.2 and later.
>
> In this case, the functions didn't behave as documented, namely to
> conform to the general conversion rules, which had a relevant change in
> PHP 8.0.  Apparently, this case has been overlooked when the change had
> been implemented, and only been noticed recently (what still surprises
> me).  Anyway, fixing the issue now is not really introducing a BC break,
> since code relying on the previous behavior did not conform to the
> documentation.
>

What I can see is two noble, but conflicting ideals:
1/ sort() and ksort() should be consistent about their sorting algorithms.
I think we can all agree about that in the ideal case, at least.
2/ Behavior within a minor release should be self-consistent and
predictable.

Knowing the discrepancy exists in current releases of 8.0.x and 8.1.x,
we're currently in a position where ideal #2 is satisfied, but ideal #1 is
failing in a small, and subtle way.  The potential consequence of this is
sites out there where the inconsistency shows through to end users (though
I wouldn't actually expect any breakages as a result, just clowniness).

With the recently applied patch, we would resolve ideal #1, but we would do
so at the cost of ideal #2.  It's doubtful this will actually break any
code either, but potentially user-facing affects could be visible,
especially in a mixed-version environment (such as during a rolling
upgrade).

Given the above, my initial inclination is to err on the side of
conservatism for 8.0.x at the least (we're nearly at the end of our primary
bug-fix cycle anyway) by reverting the fix on our branch.
For 8.1, I think we have a more difficult decision to make with over a year
of bug-fix releases to go, and I might be swayed to keep the fix around
there.

> > Fortunately, there is not yet a release in each version that
incorporates
> > this change. I think it is possible to revert now.
>
> Well, the fix is part of the currents RCs; that doesn't make it
> impossible to revert, but RMs should have a say in that.  Thus I'm
> adding Sara and Gabriel as recepients.
>

Don't forget Ben and Patrick as well, this impacts 8.1 equally.

-Sara

--0000000000000cea9605e727672b--