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

Add composer plugin to auto-install missing implementations #208

Merged
merged 1 commit into from
Feb 9, 2023
Merged

Add composer plugin to auto-install missing implementations #208

merged 1 commit into from
Feb 9, 2023

Conversation

nicolas-grekas
Copy link
Collaborator

@nicolas-grekas nicolas-grekas commented Jan 18, 2023

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Documentation php-http/documentation#301
License MIT

This PR makes discovery able to auto-install a missing implementation when a dependency requires both php-http/discovery and one of the supported *-implementation virtual packages.

The code comes from https://github.com/FriendsOfPHP/well-known-implementations, which I plan to retire if this approach is accepted.

Note that we started discussing this topic with @dbu at SymfonyCon and we agreed this could be the best path forward.

To Do

  • Updated CHANGELOG.md to describe new feature
  • Documentation pull request created
  • Make the plugin work with Composer v1 and PHP 7.1
  • Add rule to install http-interop/http-factory-guzzle when Guzzle's PSR-7 v1 is needed/installed and PSR-17 implementation is required
  • Add more test

@nicolas-grekas
Copy link
Collaborator Author

nicolas-grekas commented Jan 20, 2023

PR is ready. It comes with updates to the CI. (The remaining failures are unrelated)

Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

thanks a lot for the contribution!

and thanks for the cleanup of the build system. i will extract those changes into a separate MR to fix the issues separately.

i have a few questions, mostly requesting more documentation to make the configuration of the plugin understandable without understanding the algorithm in its very details.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/plugin.yml Show resolved Hide resolved
.github/workflows/plugin.yml Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
src/Composer/Plugin.php Show resolved Hide resolved
src/Composer/Plugin.php Show resolved Hide resolved
src/Composer/Plugin.php Show resolved Hide resolved
src/Composer/Plugin.php Show resolved Hide resolved
src/Composer/Plugin.php Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@dbu
Copy link
Contributor

dbu commented Feb 8, 2023

i merged the build setup cleanups to 1.x branch. please rebase this MR :-)

@nicolas-grekas
Copy link
Collaborator Author

PR rebased and green, comments addressed. Thanks for the review!

composer.json Show resolved Hide resolved
src/Composer/Plugin.php Outdated Show resolved Hide resolved
src/Composer/Plugin.php Outdated Show resolved Hide resolved
@nicolas-grekas
Copy link
Collaborator Author

nicolas-grekas commented Feb 8, 2023

Comments addressed

@nicolas-grekas
Copy link
Collaborator Author

Rebased, all green, ready for merge + tag :)

Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

thanks for the contribution

@clxmstaab
Copy link

clxmstaab commented Feb 9, 2023

just to ping the relevant people: this PR lead to a breaking change for the current minor version which was reported in #213

no offence.

@Tofandel
Copy link

Tofandel commented Feb 9, 2023

It also has a bug, running composer install now with the plugin enabled leads to an infinite loop of composer install

@nicolas-grekas nicolas-grekas deleted the composer-plugin branch February 9, 2023 14:09
@nicolas-grekas
Copy link
Collaborator Author

@Tofandel thanks for the reproducer, I opened #214 to fix the issue.

@dunglas
Copy link

dunglas commented Feb 10, 2023

There is another issue with this plugin. If you pass options to composer update such as --ignore-platform-reqs they are discarded when the second update is discarded, and so the second update fails.

@nicolas-grekas
Copy link
Collaborator Author

Good catch @dunglas. This should be fixed by #216

nicolas-grekas added a commit to symfony/symfony that referenced this pull request Feb 10, 2023
…mplementations" (nicolas-grekas)

This PR was merged into the 6.2 branch.

Discussion
----------

[HttpClient] Revert support for "friendsofphp/well-known-implementations"

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This reverts PR #47950

