Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.x series does not support PHP 8.3 #175

Closed
dereuromark opened this issue Oct 26, 2023 · 20 comments
Closed

2.x series does not support PHP 8.3 #175

dereuromark opened this issue Oct 26, 2023 · 20 comments
Labels
Question Further information is requested

Comments

@dereuromark
Copy link

dereuromark commented Oct 26, 2023

Summary

PHP 8.3 support is needed for 2.x series.

See
cakephp/cakephp#17387

Current behavior

Not installable, probably for no good reason as 8.3 should be compatible out of the box

How to reproduce

See above link

Expected behavior

~8.3 included in the constraint for PHP so folks stuck on that 2.x series of this library can still do 8.3 checking and development of their library code.

Many libraries and frameworks will have to support their major versions for a few years longer, so it is important that they can ship with secure/safe PHP versions in the present and future.
As per semver they cannot just modify their public API and as such are locked within 2.x series for now (no types added etc).

Thank you

@dereuromark dereuromark added the Bug Something isn't working label Oct 26, 2023
@Ocramius
Copy link
Member

I don't think 2.x should be revived: is the adoption (either bump or dependency widening) to 3.x not viable?

Can we perhaps help there instead, if there are any blockers?

@Ocramius Ocramius added Question Further information is requested and removed Bug Something isn't working labels Oct 26, 2023
@froschdesign
Copy link
Member

@dereuromark

It shows that laminas/laminas-diactoros in all compatible versions to cake 4.4 and 4.5 do not offer 8.3 support due to their bad way of constraining.

What do you mean with "bad way of constraining"?

@Ocramius
Copy link
Member

From your upstream issue:


    - cakephp/cakephp[4.4.17, ..., 4.5.0] require laminas/laminas-diactoros ^2.2.2 -> satisfiable by laminas/laminas-diactoros[2.2.2, ..., 2.25.2].

This is what I'd suggest widening to "laminas/laminas-diactoros": "^2 || ^3" first

@Ocramius
Copy link
Member

@dereuromark

It shows that laminas/laminas-diactoros in all compatible versions to cake 4.4 and 4.5 do not offer 8.3 support due to their bad way of constraining.

What do you mean with "bad way of constraining"?

See #117

It is a past (and present) disagreement from the issue author with our approach to dependency management, which we explained in depth to be well founded, well intended, and I assume well understood now.

@Ocramius
Copy link
Member

BTW, we use --ignore-platform-reqs in some CI pipelines ourselves, where upgrades aren't feasible yet :-)

@dereuromark
Copy link
Author

I don't think 2.x should be revived: is the adoption (either bump or dependency widening) to 3.x not viable?

cakephp/cakephp#17388 shows that this is not possible.

PHP Fatal error:  Declaration of Cake\Http\Response::withStatus($code, $reasonPhrase = '') must be compatible with Psr\Http\Message\ResponseInterface::withStatus(int $code, string $reasonPhrase = ''): Psr\Http\Message\ResponseInterface in /home/runner/work/cakephp/cakephp/src/Http/Response.php on line 619

I am sure tons of other libraries will run into the same issue sooner or later, so this is not about reviving, but providing
reasable minimum compatibility.

BTW, we use --ignore-platform-reqs in some CI pipelines ourselves, where upgrades aren't feasible yet :-)

This will not help in this case for all the libraries, as the constraint is still blocking it beyond the time schedule of stable release of 8.3 - and it creates other side effects as pulling invalid other dependencies that can create noise and make it harder to determine the actual compatibility in regards to this library and PHP 8.3 alone.

@dereuromark dereuromark changed the title 2.x series does not support 8.3 2.x series does not support PHP 8.3 Oct 26, 2023
@Ocramius
Copy link
Member

@dereuromark it looks like the problem is not with this library, but with the transient dependency on psr/http-message?

Perhaps you need to "require": {"psr/http-message": "^1"} for now?

We support both major versions:

"psr/http-message": "^1.1 || ^2.0"

@Ocramius
Copy link
Member

This will not help in this case for all the libraries, as the constraint is still blocking it beyond the time schedule of stable release of 8.3

That is fine BTW: we don't plan to release all libraries before PHP 8.3 is released, and in fact, we are working towards improving our dependency upgrades on that when the latest -RC are there, but we are not there yet /cc @Xerkus

@dereuromark
Copy link
Author

dereuromark commented Oct 26, 2023

I tried the suggested lock for http-message, but then shows the actual issue and why the PHP8.3 support for 2.x series was thought to be the only viable path:

Error: Call to undefined function Laminas\Diactoros\marshalUriFromSapi()

https://github.com/cakephp/cakephp/actions/runs/6653289986/job/18078957898#step:18:183

We would have to move several functions / classes into our namespace in order for it to work out:
cakephp/cakephp#17388
Quite the heavy changes, as well as not fully BC since those classes might be expected on projects or alike.

@Ocramius
Copy link
Member

@laminas/technical-steering-committee do we want to do another 2.x minor? Is that viable?

@boesing
Copy link
Member

boesing commented Oct 26, 2023

Did we replace the sapi thing? if so, there might be an alternative. cake could check if function exist to make itself compatible with both versions.
but thats only a brain dump cuz I remember that we did something with the URL marshaling stuff. So there must be an alternative.

@boesing
Copy link
Member

boesing commented Oct 26, 2023

Regarding php support of v2, if that is doable without code changes, I would probably not mind backporting it.

@weierophinney
Copy link
Member

The problem with issuing a new minor on v2 is that we then need to support that branch for another 3 years with security fixes. Considering the number of changes in v3, this could potentially be a lot of work for us to maintain.

@dereuromark
Copy link
Author

I don't think you do, you can also state that in the release.
All people care about is that they can use it with PHP 8.3 - which would be possible without the constraint locking it down.
So allowing it to be there should not have anything to do with any other maintenance to that code.

@dereuromark
Copy link
Author

If the changes are that minimal, you can also just do a patch release on

2.26.x

A minor seems almost overkill to quickly allow something that is quite obvious in nature regarding compatibility and in light of 2023/2024 support of PHP.

@weierophinney
Copy link
Member

@dereuromark

I don't think you do, you can also state that in the release.
All people care about is that they can use it with PHP 8.3 - which would be possible without the constraint locking it down.
So allowing it to be there should not have anything to do with any other maintenance to that code.

This is simply not true. We have a published security policy, and it states that we maintain a release branch until the minimum supported PHP version is no longer supported by php.net.

What you think is beside the point here; we are the ones who have to maintain it, and making exceptions to things like security policies loses our users' trust in the project. If we make a new release branch, we have to maintain it.

A minor seems almost overkill

However, again, our policy is that we only add support for new PHP versions in new minor releases. There are a ton of reasons why we do this, and I'm not going to go into them here.

I'm not saying we won't do it; I'm pointing out the ramifications of doing it so that those of us on the steering committee know what we are committing to if we do so.

@ADmad
Copy link
Contributor

ADmad commented Oct 27, 2023

Regarding php support of v2, if that is doable without code changes, I would probably not mind backporting it.

It seems to work fine with PHP 8.3 without any code changes https://github.com/ADmad/laminas-diactoros/actions/runs/6667118891

@Ocramius
Copy link
Member

@ADmad send a patch against 2.26.x (with composer.lock hash updated, please), please, and I shall prepare a release and see how I can merge it up too.

@gsteel
Copy link
Member

gsteel commented Nov 2, 2023

Resolved via #176

@gsteel gsteel closed this as completed Nov 2, 2023
@ADmad
Copy link
Contributor

ADmad commented Nov 3, 2023

Thanks you guys for this. Really appreciate the fact that the laminas team is willing to make changes to accommodate down stream users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question Further information is requested
Projects
None yet
Development

No branches or pull requests

7 participants