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 support for composer 2 #617

Merged
merged 2 commits into from Jun 14, 2020
Merged

Add support for composer 2 #617

merged 2 commits into from Jun 14, 2020

Conversation

jderusse
Copy link
Member

@jderusse jderusse commented Apr 25, 2020

This PR add compatibility with composer-plugin-api: ^2.0 and fixes #661

Maintaining compatibility with both 1.x and 2.0 was too complex.
I think we need to release a version 2.0 of flex compatible with composer 2.

  • remove ParallelDownloader (no more needed with composer 2)
  • replace TruncateRepository by eventListener on PluginEvents::PRE_POOL_CREATE

note: I didn't change the way commands are added to the application, current plugin's contract expect a class name, while we currently inject command instances (with config and flex itself injected inside) in the application.

@jderusse jderusse force-pushed the composer-2 branch 5 times, most recently from 79d8cca to 87a1084 Compare Apr 25, 2020
src/Command/RecipesCommand.php Outdated Show resolved Hide resolved
@maxhelias
Copy link
Contributor

@maxhelias maxhelias commented May 1, 2020

@jderusse for the command instances, I think we can declare the flex class itself as the command provider. This would allow us to keep the injection of the configuration and flex

@jderusse
Copy link
Member Author

@jderusse jderusse commented May 1, 2020

That's a good idea @maxhelias , I totally missed that feature I implemented ^^

Unfortunaly, it does not works for some command (require, install, update). Composer prevent overriding internal methods (https://github.com/composer/composer/blob/master/src/Composer/Console/Application.php#L180)

One solution could be to declare the command with name "symfony:require" and alias "require" (aliases are not (yet) checked, and will override the composer's command)
WDYT @nicolas-grekas

side note: Composer improved a lots dependencies resolution. TruncatedPackage is maybe not needed anymore? Do you have good project to benchmark?

@nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented May 1, 2020

Unfortunaly, it does not works for some command (require, install, update)

yep, that's why we used this technic. Then, better have the same strategy for all commands.

TruncatedPackage is maybe not needed anymore? Do you have good project to benchmark?

it's not only for performance, but to enforce the version of nested dependencies. So yes, it's needed.

@k0d3r1s
Copy link

@k0d3r1s k0d3r1s commented May 16, 2020

hey, what's happening with this one? any eta?

@stof
Copy link
Member

@stof stof commented May 18, 2020

Well, as I discussed in the issue, enforcing constraints should be doable with a listener on the pool building in Composer 2. This would remove the need for TruncatedPackageRepository.

And for commands, adding support for aliases in require, update and remove should be doable by manipulating the arguments using the symmfony/console console.command event instead.

@jderusse
Copy link
Member Author

@jderusse jderusse commented May 19, 2020

Well, as I discussed in the issue, enforcing constraints should be doable with a listener on the pool building in Composer 2. This would remove the need for TruncatedPackageRepository.

The TruncatedPackageRepositoryis already removed, and replaced by a listener on PluginEvents::PRE_POOL_CREATE

And for commands, adding support for aliases in require, update and remove should be doable by manipulating the arguments using the symmfony/console console.command event instead

Hmm composer does not inject the EventDispatcher in BaseApplication, (bu can be doable in the backtrace hack). And command override is not only about alias (ie. adding option unpack in require). I afraid we still have to replace original's commandes and keep the hack.

PR rebased

@vasilvestre
Copy link

@vasilvestre vasilvestre commented May 20, 2020

This is needed for PHP 8 on Symfony Flex (and so in all other versions of Symfony). #624

@vasilvestre
Copy link

@vasilvestre vasilvestre commented May 22, 2020

@jderusse could you please rebase ?

@nicolas-grekas nicolas-grekas force-pushed the composer-2 branch 6 times, most recently from 6d1e73c to f90d97a Compare Jun 13, 2020
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

This PR is now compatible with composer 1 and 2.
I think it's ready to merge: the code for composer 1 didn't change, and the one for composer 2 is experimental anyway as long as composer 2 also is.

@fabpot
Copy link
Member

@fabpot fabpot commented Jun 14, 2020

Thank you @jderusse.

@fabpot fabpot merged commit 0fc2cd9 into symfony:master Jun 14, 2020
2 checks passed
lasdou
Copy link

@lasdou lasdou commented on d790a5c Jun 14, 2020

Choose a reason for hiding this comment

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

thanks for restore the composer 1 compatibily, i was not understand why my composer install fails, now it's working again. big thank you for the quick fix.

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

8 participants