Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118538
Message-ID: <bb65a1c7-5e53-0430-7b30-e1aceacc623a@gmx.de>
Date: Mon, 29 Aug 2022 12:48:04 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.0
Content-Language: de-DE
To: Sara Golemon <pollita@php.net>, 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>
References: <CAMs36Hdg7Ydx+B6b1fSGosds3mC2KppN7xbfOi3zwPW8cRncsQ@mail.gmail.com>
 <355260c3-880f-a497-698f-c12175192485@gmx.de>
 <CAESVnVpeOd==hz619F92embes4iE+2CaSVC17i9ozJcwDhk3Aw@mail.gmail.com>
In-Reply-To: <CAESVnVpeOd==hz619F92embes4iE+2CaSVC17i9ozJcwDhk3Aw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: ksort breaking change
From: cmbecker69@gmx.de ("Christoph M. Becker")

On 26.08.2022 at 18:28, Sara Golemon wrote:

> 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 algorithm=
s.
> 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 i=
s
> sites out there where the inconsistency shows through to end users (thou=
gh
> 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 prim=
ary
> 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 y=
ear
> of bug-fix releases to go, and I might be swayed to keep the fix around
> there.

I have reverted the commit on PHP-8.0 just now.  Please don't forget to
revert on PHP-8.0.23.

>>> 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.

Yeah, would be good to hear whether the commit should be reverted on
PHP-8.1 as well.

=2D-
Christoph M. Becker