Re: ksort breaking change
| From: | Ben Ramsey | Date: | Tue, 30 Aug 2022 14:25:07 +0000 |
| Subject: | Re: ksort breaking change | ||
| References: | 1 2 3 4 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-118541@lists.php.net to get a copy of this message | ||
> On Aug 29, 2022, at 05:48, Christoph M. Becker <cmbecker69@gmx.de> wrote:
>
> 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,
Attachment: [application/pgp-signature] Message signed with OpenPGP signature.asc
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.
>
> 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.
I decided to revert this in PHP-8.1 and PHP-8.1.10 branches, as well.
Cheers,
Ben
Attachment: [application/pgp-signature] Message signed with OpenPGP signature.asc