-
Notifications
You must be signed in to change notification settings - Fork 16
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
Fixes #16: Support Composer 2. #17
Conversation
I would say this plugin should not be needed anymore in v2, we have even more optimizations in the pipeline compared to current snapshots, so hopefully it becomes completely unnecessary to do such hackery. |
It would still be useful to have Composer 2 support (even if it does nothing useful) for the transition period. |
You could release a new version (or something like that) that is compatible with Composer 2 but implements no functionality. Then projects could declare a depedency on `^1.1 | ^2' and rely on composer to pick the correct version. Or you could even call the version 9.99.99 to point out that it is end of life. |
@alexpott Wouldn't that be a problem for projects where some developers use composer 1 and some composer 2 simultaneously? The compatible version would be then resolved once, for developer running |
@zaporylie I don't think so. Once you update to composer 2 and commit a lock file you're going to get
in your lock file. At that point I wouldn't want to guarantee that composer 1 works on your project. If you've not committed the lock file - which is best for libraries then composer will resolve the correct version for you. I don't think that using composer 1 and 2 on the same lock file is a reasonable expectation. But I maybe wrong. |
IMO the best way and that's what I wrote on composer/composer#8726 is to ship a version of the plugin capable of running on both composer 1 and 2. If you can easily do that then do it, it'll be easier for everyone. Lock files from v2 and v1 are compatible, and composer 1 will remain usable on projects where some people/the lock file started using v2. The main possible incompatibility point is using a plugin which requires composer 1 or 2. |
This PR looks incomplete. The |
Performance improvements in Composer 2 are massive and I think best is to just early return from Plugin when Composer 2 is detected. This will give proper support for both Composer 1 and 2 and allow teams with developers using Composer 1 and 2 to work on the same repo. I guess this package can also be marked as potentially deprecated and be maintained only until Composer 1 EOL. I pushed an additional commit to this PR + rebased so we have tests in place - if anyone could look at it I'm ok in merging it asap. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me. Maybe change wording to recommend to remove the plugin for composer 2
Yes, +1 on landing this. |
I can't formally approve my own PR, but this looks good to me. Thanks! |
Is there anything blocking this? It would make it easier to unlock testing of other parts of composer2 with existing Drupal sites. |
Thank you! |
Thanks to everyone involved 🙂 |
Fixes #16
This is just a draft and known to be incomplete. At a minimum, Composer's RemoteFilesystem no longer exists, you need to switch to the HttpDownloader instead. There may be other issues as well.
You should follow the upgrade guide and test to ensure it works as expected: composer/composer#8726
However, I'm actually not sure if this plugin is even needed with Composer 2. Maybe it would be better to deprecate it? Check out these benchmarks for an install with Drupal core:
Composer 1 without Composer Drupal Optimizations
Composer 1 with Composer Drupal Optimizations
Composer 2 without Composer Drupal Optimizations
As you can see, Composer 2 without this plugin is an order of magnitude more efficient than even Composer 1 with the plugin. Porting this plugin to Composer 2 might make those numbers even better, but I'm not sure how much so.