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

Composer 2.x compatibility #108

Closed
danepowell opened this issue Mar 31, 2020 · 17 comments · Fixed by #111
Closed

Composer 2.x compatibility #108

danepowell opened this issue Mar 31, 2020 · 17 comments · Fixed by #111

Comments

@danepowell
Copy link

danepowell commented Mar 31, 2020

The release of Composer 2.0 is imminent, and it requires updates for composer plugins such as this package. Until this package is explicitly made compatible with Composer 2, it cannot be installed with Composer 2 (even as a dependency of another package).

This package needs to be updated not only to ensure compatibility with Composer 2, but to unblock downstream projects that rely on it and want to support composer 2.

Hopefully you received an email from the Composer maintainer about this, but if not let me know and I can forward it to you or put you in contact. Cheers.

@jrfnl
Copy link
Member

jrfnl commented Apr 1, 2020

@danepowell While I agree it would be good to support Composer 2.0 once it comes out, I can't find any useful information to confirm what you are saying, let alone inform Composer plugin developers on what is needed to support Composer 2.

  • There is no Composer 2 pre-release yet (alpha/beta/RC);
  • There is no changelog which I would expect with such a release to give a clue about what breaking changes are involved;
  • There is no information about Composer 2 yet on https://getcomposer.org/
  • There is no wiki on GitHub with a roadmap for Composer 2 (timing/features);
  • There are no upgrade instructions available either in a wiki or elsewhere;
  • The Milestone on GitHub has no due date, so even the "imminent release" part can't be verified.

If you have links to relevant sources, please add them to this issue as, as things stand, it is impossible to action this for now.

@Seldaek
Copy link
Contributor

Seldaek commented Apr 1, 2020

I posted the info at composer/composer#8726 now - there still isn't a lot of what you mentioned, but it's a start :)

@jrfnl
Copy link
Member

jrfnl commented Apr 1, 2020

Thanks @Seldaek for chipping in and posting that information. I'll try and have a look at it in more detail soon.

Unfortunately this project doesn't have really have any integration tests at this moment, so testing this will not be easy for us as there are a lot of scenarios to consider.

@Seldaek
Copy link
Contributor

Seldaek commented Apr 1, 2020

Had a quick look through the source and tbh I don't think you'll have to change anything beyond the composer.json requirement. I might have missed something obviously but it doesn't seem to touch anything we broke.

@jrfnl
Copy link
Member

jrfnl commented Apr 1, 2020

@Seldaek You're a star! Thank you so much for that.

I presume we should add (empty) deactivate() and uninstall() methods though ? Not that anything should happen in that case for this plugin. Removing a set installed_paths may break more than it fixes.

I've just pushed up a WIP branch which can be used for testing: https://github.com/Dealerdirect/phpcodesniffer-composer-installer/tree/WIP/feature/support-composer-2.0
https://github.com/Dealerdirect/phpcodesniffer-composer-installer/tree/feature/support-composer-2.0

@Seldaek
Copy link
Contributor

Seldaek commented Apr 1, 2020

Yup that looks good as a first step. I'd say if some basic testing doesn't surface any issues it's probably fine..

@danepowell
Copy link
Author

Thanks, I've tested your WIP branch (using a local path repo) and confirmed that it works great with Composer 2. Let me know if you need anything else to get this into a new release.

@jrfnl
Copy link
Member

jrfnl commented Apr 20, 2020

@danepowell Thanks so much for testing this!

I kind of want to have a better read through the upgrade guide @Seldaek has posted since before opening a PR, but I hope to get to that some time this week.

@danepowell
Copy link
Author

Hey @jrfnl I don't want to rush you, but any chance you could provide a new timeline for this? Composer 2 is expected to be released in the next few weeks, and downstream projects that depend on phpcodesniffer-composer-installer can't start their own upgrade testing until phpcodesniffer-composer-installer has the integration. Thanks!

@jrfnl
Copy link
Member

jrfnl commented May 6, 2020

@danepowell I'd be happy to promote the WIP branch to a PR, but would still really like to see some more people testing it and confirming all is ok for lack of integration tests.

Two people testing for just their scenario for a plugin with millions of installs seems a bit meager....

@Seldaek
Copy link
Contributor

Seldaek commented May 6, 2020

@jrfnl a PR would increase visibility and likeliness that people test too IMO, plus it makes it easier to give feedback.

@jrfnl
Copy link
Member

jrfnl commented May 6, 2020

@Seldaek I just ran some tests myself before opening the PR and am running into problems. Will open an issue upstream.

@jrfnl
Copy link
Member

jrfnl commented May 6, 2020

@Seldaek Grr... one of those... I can't seem to create a scenario to consistently reproduce the issue. Might have been a fluke. Would you still like me to report the issue with the stack trace or leave it ?

This was the error I got:

  [Composer\DependencyResolver\SolverBugException]
  Reached invalid decision id 0 while looking through (-230) for a literal present in the analyzed rule (-75|457|458|459|1139).
  This exception was most likely caused by a bug in Composer.

jrfnl added a commit that referenced this issue May 6, 2020
Minimal changes to updated the plugin to support Composer 2.0 which is expected late May/beginning of June.

Changes are based on guidance found in:
* #108
* composer/composer#8726
* https://github.com/composer/composer/blob/master/UPGRADE-2.0.md#for-integrators-and-plugin-authors

Tested by danepowell and myself (Windows 7). Further testing would be very welcome!

Fixes 108
@Seldaek
Copy link
Contributor

Seldaek commented May 6, 2020 via email

@jrfnl
Copy link
Member

jrfnl commented May 6, 2020

@Seldaek I figured as much and haven't found a consistently reproducible scenario. If I do, I'll be sure to report it properly.

jrfnl added a commit that referenced this issue May 11, 2020
Minimal changes to updated the plugin to support Composer 2.0 which is expected late May/beginning of June.

Changes are based on guidance found in:
* #108
* composer/composer#8726
* https://github.com/composer/composer/blob/master/UPGRADE-2.0.md#for-integrators-and-plugin-authors

Tested by danepowell and myself (Windows 7). Further testing would be very welcome!

Fixes 108
@mxr576
Copy link

mxr576 commented Jun 25, 2020

Any update on this?

@jrfnl
Copy link
Member

jrfnl commented Jun 25, 2020

@mxr576 There is a PR open for this: #111 It would be great if you could test it and leave your feedback in the PR thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants