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

WIP: make sure we install php-interop 1 when asking for the psr factory #263

Closed
wants to merge 1 commit into from

Conversation

dbu
Copy link
Contributor

@dbu dbu commented Mar 28, 2024

the 0.x versions are for the php-http factories

@dbu dbu force-pushed the fix-interop-stickyness branch 2 times, most recently from abda0a6 to 3f92725 Compare March 28, 2024 08:12
…he 0.x versions are for the php-http factories
@@ -78,7 +78,7 @@ class Plugin implements PluginInterface, EventSubscriberInterface
'slim/psr7' => [],
'laminas/laminas-diactoros' => [],
'phalcon/cphalcon:^4' => [],
'http-interop/http-factory-guzzle' => [],
'http-interop/http-factory-guzzle:>=1' => [],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas is this not the correct way to do this? i noticed in the FOSHttpCache --prefer-lowest installs that i end up with http-interop/http-factory-guzzle 0.2.0 which is for the legacy php-http factories and not the psr factories.

when i use this branch, the plugin adds a require for http-interop/http-factory-guzzle: * and then goes into an endless loop. i accidentally created a reproducer in FriendsOfSymfony/FOSHttpCache#567

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a look at the code and we don't support version constraints on the auto-installed packages.
Instead, conflict rules should be used on the consuming side (so that the correct dep is installed with or without the plugin)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is that http-interop/http-factory-guzzle is only a psr/http-factory-implementation if we use at least version 1. we have version constraints on other packages in this list, just above, e.g. phalcon or guzzlehttp/psr7. would those also break if an older version was to be installed?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other version constraints apply on the "what's already installed" side, not on the "what should be installed" one...

@dbu dbu changed the title make sure we install php-interop 1 when asking for the psr factory WIP: make sure we install php-interop 1 when asking for the psr factory Mar 28, 2024
@dbu
Copy link
Contributor Author

dbu commented May 4, 2024

thanks for the clarification.

not perfect, but prefer lowest is a CI question rather than a real world use case. and with minimal php versions increasing, this particular issue will fade out. fixing this on the FOSHttpCache side for now.

@dbu dbu closed this May 4, 2024
@dbu dbu deleted the fix-interop-stickyness branch May 4, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants