Problem/Motivation

Contrib modules which depend on core modules are currently not installable with composer-merge-plugin. One example is drupal/address. We discovered the problem shortly after DrupalCon and submitted a bugfix to the upstream project. The fix went in yesterday. The related PR is https://github.com/wikimedia/composer-merge-plugin/pull/89.

Lets update the plugin to the latest version to get it all working.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Comments

webflo created an issue. See original summary.

webflo’s picture

Status: Active » Needs review
StatusFileSize
new46.58 KB
webflo’s picture

Issue summary: View changes
webflo’s picture

+++ b/composer.lock
@@ -3762,39 +3761,8 @@
     "stability-flags": {
-        "php": 0,
-        "symfony/class-loader": 0,
-        "symfony/console": 0,
-        "symfony/dependency-injection": 0,
...

The stability-flags have been added in #2380389: Use a single vendor directory in the root. This is another bug which has been fixed.

webflo’s picture

Issue tags: +rc eligible
dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Ah great!

bojanz’s picture

The 1.3.0 release of merge plugin was just tagged, guessing that affects this patch?

webflo’s picture

StatusFileSize
new53.63 KB

New patch with the tagged release.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: 2608174-8.patch, failed testing.

webflo’s picture

Status: Needs work » Needs review
StatusFileSize
new53.67 KB
bojanz’s picture

Status: Needs review » Reviewed & tested by the community

Still good.

The last submitted patch, 2: 2608174-2.patch, failed testing.

The last submitted patch, 8: 2608174-8.patch, failed testing.

hussainweb’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new54.43 KB

I am mainly rerolling here, but I made a small change to composer.json and I'd like for someone to review it. It essentially means the same but IMO, gives a clearer meaning.

diff --git a/composer.json b/composer.json
index 04e1806..7f170ce 100644
--- a/composer.json
+++ b/composer.json
@@ -5,7 +5,7 @@
   "license": "GPL-2.0+",
   "require": {
     "composer/installers": "^1.0.21",
-    "wikimedia/composer-merge-plugin": "^1.3.0"
+    "wikimedia/composer-merge-plugin": "~1.3"
   },
   "replace": {
     "drupal/core": "~8.0"
hussainweb’s picture

Just FYI, the reroll conflict was probably because of #2609110: Update Twig to 1.23.1.

andypost’s picture

+++ b/composer.json
@@ -5,7 +5,7 @@
-    "wikimedia/composer-merge-plugin": "^1.3.0"
+    "wikimedia/composer-merge-plugin": "~1.3"

According https://getcomposer.org/doc/articles/versions.md#caret
"allow non-breaking updates"
so this change allows 1.* without checking compatibility... not sure that right

hussainweb’s picture

@andypost: Re #16. If you interpret the versions correctly, ^1.3.0 is identical to ~1.3. This is what they translate to:

^1.3.0 => ">=1.3.0, <2.0"
~1.3 => ">=1.3, <2.0"

"allow non-breaking updates"

That is if the project follows semantic versioning (which many do). Both constraints will allow 1.* from 1.3 onwards. As per semantic versioning, there shouldn't be any breaking changes in 1.* at all, which means both constraints should not break. If there happens to be a breaking change down the line (unlikely), we would have to constraint the version in both cases.

So, why change? This discussion is actually the proof why. Many are confused by the caret operator and that is why it is better to switch it to the more readable representation if possible. See Twig's version constraint for example. ^1.23.1 translates to ">=1.23.1, <2.0" whereas ~1.23.1 would translate to ">=1.23.1, <1.24". They are quite different.

I hope this helps understand the caret operator and why "~1.3" is identical to "^1.3.0".

andypost’s picture

Status: Needs review » Reviewed & tested by the community

@hussainweb thanx for explanation!
that works

mile23’s picture

+1 on the RTBC as it stands. But it would also be nice to use the new require section for core/composer.json.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 7485eaa and pushed to 8.0.x. Thanks!

@Mile23 I don;t think this should go in core/composer.json because I think it needs to be in the root composer.json as this is how merging actually works

  • alexpott committed 7485eaa on 8.0.x
    Issue #2608174 by webflo, hussainweb: Update composer-merge-plugin to...
mile23’s picture

I meant: Currently the root composer.json file has extra.include.core/composer.json. It *could* have extra.require.core/composer.json, which would error out if that file isn't present.

No worries though.

andypost’s picture

  • alexpott committed 7485eaa on 8.1.x
    Issue #2608174 by webflo, hussainweb: Update composer-merge-plugin to...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.