Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116148
MIME-Version: 1.0
References: <CAJp_myX3J8gUb3885KAdEz+xokpZ_mz1Gs_DrN2Tp+JYfewcEw@mail.gmail.com>
In-Reply-To: <CAJp_myX3J8gUb3885KAdEz+xokpZ_mz1Gs_DrN2Tp+JYfewcEw@mail.gmail.com>
Date: Thu, 23 Sep 2021 18:52:44 +0200
Message-ID: <CAF+90c_JR1mUrgHFkfYYZi2TuSmkaF3hmvT03rfW7kGnhkZf6Q@mail.gmail.com>
To: "Matthew Weier O'Phinney" <mweierophinney@gmail.com>
Cc: PHP Developers Mailing List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000acb4ff05ccac7642"
Subject: Re: [PHP-DEV] BC breaking changes in PHP 8.1
From: nikita.ppv@gmail.com (Nikita Popov)

--000000000000acb4ff05ccac7642
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Sep 22, 2021 at 3:30 PM Matthew Weier O'Phinney <
mweierophinney@gmail.com> wrote:

> Yesterday, I opened an issue regarding a change in the pgsql extension (
> https://bugs.php.net/bug.php?id=3D81464).
>
> PHP 8.0 introduced the concept of "resource objects". Where previously we
> would have resources, and use `get_resource_type()` when we needed to
> differentiate various resources, resource objects give us immutable objec=
ts
> instead that allow us to type hint. Personally, I think this is wonderful=
!
>
> The rollout for 8.0 was incomplete, however, and only touched on somethin=
g
> like 4-6 different resource types. Still, a good start.
>
> With PHP 8.1, we're seeing the addition of more of these, and it was
> encountering one of those changes that prompted the bug I previously link=
ed
> to.
>
> Here's the issue: while overall, I like the move to resource objects,
> introducing them in a MINOR release is hugely problematic.
>
> Previously, you would do constructs such as the following:
>
>     if (! is_resource($resource) || get_resource_type($resource) !=3D=3D
> $someSpecificType) {
>         // skip a test or raise an exception
>     }
>
> Resource objects, however:
>
> - Return `false` for `is_resource()` checks.
> - Raise a warning for `get_resource_type()` checks, and/or report the
> resource object class name =E2=80=94 which differs from the previous reso=
urce names
> in all cases.
>
> This means conditionals like the above BREAK. As a concrete example, I di=
d
> PHP 8.1 updates for laminas-db last week, and assumed our postgres
> integration tests were running when we finally had all tests passing
> successfully. However, what was really happening was that our test suite
> was testing with `is_resource()` and skipping tests if resources were not
> present. We shipped with broken pgsql support as a result, and it wasn't
> until test suites in other components started failing that we were able t=
o
> identify the issue.
>
> Further, the "fix" so that the code would work on both 8.1 AND versions
> prior to 8.1 meant complicating the conditional, adding a `! $resource
> instanceof \PgSql\Connection` into the mix. The code gets unwieldy very
> quickly, and having to do this to support a new minor version was
> irritating.
>
> When I opened the aforementioned bug report, it was immediately closed as
> "not an issue" with the explanation that it was "documented in UPGRADING"=
.
>
> This is not an acceptable explanation.
>
> - There was no RFC related to 8.1 indicating these changes were happening=
.
> (In fact, there was no RFC for resource objects in the first place =E2=80=
=94 which
> is concerning considering the BC implications!)
> - In semantic versioning, existing APIs MUST NOT change in a new minor
> version, only in new major versions.
>
> Reading the UPGRADING guide, there's a HUGE section of backwards
> incompatible changes for 8.1 =E2=80=94 THIRTY-FOUR of them. Nested in the=
se are
> notes of around a half-dozen extensions that once produced resources now
> producing resource objects.
>
> The pace of change in PHP is already breathtaking when you consider large
> projects (both OSS and in userland); keeping up with new features is in a=
nd
> of itself quite a challenge. Introducing BC breaks in minor versions make=
s
> things harder for everyone, as now you have to figure out not only if
> there's new features you want to adopt, but whether or not there are
> changes that will actively break your existing code. I strongly feel that
> anything in the backwards incompatible section of the UPGRADING guide
> should be deferred to 9.0, when people actually expect things to change.


I believe the changes in PHP 8.1 with the highest migration burden for
open-source libraries are the additional of tentative return types (aka
"put #[ReturnTypeWillChange] everywhere") and deprecation of null arguments
to internal functions, followed by the float to int precision-loss
deprecation and, depending on project, the Serializable deprecation.

What all of these have in common, is that they are all semver compliant
changes, because they "only" introduce deprecations. Deprecations are
explicitly not considered backwards-compatibility breaks.

Now, there are two problems with this picture: The first one is that
deprecations often get promoted to exceptions by generic error handlers. I
believe that this continues to be the default behavior of PHPUnit for
example. This means that in practice, deprecations do break code, even
though they are intended not to.

The second one is that this does not really hold up for open-source
libraries. At the application layer, you can suppress all deprecations, and
call it a day. At the library layer, the usual perception is that if your
library throws deprecation warnings, it's not compatible with the given
version of PHP. Taken in conjunction with deprecation to exception
promotion (in test suites if nothing else) there is some truth to that
perception.

While deprecations are intended as a backwards-compatible mechanism to warn
you about upcoming changes without requiring immediate action, in reality
open-source libraries have to treat deprecations as immediate breakage.

One open-source project with a slightly different attitude towards
deprecations is Symfony, where deprecation notices during test runs are
aggregated and considered quite ordinary. Of course Symfony does address
PHP-related deprecations before release, but this happens gradually. We
were able to run the Symfony test suite just fine during most of the PHP
8.1 development phase, much unlike most other projects, which would crash
and burn on encountering the first "tentative return types" deprecation.

The reason why I'm going off on this tangent: I believe that even if PHP
followed semver to the letter, I don't think the migration burden for
open-source libraries would change to any appreciable degree. If we dropped
all non-deprecation changes from PHP 8.1, the upgrade would be a bit
easier, but not by much. This is not where the main cost is.

I'm not sure what we can do about that though. Sure, we could stop with the
(runtime) deprecations, only document the change and then directly
implement it at the next major version. That would be much simpler for us
(generating runtime deprecations is actually a major implementation pain,
as well as a big performance concern) and would make minor version upgrades
for libraries much simpler. However, it also removes the ability to address
problems before they turn into fatal errors, e.g. by recording any stray
deprecation warnings in production. There was certainly a big outcry over
the handful of backwards-incompatible changes in PHP 8.0 that did not
previously trigger deprecation warnings.

Regards,
Nikita

--000000000000acb4ff05ccac7642--