wikimedia / composer-merge-plugin

Merge one or more additional composer.json files at Composer runtime
MIT License
935 stars 160 forks source link

Add support for Composer v2 #189

Closed mcaskill closed 3 years ago

mcaskill commented 4 years ago

Latest Update 2021-01-25 14:25 EST

Notes:

  1. Avoid unnecessary issue bumping. Please test this pull request and report issues or approve working state.
    See comment.
  2. Stability of this pull request's changeset needs additional testing from the community, such as unintentional updates upon requiring this plugin.
    See comment.
  3. Project is stuck in the process of transferring ownership to a new team within the Wikimedia Foundation.
    See comment.
  4. Since v1.4 of the plugin does not support Composer v2, the plugin is ignored when updating to v1.5. As a result, Composer is unaware of the merged dependencies and removes them. Either update the plugin with Composer v1 first or run composer update again.
    See comment.
  5. Unintentional lock file updates caused by isFirstInstall() on Composer v2. Listening only to post-update-cmd instead of post-install-cmd might be more sensible. See comment and followup.
  6. Please be patient, we don't want to push broken changes.

How to test:

{
    "repositories": [
        {
            "type":"vcs",
            "url":"https://github.com/mcaskill/composer-merge-plugin"
        }
    ],
    "require": {
        "wikimedia/composer-merge-plugin": "dev-feature/composer-v2 as 1.5.0"
    }
}

Tasks:


Replaces #187, #185, as fix for #184 (as per composer/composer#8726)

I've tested using the example and the unit tests and it works in Composer v1 and v2.

This alternative proposal forgoes PRE_DEPENDENCIES_SOLVING entirely to avoid over-complicating the codebase and streamline the plugin's merge logic.

The issue with the previous attempts (as well as attempts on other plugins) stems from a miscommunication from the Composer team of the major difference in how v1 and v2 resolve and install dependencies.

[…] Roughly speaking:

  • Composer v1: Composer resolves dependencies (dispatching *_DEPENDENCIES_SOLVING), iterates packages (while dispatching PRE_PACKAGE_* before PRE_FILE_DOWNLOAD), then finally writes the lock file.
  • Composer v2: Composer resolves dependencies (dispatching PRE_POOL_CREATE), writes the lock file, dispatches PRE_OPERATIONS_EXEC, downloads the packages (while dispatching PRE_FILE_DOWNLOAD), then iterates packages (while dispatching PRE_PACKAGE_*).

In particular, people are assuming that PRE_OPERATIONS_EXEC is a replacement for PRE_DEPENDENCIES_SOLVING since they are dispatched in a similar-looking routine. The closest event to the latter would actually be PRE_POOL_CREATE. — composer/composer#8726

Back to my proposal.

Currently, the use of PRE_DEPENDENCIES_SOLVING in the plugin is used to inject duplicate requirements (usually with a different version constraints) into the solver (while distinguishing between require and require-dev).

What I propose replaces the duplicate links tracking in ExtraPackage by back porting Composer 2's new static MultiConstraint::create() method which can be used to resolve complex-constraints early on. In turn, this makes the need for PRE_DEPENDENCIES_SOLVING obsolete.

I hope this PR will, at the very least, help to figure out how to support Composer v2.

Seldaek commented 3 years ago

FWIW I tend to agree that bumping dependencies is not a BC break. As long as your API does not change, it's not a BC break. Your dependencies are in theory not part of the API, they're an implementation detail the consumers should not have to know about.

That said, with PHP version I can see that this being a dependency of all php code, including your consumers, it can make sense to treat it as a BC break too. Projects like Symfony for example do this and I find it reasonable too, it's much easier to communicate to users for starters, and it's part of their BC promise, which is an extension of semver.

The main benefit I see of bumping to 2.0 here would be that it leaves the door open to releasing a new 1.5 for new features for composer 1, but if you don't see yourselves doing that then IMO leaving it at 1.5 now makes more sense, and it simplifies the upgrade path as people requiring the merge plugin with ^1 won't have to do anything to benefit from the Composer 2 support.

mcaskill commented 3 years ago

I think people have lost track of why the "1.5 vs 2.0" proposal.

Me and @tstarling are referring to renaming the plugin's PHP namespace to prevent MergePlugin 1.5/2.0 from using the, potentially already autoloaded, ExtraPackage (getMergedRequirements()) and PluginState (isComposer1()) from 1.4 since I've introduced breaking changes.

We could introduce a similar class-swapping mechanism as Composer's PluginManager or introducing a versioned namespace to the Merge Plugin.

Instead of Wikimedia\Composer\Merge2, maybe Wikimedia\Composer\Merge\V2 […]

Such namespacing implies we bump the package's version to 2.0.0 because how you namespace 1.5.x.

Alternatively, maybe we can rename ExtraPackage (to Package) and PluginState (?).