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

Fixes #16: Support Composer 2. #17

Merged
merged 3 commits into from
Oct 22, 2020
Merged

Conversation

danepowell
Copy link
Contributor

@danepowell danepowell commented Apr 22, 2020

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

Memory usage: 424.89MiB (peak: 1133.92MiB), time: 18.24s

Composer 1 with Composer Drupal Optimizations

Memory usage: 342.13MiB (peak: 459.25MiB), time: 8.43s

Composer 2 without Composer Drupal Optimizations

Memory usage: 21.84MiB (peak: 135.12MiB), time: 9.57s

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.

@danepowell danepowell mentioned this pull request Apr 27, 2020
@Seldaek
Copy link

Seldaek commented May 1, 2020

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.

@prudloff-insite
Copy link

It would still be useful to have Composer 2 support (even if it does nothing useful) for the transition period.
We are likely to have contributors using both Composer 1 and Composer 2 for a while.

@alexpott
Copy link

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.

@zaporylie
Copy link
Owner

@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 composer update zaporylie/composer-drupal-optimizations and locked in composer.lock.
I am leaning toward making plugin compatible with both versions for the transition period.

@alexpott
Copy link

@zaporylie I don't think so. Once you update to composer 2 and commit a lock file you're going to get

-    "plugin-api-version": "1.1.0"
+    "plugin-api-version": "2.0.0"

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.

@Seldaek
Copy link

Seldaek commented Jul 21, 2020

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.

@effulgentsia
Copy link

This PR looks incomplete. The Plugin::activate() method calls Factory::createRemoteFilesystem(), but that was refactored in Composer 2.

@zaporylie
Copy link
Owner

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.

Copy link

@andypost andypost left a 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

@heddn
Copy link

heddn commented Oct 8, 2020

Yes, +1 on landing this.

@danepowell
Copy link
Contributor Author

I can't formally approve my own PR, but this looks good to me. Thanks!

@neclimdul
Copy link
Contributor

Is there anything blocking this? It would make it easier to unlock testing of other parts of composer2 with existing Drupal sites.

@zaporylie zaporylie merged commit 4e252a5 into zaporylie:master Oct 22, 2020
@andypost
Copy link

Thank you!

@zaporylie
Copy link
Owner

Thanks to everyone involved 🙂

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.

Support for Composer 2
10 participants