The major features of "friendsofphp/well-known-implementations" are now available in "php-http/discovery"  [v1.15.0](https://github.com/php-http/discovery/releases/tag/1.15.0), see:
- php-http/discovery#209
- php-http/discovery#208

Given the extremely low download stats of "friendsofphp/well-known-implementations", I propose to just remove support for it as a bugfix:
https://packagist.org/packages/friendsofphp/well-known-implementations

This will save us from maintaining this integration, which is unused code in practice anyway.

Commits
-------

081aa00 Revert "[HttpClient] Add support for "friendsofphp/well-known-implementations""
leofeyer pushed a commit to contao/contao that referenced this pull request Feb 15, 2023
Description
-----------

See also php-http/discovery#208, contao/managed-edition#62 and contao/conflicts#44.

Fixes the CI chain.

Commits
-------

5b2b068 disallow php-http/discovery
leofeyer pushed a commit to contao/newsletter-bundle that referenced this pull request Feb 15, 2023
Description
-----------

See also php-http/discovery#208, contao/managed-edition#62 and contao/conflicts#44.

Fixes the CI chain.

Commits
-------

5b2b068a disallow php-http/discovery
leofeyer pushed a commit to contao/comments-bundle that referenced this pull request Feb 15, 2023
Description
-----------

See also php-http/discovery#208, contao/managed-edition#62 and contao/conflicts#44.

Fixes the CI chain.

Commits
-------

5b2b068a disallow php-http/discovery
leofeyer pushed a commit to contao/installation-bundle that referenced this pull request Feb 15, 2023
Description
-----------

See also php-http/discovery#208, contao/managed-edition#62 and contao/conflicts#44.

Fixes the CI chain.

Commits
-------

5b2b068a disallow php-http/discovery
leofeyer pushed a commit to contao/listing-bundle that referenced this pull request Feb 15, 2023
Description
-----------

See also php-http/discovery#208, contao/managed-edition#62 and contao/conflicts#44.

Fixes the CI chain.

Commits
-------

5b2b068a disallow php-http/discovery
leofeyer pushed a commit to contao/test-case that referenced this pull request Feb 15, 2023
Description
-----------

See also php-http/discovery#208, contao/managed-edition#62 and contao/conflicts#44.

Fixes the CI chain.

Commits
-------

5b2b068a disallow php-http/discovery
leofeyer pushed a commit to contao/faq-bundle that referenced this pull request Feb 15, 2023
Description
-----------

See also php-http/discovery#208, contao/managed-edition#62 and contao/conflicts#44.

Fixes the CI chain.

Commits
-------

5b2b068a disallow php-http/discovery
leofeyer pushed a commit to contao/news-bundle that referenced this pull request Feb 15, 2023
Description
-----------

See also php-http/discovery#208, contao/managed-edition#62 and contao/conflicts#44.

Fixes the CI chain.

Commits
-------

5b2b068a disallow php-http/discovery
leofeyer pushed a commit to contao/calendar-bundle that referenced this pull request Feb 15, 2023
Description
-----------

See also php-http/discovery#208, contao/managed-edition#62 and contao/conflicts#44.

Fixes the CI chain.

Commits
-------

5b2b068a disallow php-http/discovery
leofeyer pushed a commit to contao/core-bundle that referenced this pull request Feb 15, 2023
Description
-----------

See also php-http/discovery#208, contao/managed-edition#62 and contao/conflicts#44.

Fixes the CI chain.

Commits
-------

5b2b068a disallow php-http/discovery
leofeyer pushed a commit to contao/manager-bundle that referenced this pull request Feb 15, 2023
Description
-----------

See also php-http/discovery#208, contao/managed-edition#62 and contao/conflicts#44.

Fixes the CI chain.

Commits
-------

5b2b068a disallow php-http/discovery
BacLuc added a commit to ecamp/ecamp3 that referenced this pull request Apr 4, 2023
Was added in php-http/discovery in v1.15.0
PR: php-http/discovery#208
I would not activate this plugin in a lock file maintenance PR.
Issue to maybe enable the plugin: #3401
BacLuc added a commit to ecamp/ecamp3 that referenced this pull request Apr 5, 2023
Was added in php-http/discovery in v1.15.0
PR: php-http/discovery#208
I would not activate this plugin in a lock file maintenance PR.
Issue to maybe enable the plugin: #3401
brandon-reed1 added a commit to brandon-reed1/Discovery that referenced this pull request Jun 12, 2023
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

5 participants