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

Work around Composer multi-version loading hack using a "V2" namespace #206

Merged
merged 3 commits into from
Feb 24, 2021

Conversation

tstarling
Copy link
Contributor

Composer would try to load two versions of the same plugin, by reading the main plugin file and rewriting the class name. But this doesn't work if the plugin has multiple classes. It tries to run the new plugin class with old helper classes.

So:

  • Move all classes to Wikimedia\Composer\Merge\V2. This includes classes that were previously in the parent namespace.
  • Clean up unused "use" statements.

Note that the namespace version number will have to be incremented every time there is an internal interface change.

Closes #205

Composer would try to load two versions of the same plugin, by reading
the main plugin file and rewriting the class name. But this doesn't work
if the plugin has multiple classes. It tries to run the new plugin class
with old helper classes.

So:

* Move all classes to Wikimedia\Composer\Merge\V2. This includes classes
  that were previously in the parent namespace.
* Clean up unused "use" statements.

Note that the namespace version number will have to be incremented every
time there is an internal interface change.

Closes wikimedia#205
@tstarling tstarling requested a review from reedy February 23, 2021 05:36
src/ExtraPackage.php Outdated Show resolved Hide resolved
Copy link
Contributor

@mcaskill mcaskill left a comment

Choose a reason for hiding this comment

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

The only alternative to this approach would be to implement runtime class replacement like Composer does for the main plugin file (see /src/Composer/Plugin/PluginManager.php).

What if we introduced a Factory class that could do that, if it detects a key method is missing?
In our situation ExtraPackage::getMergedRequirement() or PluginState::isComposer1().

@tstarling
Copy link
Contributor Author

I'm not really interested in exploring alternative approaches, I just want to release it and move on to the next thing. This way is simple, it works, and it's done already. If @mcaskill approves then I'll merge and release it as 2.0.0.

composer.json Outdated Show resolved Hide resolved
Co-authored-by: Chauncey McAskill <chauncey@mcaskill.ca>
@tstarling tstarling merged commit 8ca2ed8 into wikimedia:master Feb 24, 2021
@tstarling
Copy link
Contributor Author

I tagged "2.0.0" before I realised that "v2.0.0" would be the correct convention for this package (for other packages I have used the plain version number without the "v"). So I deleted the tag, deleted the package version from packagist, and tagged v2.0.0. I didn't see v2.0.0 in packagist, so I released the same commit as v2.0.1, expecting that packagist got confused about the deleted version. Both v2.0.0 and v2.0.1 did end up in packagist eventually.

The point is, this is released as 2.0.1, please test and enjoy.

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

Successfully merging this pull request may close these issues.

Update throws php-error
3 participants