Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:121748
MIME-Version: 1.0
References: <CAH5C8xX5Wzbm1eerFJJ5hCZdUU8rci8NhBwkSwprRKAG531G7Q@mail.gmail.com>
 <CAEKnhAFDYiPoKwjj2J6akEaZ6WgHkaYia9su_fNQGSKidCt=aw@mail.gmail.com> <CAGKNXQHnvNKKXBMNTKq5KbuskJ5OtaSejEU6VRDgn5ngKtSprA@mail.gmail.com>
In-Reply-To: <CAGKNXQHnvNKKXBMNTKq5KbuskJ5OtaSejEU6VRDgn5ngKtSprA@mail.gmail.com>
Date: Tue, 21 Nov 2023 22:42:47 +0100
Message-ID: <CAH5C8xV1o1EmOyEmPHt6+ZVjUsB37dMDyBH+YQAzrKtBDvuPyg@mail.gmail.com>
To: Philip Hofstetter <phofstetter@sensational.ch>
Cc: Jakub Zelenka <bukka@php.net>, PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000798405060ab07c92"
Subject: Re: [PHP-DEV] [RFC] [Discussion] Resource to object conversion
From: kocsismate90@gmail.com (=?UTF-8?B?TcOhdMOpIEtvY3Npcw==?=)

--000000000000798405060ab07c92
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Derick, Jakub, Phiip


> Did you do an analyses as to how much either of these changes could break
> anything?


I updated the RFC with some impact analysis. The numbers support my
hypothesis that the conversion
of auxiliary stream resources would cause hardly any BC break - at least in
case of the top 2000 PHP
packages. That's why I believe we can migrate them separately from the
primary ones, which are used
much more often.

 I wonder whether it was previously discussed to have all these converted
> objects implement a `Resource` interface and have `is_resource()` check f=
or
> that.
>

Yes, the RFC links two threads about this topic:
https://externals.io/message/116127 and
https://externals.io/message/104361#104369. Even though
there's a clear interest in changing how is_resource() works, doing so
would bring us to a minefield...

Please could you add a separate vote for primary streams if the resource to
> object conversion should be done at all (requiring 2/3 votes to be
> accepted).

I will personally vote against this if there is no is_resource change as I
> think it's just too big BC break even for 9.0 - it will likely require
> massive update of many code bases.


Yes, I added the option.


> Both the function and maybe also the interface could then also be marked
> as deprecated, but it would allow for a much more painless transition.
>

While I would love to see resources go altogether, deprecating
is_resource() is way too early. Even if php-src itself manages to sunset
all the built-in resources,
there will still be lots of third party extensions which will still rely on
them.

Personally, I=E2=80=99m now relying on psalm to detect such issues, so if I=
 had a
> vote I would selfishly vote yes anyways, but still: for those without
> static analysis,
>
this would IMHO make things much easier.
>

Yeah, using static analysis should be mandatory for mission critical
systems, since these tools can easily detect issues like wrong
is_resource() checks.
However, I acknowledge that doing so is not always possible due to some
constraints (i.e. budget limits). Fortunately, the possible issues
caused by resource
to object conversions are usually not too difficult to find out. For
example, in case of the Process resource, one has to search for all
proc_open() invocations, and
check whether the return values are correctly checked. According to my
experience, the is_resource() checks are usually very close to the creation
of resources, so
they can be fixed easily when needed.

Regards,
M=C3=A1t=C3=A9

--000000000000798405060ab07c92--