Problem/Motivation

Composer 2 is on its way, and may provide benefits to Drupal sites. It should be possible to use Composer 2 to run commands such as require, update and install on Drupal sites.

https://github.com/composer/composer/blob/master/CHANGELOG.md#200--2020-

Proposed resolution

  • Declare that our plugins work with the ^2 Composer API
  • Implement the new methods of the Composer 2.x plugin interface
  • Upgrade to composer/installers ^1.9 (earlier versions are not compatible with the Composer 2 plugin API)
  • Remove wikimedia/composer-merge plugin
  • Call OperationInterface:: getOperationType() instead of OperationInterface::getJobType() (renamed in Compsoer 2)
  • Skip tests that will not work with Composer 2 until there is a stable release of our Composer plugins that are compatible with Composer 2.

Note that Drupal also has Composer dependencies internally, including composer/composer (require-dev only) and composer/semver (required in drupal/core). Updating these components cannot be done until there is a stable release available; therefore, updating these components is out of scope for this issue. See below.

Remaining tasks

User interface changes

None

API changes

Drupal must react to some Composer API changes, as mentioned above; however, no APIs defined by Drupal change.

Data model changes

None

Release notes snippet

The Composer plugins provided by Drupal -- the scaffold plugin, the vendor hardening plugin, and the project message plugin -- are now compatible with Composer 2, and Drupal core now allows using either Composer 1 or 2.

CommentFileSizeAuthor
#103 3126566-8.9.x-103.patch8.6 KBgreg.1.anderson
#103 101-to-103-interdiff.txt2.74 KBgreg.1.anderson
#101 3126566-8.9.x-101.patch8.97 KBgreg.1.anderson
#101 96-to-101-interdiff.txt581 bytesgreg.1.anderson
#96 3126566-96.patch9.54 KBgreg.1.anderson
#96 64-to-96-interdiff.txt5.16 KBgreg.1.anderson
#89 interdiff-61-83.txt1.47 KBhussainweb
#83 3126566-82.patch11 KBhussainweb
#83 interdiff-72-82.txt23.45 KBhussainweb
#72 3126566-72.patch34.33 KBhussainweb
#72 interdiff-61-72.txt24.46 KBhussainweb
#64 interdiff-62-64.txt1.23 KBjungle
#64 3126566-64.patch11.85 KBjungle
#62 raw-interdiff-61-62.txt6.32 KBjungle
#62 3126566-8.9.x-62.patch10.62 KBjungle
#61 interdiff-59-61.txt887 bytesjungle
#61 3126566-61.patch9.99 KBjungle
#59 3126566-59.patch10.09 KBgreg.1.anderson
#59 53-to-59-interdiff.txt693 bytesgreg.1.anderson
#53 3126566-53.patch10.12 KBtedbow
#53 interdiff-50-53.txt1.1 KBtedbow
#50 3126566-50.patch10.05 KBjungle
#50 interdiff-49-50.txt2.04 KBjungle
#49 3126566-49.patch10.08 KBtedbow
#49 interdiff-47-49.txt1.58 KBtedbow
#47 3126566-47-9.0.x.patch9.74 KBtedbow
#29 18-to-29-interdiff.txt31.96 KBgreg.1.anderson
#29 9.0.x-3126566-29.patch13.17 KBgreg.1.anderson
#29 8.9.x-3126566-29.patch15.76 KBgreg.1.anderson
#22 3126566-22.patch13.17 KBgreg.1.anderson
#22 20-to-22-interdiff.txt2.67 KBgreg.1.anderson
#20 3126566-20.patch13.46 KBgreg.1.anderson
#18 3126566-18.patch40.53 KBgreg.1.anderson
#18 15-to-18-interdiff.txt4.34 KBgreg.1.anderson
#15 3126566-15.patch42.12 KBgreg.1.anderson
#15 12-to-15-interdiff.txt5.71 KBgreg.1.anderson
#12 3126566-12.patch36.41 KBgreg.1.anderson
#12 10-to-12-interdiff.txt12.88 KBgreg.1.anderson
#10 3126566-10.patch31.78 KBgreg.1.anderson
#10 6-to-10-interdiff.txt615 bytesgreg.1.anderson
#6 3126566-6.patch31.17 KBgreg.1.anderson
#6 4-to-6-interdiff.txt4.92 KBgreg.1.anderson
#4 3126566.patch3.09 KBgreg.1.anderson
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch created an issue. See original summary.

andypost’s picture

In related it was reported that psr-4 checks are more strict

heddn’s picture

Not directly related, but linking in anyway: https://github.com/composer/composer/pull/8655

greg.1.anderson’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
3.09 KB

Just bumping our Composer plugin compatibility to allow for either ^1 or ^2 seems to mostly do the trick for making our plugins work with Composer 2. It is also necessary to implement the two new methods of the plugin interface. With these changes, our plugins work under ad-hoc testing using the latest snapshot of Composer 2. Note that the test bot runs the tests currently run with Composer 1.x. When I install Composer 2.x on my local machine and run the scaffold plugin tests, then two of the scaffold tests fail. I haven't isolated why yet, but it looks like it has something to do with the techniques used to set up the test fixtures rather than actual bugs in the scaffold plugin.

Note also that the drupal/drupal project requires composer/composer ^1 for testing. I haven't yet run the tests with Composer 2.x in the lock file, nor have I ran the full test suite. We should discuss how we would like to do testing for Composer 1 and Composer 2, e.g. should we always use Composer 1 with Drupal 8.9.x, and always use Composer 2 for Drupal 9.0.x, or should we run tests using both Composer 1 and Composer 2 with Drupal 9.0.x, etc.

As mentioned in #2, additional required changes might be discovered once we increase the breadth of the Composer 2 testing.

greg.1.anderson’s picture

When attempting to install the Core Scaffold plugin test fixtures with Composer 2, the following errors are reported:

$ composer update
Loading composer repositories with package information
Updating dependencies
Nothing to modify in lock file
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 4 installs, 0 updates, 0 removals
  - Installing drupal/core-composer-scaffold (8.9.x-dev): Source already present
    Install of drupal/core-composer-scaffold failed
  - Installing fixtures/drupal-assets-fixture (dev-master): Source already present
    Install of fixtures/drupal-assets-fixture failed
  - Installing fixtures/drupal-core-fixture (dev-master): Source already present
    Install of fixtures/drupal-core-fixture failed
  - Installing fixtures/scaffold-override-fixture (dev-master): Source already present
    Install of fixtures/scaffold-override-fixture failed

This is likely a bug in Composer 2 path repositories, although I have not isolated the cause.

greg.1.anderson’s picture

Here's a patch with drupal/drupal's internal composer/composer dependency bumped to dev-master. Also upped composer/semver to 2.x, because that is required by Composer 2.x. This is just to see the failures caused by the path repository thing; maybe this will expose some of the issues hinted at in #2.

Built via composer update composer/composer --with-dependencies, so there are a couple of updates that might not be strictly necessary. composer.lock omitted from interdiff.

Status: Needs review » Needs work

The last submitted patch, 6: 3126566-6.patch, failed testing. View results

catch’s picture

#2951375: Allow for build targets is open for core min/max dependency testing.

Drupal 8.9 where it's depending on composer/composer should continue to specify Composer 1, so if 9.x switches to composer 2 then that is probably good enough for min/max until we have something proper sorted out?

greg.1.anderson’s picture

The wikimedia/composer-merge-plugin is not working with Composer 2 yet. Since we deprecated this plugin in Drupal 8.8.0, we should just remove it in Drupal 9. It is only present in drupal/drupal (core development); it is not used, but its presence causes warnings.

I create #3127013: Remove wikimedia/composer-merge-plugin to track.

greg.1.anderson’s picture

Status: Needs work » Needs review
FileSize
615 bytes
31.78 KB

Let's try to monkey-patch Composer 2 into the test bot.

Status: Needs review » Needs work

The last submitted patch, 10: 3126566-10.patch, failed testing. View results

greg.1.anderson’s picture

#10 revealed that we also need to upgrade composer/installers in order to use Composer 2. Support for Composer 2 is already available in composer/installers 1.9.0.

Also jumping in and removing wikimedia/composer-merge-plugin in this patch, for the same reason. It would still be best to get #3127013: Remove wikimedia/composer-merge-plugin committed in advance of this issue.

greg.1.anderson’s picture

Status: Needs work » Needs review

Setting status to run the tests.

Status: Needs review » Needs work

The last submitted patch, 12: 3126566-12.patch, failed testing. View results

greg.1.anderson’s picture

This patch might test green, although I disabled one feature and two tests in order to get here. See "remaining tasks", above.

greg.1.anderson’s picture

Status: Needs work » Needs review

The main purpose of #15 is to run the tests on the interim patch. :P Setting status so that happens.

greg.1.anderson’s picture

I requested a stable composer/semver 2.x release, because that would be somewhat convenient to have for Composer 2 testing.

greg.1.anderson’s picture

Here's a patch that restores the Core Composer Scaffold feature removed in #15, updated to work with either Composer 1 or Composer 2.

greg.1.anderson’s picture

Issue summary: View changes
greg.1.anderson’s picture

#18 proves that our changes to our Composer plugins are sufficient to make them compatible with Composer 2, but we obviously do not want to start using Composer 2.0@dev or composer/semver 2.0@dev yet.

Here's a patch that keeps all of the Composer compatibility changes needed from #18, including the version bump of composer/installers, but otherwise does not increase the version of any internal component. Composer 1.x is back in the lock file, and most tests are run with Composer 1.x. At the end, I bump Composer up to the 2.x dev release, and then run the Composer plugin tests again (n.b Composer 1.x is still in the lock file for these tests, but this works; the only code mixing between Composer 2 and Drupal AFAIK is in the functional tests when the Composer tool is used to run the Drupal Composer plugins, and these avoid using APIs other than Composer's).

I made the dubious decision to revert to Composer 1 testing AND switch to the 9.0.x branch in the same patch, so I have no interdiff from #18. I'll go back and make an 8.9.x version of this later so that I can rectify that.

jungle’s picture

+++ b/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
@@ -105,6 +105,11 @@ public function testVerifyTemplateTestProviderIsAccurate() {
+    if (strstr($composerVersionLine, 'Composer version 2') !== false) {

+++ b/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Functional/ScaffoldUpgradeTest.php
@@ -44,6 +44,10 @@ protected function setUp(): void {
+    if (strstr($composerVersionLine, 'Composer version 2') !== false) {

Should use !== FALSE, not !== false.

greg.1.anderson’s picture

Version: 8.9.x-dev » 9.0.x-dev
Issue tags: +Composer initiative
FileSize
2.67 KB
13.17 KB

My custom CI commands were not right, and apparently were causing the test to abort and re-run over and over again. Trying a different way. Also fixed style issues from #21.

greg.1.anderson’s picture

Issue summary: View changes
catch’s picture

Here's a patch that keeps all of the Composer compatibility changes needed from #18, including the version bump of composer/installers, but otherwise does not increase the version of any internal component. Composer 1.x is back in the lock file, and most tests are run with Composer 1.x

Doing the compatibility changes without bumping the version sounds good here - we can have a tracking issue for the actual update (with a patch to run tests against too). What do you mean by 'most tests' though?

jungle’s picture

Deprecation Notice: Class Symfony\Cmf\Component\Routing\Tests\Mapper\FieldMapEnhancerTest located in ./vendor/symfony-cmf/routing/Tests/Enhancer/FieldMapEnhancerTest.php does not comply with psr-4 autoloading standard. It will not autoload anymore in Composer v2.0. in phar:///usr/local/bin/composer/src/Composer/Autoload/ClassMapGenerator.php:201
Deprecation Notice: Class Symfony\Cmf\Component\Routing\PagedRouteCollectionTest located in ./vendor/symfony-cmf/routing/Tests/Routing/PagedRouteCollectionTest.php does not comply with psr-4 autoloading standard. It will not autoload anymore in Composer v2.0. in phar:///usr/local/bin/composer/src/Composer/Autoload/ClassMapGenerator.php:201
Deprecation Notice: Class Symfony\Cmf\Component\Routing\LazyRouteCollectionTest located in ./vendor/symfony-cmf/routing/Tests/Routing/LazyRouteCollectionTest.php does not comply with psr-4 autoloading standard. It will not autoload anymore in Composer v2.0. in phar:///usr/local/bin/composer/src/Composer/Autoload/ClassMapGenerator.php:201
Deprecation Notice: Class Html2Text\StrToUpperTest located in ./vendor/html2text/html2text/test/HeaderTest.php does not comply with psr-4 autoloading standard. It will not autoload anymore in Composer v2.0. in phar:///usr/local/bin/composer/src/Composer/Autoload/ClassMapGenerator.php:201
Deprecation Notice: Class HTMLPurifier_Language_en_x_test located in ./vendor/ezyang/htmlpurifier/library/HTMLPurifier/Language/classes/en-x-test.php does not comply with psr-0 autoloading standard. It will not autoload anymore in Composer v2.0. in phar:///usr/local/bin/composer/src/Composer/Autoload/ClassMapGenerator.php:201
Deprecation Notice: Class EasyRdf_Serialiser_JsonLd located in ./vendor/easyrdf/easyrdf/lib/EasyRdf/Serialiser/JsonLd_real.php does not comply with psr-0 autoloading standard. It will not autoload anymore in Composer v2.0. in phar:///usr/local/bin/composer/src/Composer/Autoload/ClassMapGenerator.php:201
Deprecation Notice: Class EasyRdf_Parser_JsonLd located in ./vendor/easyrdf/easyrdf/lib/EasyRdf/Parser/JsonLdImplementation.php does not comply with psr-0 autoloading standard. It will not autoload anymore in Composer v2.0. in phar:///usr/local/bin/composer/src/Composer/Autoload/ClassMapGenerator.php:201

Got some deprecation notices Composer v2.0 relevant, 3116414 is an issue of coder, which has similar warnings too.

greg.1.anderson’s picture

Title: Composer 2 compatibility » Composer 2 compatibility in Drupal's Composer plugins
Issue summary: View changes

#24: Good idea, I'll do that.

#25: Nothing we can do about that until we update those components. I'll add that to the follow-on issue. Since the follow-on issue already entails updating composer/composer and composer/semver, it makes sense to update the other dependencies with deprecations at the same time.

greg.1.anderson’s picture

What do you mean by 'most tests' though?

Sorry, I was editing my comment as I was working on my strategy for the patch. That sentence was a fragment that didn't get fixed up.

In #22, all tests are run on Composer 1.x. At the end of the test run, #22 runs the Composer plugin tests again using the Composer 2 cli. This means that the functional tests run with Composer 2.x; however, most of the other tests in this part of the run (the unit tests) still run against the Composer 1 APIs from our vendor directory, because I didn't try to do an on-the-fly upgrade of composer/composer and composer/semver in the tests. Doing that seemed dubious and fragile, so I thought it best to skip it at this stage, and just rely on the all-Composer-2-tests in #18 until the follow-on issue.

greg.1.anderson’s picture

Title: Composer 2 compatibility in Drupal's Composer plugins » Allow Drupal to work with Composer 2
Issue summary: View changes

This patch does a little more than just plugin compatibility, so my last title update isn't very accurate. Trying again.

greg.1.anderson’s picture

Here is a cleaned-up version of the 8.9.x patch. The patch from #22 is also re-uploaded here without change simply to avoid any confusion over which patches are most recent.

An interdiff with #18 is also provided; this shows the roll-back from using Composer 2 dependencies internally to this patch, which uses Composer 1 dependencies internally + changes to be compatible with Composer 2.

Finally, I discovered one more situation that arises with Composer 2 and Drupal 8/9:

$ composer update --lock
> Drupal\Composer\Composer::ensureComposerVersion
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires drupal/coder == 8.3.7.0, it is satisfiable by drupal/coder[8.3.7] from composer repo (https://repo.packagist.org) but drupal/coder[dev-2.x, 2.x-dev (alias of dev-2.x), 2.0.0-alpha1, 2.0.0-alpha2, 2.0.0-beta1, 2.0.0-rc1, 2.0.0, 2.1.0, 2.2.0, 2.3.0, 2.4.0, 2.5.0, 2.6.0, 2.7.0, 2.8.0, 2.9.0, 2.10.0, 2.11.0, 2.12.0, dev-3.x, 3.x-dev (alias of dev-3.x), 3.0.0, 3.1.0, 3.2.0, 3.3.0, 3.4.0, 3.5.0, 3.6.0, 3.7.0, 3.8.0] from composer repo (https://packages.drupal.org/8) has higher repository priority. The packages with higher priority do not match your constraint and are therefore not installable. See https://getcomposer.org/repoprio for details and assistance.

You are using a snapshot build of Composer 2, which may be the cause of the problem. Run `composer self-update --stable` and then try again. In case it solves the problem, please report an issue mentioning Composer 2.

It seems that what is happening here is that Composer 1 is happy to dismiss incompatible constraints from higher-priority repository and instead satisfy the request from a lower-priority repository, but Composer 2 considers this an error. I'm not sure if we do or do not have a good workaround if we have to adapt to this behavior.

greg.1.anderson’s picture

Good news: the drupal/coder problem reported in #29 only occurs in the drupal/drupal repository (core development).

I created a fork of drupal/recommended-project and its immediate dependencies, and applied the patch from #29. To reproduce my ad-hoc test:

$  composer create-project --stability=dev greg-1-anderson/drupal-recommended-project
$  cd drupal-recommended-project/
$  composer require drupal/ctools
$  composer require drupal/coder

I won't reproduce all of the output, but in the test run above, drupal/coder installed without incident.

greg.1.anderson’s picture

greg.1.anderson’s picture

Issue summary: View changes

Fixing bad edit in #31.

greg.1.anderson’s picture

greg.1.anderson’s picture

I should also mention that now that #3127013: Remove wikimedia/composer-merge-plugin has been merged to the 8.9.x branch, the 8.9.x patch on #29 needs a re-role. The 9.0.x patch on #29 was done after wikimedia composer merge was removed from the 9.0.x branch, though, and therefore is still fine.

I'll re-roll the 8.9.x patch after the 9.0.x is committed.

greg.1.anderson’s picture

We can fix the drupal/coder problem described in #29 if we switch to installing it from Drupal Packagist instead of Packagist. To do this, just switch to the version scheme in Drupal Packagist:

COMPOSER_ROOT_VERSION=8.9.x-dev composer require --dev drupal/coder:^3.7

I'm not clear on the difference between the drupal/coder in Packagist vs the drupal/coder on drupal.org. The version on drupal.org appears to be newer (has a 3.8.0 release that isn't on Packagist yet), so perhaps this solution is preferable, even?

If folks think this is a good solution, I'll update the patches to use the other repository to fetch drupal/coder.

UPDATE As pointed out in the issues linked below, installing drupal/coder from Drupal Packagist does not work, because doing so causes it to be installed as a module, to modules/contrib. We install from Packagist so that it will be installed to the vendor directory instead.

drumm’s picture

greg.1.anderson’s picture

Also:

Code in Packagist endpoint responsible for mapping projects to types.

greg.1.anderson’s picture

I think that #2851279: Update version requirements for drupal/coder to best practices is a good follow-on issue to track the drupal/coder problem. I commented on that issue to note that it is blocking Composer 2 use for Drupal core development. We won't be able to do that until there's a stable release of composer/semver (and maybe composer/composer as well), so we needn't let that issue block us here.

greg.1.anderson’s picture

Issue summary: View changes
greg.1.anderson’s picture

Semver 2.0.0 stable is out! This opens the door for us to bump up to `composer/semver: ^1|^2` in #3128631: Update dependencies composer/composer ^2 and composer/semver to ^3. We might want to decouple the composer/semver update from the composer/composer update.

Dane Powell’s picture

Status: Needs review » Reviewed & tested by the community

#29 seems to work well with both Composer 1 and 2, and the code looks good based on what I know about the Composer 1/2 upgrade process.

My interest is to get this into core ASAP so that downstream projects depending on core-composer-scaffold can also start work on the upgrade process: https://github.com/acquia/blt/pull/4088

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

The 9.0.x patch doesn't apply anymore.

alexpott’s picture

+++ b/core/drupalci.yml
@@ -55,3 +55,9 @@ build:
+      # Re-run Composer plugin tests after installing Composer 2
+      container_command.composer-upgrade:
+        commands:
+          - "sudo composer self-update --snapshot"
+          - "./vendor/bin/phpunit -c core --group VendorHardening,ProjectMessage,Scaffold"
+        halt-on-fail: true

This is super smart. I wish we could control the test matrix in the project though and not run this on postgres etc...

greg.1.anderson’s picture

#43: Maybe we should take out Composer 2 testing for now, and tackle testing as a follow-on issues.

xjm’s picture

Issue tags: +beta target, +rc deadline
alexpott’s picture

@greg.1.anderson I think it's good to leave it in. That way we can be sure we don't break it. My comment was to compliment the idea and to remark on functionality that would be useful but not in-scope here.

tedbow’s picture

Status: Needs work » Needs review
FileSize
9.74 KB

Ok here is reroll for 9.0.x

tedbow’s picture

Issue tags: -Needs reroll
tedbow’s picture

couple nit fixes and a question

  1. +++ b/composer/Plugin/Scaffold/AllowedPackages.php
    @@ -96,8 +96,15 @@ public function getAllowedPackages() {
    +    $package = $operationType == 'update' ? $operation->getTargetPackage() : $operation->getPackage();
    

    Should use ===

  2. +++ b/composer/Plugin/Scaffold/AllowedPackages.php
    @@ -176,6 +183,24 @@ protected function evaluateNewPackages(array $allowed_packages) {
    +   * @param \Composer\DependencyResolver\Operation\OperationInterface $operation
    +   * @return string
    

    Both need comments

  3. +++ b/composer/Plugin/Scaffold/AllowedPackages.php
    @@ -176,6 +183,24 @@ protected function evaluateNewPackages(array $allowed_packages) {
    +  protected function getOperationType($operation) {
    

    We should type hint OperationInterface here.

  4. +++ b/composer/Plugin/Scaffold/AllowedPackages.php
    @@ -176,6 +183,24 @@ protected function evaluateNewPackages(array $allowed_packages) {
    +    if (method_exists($operation, 'getJobType')) {
    +      return $operation->getJobType();
    +    }
    +    if (method_exists($operation, 'getOperationType')) {
    +      return $operation->getOperationType();
    +    }
    +    return '';
    

    Won't the $operation always have 1 or the other of the methods? Just depends on the version of composer? Shouldn't we throw exception instead of returning ''?

jungle’s picture

Replaced strstr() with strpos()

-    if (strstr($composerVersionLine, 'Composer version 2') !== FALSE) {
+    if (strpos($composerVersionLine, 'Composer version 2') !== FALSE) {
Note:

If you only want to determine if a particular needle occurs within haystack, use the faster and less memory intensive function strpos() instead.

-- @see https://www.php.net/manual/en/function.strstr.php

jungle’s picture

BTW. filed a new issue to replace strstr() with strpos()

greg.1.anderson’s picture

+1 on 1. through 4. in #49.

tedbow’s picture

Chatted with @greg.1.anderson. This does #49.4

I did the other things in the patch in #49

xjm’s picture

Thanks @tedbow and @greg.1.anderson. Having Composer 2 compatibility is very important for Drupal's future.

A couple questions:

  • Will the change in AllowedPackages affect anything aside from enabling Composer 2 packages to be installed?
  • Are there any edgecase packages/scripts/setups/etc. that might be affected by the fixes here?
greg.1.anderson’s picture

1. The change in AllowedPackages allows Drupal core to work when composer/composer:^2 is required for Core development, while also allowing composer/composer:^1 to continue working.

2. The behavior of AllowedPackages should remain completely unchanged regardless of the version of Composer being used; there shouldn't be any edge cases or side effects from that code.

I did quite a bit of ad-hoc testing and tried to make the tests cover the important behaviors, but it's always possible that something unexpected could come up. I'm pretty confident with what we have here, though.

I am +1 on #53, but can't RTBC. I'll try to find time to write up the change record later this afternoon.

greg.1.anderson’s picture

Issue summary: View changes

I created a change record for this issue, and updated the release notes snippet to match the wording used in the CR.

greg.1.anderson’s picture

Issue summary: View changes
kim.pepper’s picture

+++ b/composer/Plugin/Scaffold/AllowedPackages.php
@@ -176,6 +181,27 @@ protected function evaluateNewPackages(array $allowed_packages) {
+    throw new \RuntimeException('Object does not implemented expected method in ' . OperationInterface::class . '.');

Ubernit: 'implemented' should be 'implement'.

greg.1.anderson’s picture

FIx for #58.

alexpott’s picture

Status: Needs review » Needs work
+++ b/composer/Plugin/Scaffold/AllowedPackages.php
@@ -176,6 +181,27 @@ protected function evaluateNewPackages(array $allowed_packages) {
+  protected function getOperationType(OperationInterface $operation) {
+    if (method_exists($operation, 'getJobType')) {
+      return $operation->getJobType();
+    }
+    if (method_exists($operation, 'getOperationType')) {
+      return $operation->getOperationType();
+    }
+    throw new \RuntimeException('Object does not implement expected method in ' . OperationInterface::class . '.');
+  }

It would nice to comment which one is composer 1 and which is composer 2. It looks like we're doing Composer 1 call first. Which probably makes sense now but won't once everyone has Composer 2.

Also I think triggering a specific exception here is unnecessary - PHP will thrown an exception if the method doesn't exist and it's not something we're going to catch and do something with. So something like:

  protected function getOperationType(OperationInterface $operation) {
    // Use Composer 2 method.
    if (method_exists($operation, 'getOperationType')) {
      return $operation->getOperationType();
    }
    // Fallback to Composer 1 method.
    return $operation->getJobType();
  }

Other than that this is looking good to go to me for 9.x to me.

I also think we should consider rolling this patch for 8.9.x so we have a Drupal 8 compatible with COmposer 2 too.

jungle’s picture

jungle’s picture

A patch for 8.9.x,

Followed similar steps here (#3122112) making sure the package got updated against config.platform.php: 7.0.8.

The last submitted patch, 61: 3126566-61.patch, failed testing. View results

jungle’s picture

-    "composer-plugin-api": "^1.1"
+    "composer-plugin-api": "^1.1 || ^2.0.0"

A patch for 8.9.x again, updating the composer-plugin-api version to see if the testing passes.

For 9.0.x testing failed in #61, probably, it's a random failure.

Kristen Pol’s picture

Assigning to myself to review.

Kristen Pol’s picture

Assigned: Unassigned » Kristen Pol
Kristen Pol’s picture

Assigned: Kristen Pol » Unassigned
Status: Needs review » Needs work

Thanks for the patches.

1) Compared D8 vs D9 patches and not sure why these are included in D8 patch. They are not used anywhere in the codebase.

+++ b/composer.lock
@@ -117,6 +120,7 @@
+                "MantisBT",

https://github.com/mantisbt/mantisbt

+++ b/composer.lock
@@ -171,6 +175,7 @@
+                "sylius",

https://github.com/Sylius/Sylius

2) Patches applied cleanly

[mac:kristen:drupal-8.9.x-dev]$ patch -p1 < 3126566-64.patch
patching file composer.json
patching file composer.lock
patching file composer/Metapackage/CoreRecommended/composer.json
patching file composer/Plugin/ProjectMessage/MessagePlugin.php
patching file composer/Plugin/ProjectMessage/composer.json
patching file composer/Plugin/Scaffold/AllowedPackages.php
patching file composer/Plugin/Scaffold/Plugin.php
patching file composer/Plugin/Scaffold/composer.json
patching file composer/Plugin/VendorHardening/VendorHardeningPlugin.php
patching file composer/Plugin/VendorHardening/composer.json
patching file core/drupalci.yml
patching file core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
patching file core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Functional/ScaffoldUpgradeTest.php
[mac:kristen:drupal-9.0.x-dev]$ patch -p1 < 3126566-61.patch
patching file composer.json
patching file composer.lock
patching file composer/Plugin/ProjectMessage/MessagePlugin.php
patching file composer/Plugin/ProjectMessage/composer.json
patching file composer/Plugin/Scaffold/AllowedPackages.php
patching file composer/Plugin/Scaffold/Plugin.php
patching file composer/Plugin/Scaffold/composer.json
patching file composer/Plugin/VendorHardening/VendorHardeningPlugin.php
patching file composer/Plugin/VendorHardening/composer.json
patching file composer/Template/LegacyProject/composer.json
patching file composer/Template/RecommendedProject/composer.json
patching file core/drupalci.yml
Hunk #1 succeeded at 50 (offset -5 lines).
patching file core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
patching file core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Functional/ScaffoldUpgradeTest.php
[mac:kristen:drupal-9.1.x-dev]$ patch -p1 < 3126566-61.patch
patching file composer.json
patching file composer.lock
patching file composer/Plugin/ProjectMessage/MessagePlugin.php
patching file composer/Plugin/ProjectMessage/composer.json
patching file composer/Plugin/Scaffold/AllowedPackages.php
patching file composer/Plugin/Scaffold/Plugin.php
patching file composer/Plugin/Scaffold/composer.json
patching file composer/Plugin/VendorHardening/VendorHardeningPlugin.php
patching file composer/Plugin/VendorHardening/composer.json
patching file composer/Template/LegacyProject/composer.json
patching file composer/Template/RecommendedProject/composer.json
patching file core/drupalci.yml
Hunk #1 succeeded at 50 (offset -5 lines).
patching file core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
patching file core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Functional/ScaffoldUpgradeTest.php

3) Reviewed the changes requested in #60 are in the patches and the changes are identical.

4) I see that @greg.1.anderson has some commands that were manually tested but I'm not sure the best way to test this myself.

Kristen Pol’s picture

I noticed another thing when checking the code for composer/installers, composer/composer, and composer-plugin-api after applying the patches:

drupal-8.9.x-dev

composer/installers

  • ./composer/Metapackage/CoreRecommended/composer.json: "composer/installers": "v1.9.0",
  • ./composer/Plugin/Scaffold/README.md: "composer/installers": "^1.2",
  • ./composer/Template/RecommendedProject/composer.json: "composer/installers": "^1.2",
  • ./composer/Template/LegacyProject/composer.json: "composer/installers": "^1.2",
  • ./composer.json: "composer/installers": "^1.9",

composer/composer

  • ./composer/Metapackage/DevDependencies/composer.json: "composer/composer": "^1.9.1",
  • ./composer/Metapackage/PinnedDevDependencies/composer.json: "composer/composer": "1.9.1",
  • ./composer/Plugin/Scaffold/composer.json: "composer/composer": "^1.8@stable"
  • ./composer.json: "composer/composer": "^1.9.1",

composer-plugin-api

  • ./composer/Plugin/VendorHardening/composer.json: "composer-plugin-api": "^1.1 || ^2.0.0"
  • ./composer/Plugin/ProjectMessage/composer.json: "composer-plugin-api": "^1.1 || ^2.0.0"
  • ./composer/Plugin/Scaffold/composer.json: "composer-plugin-api": "^1.0.0 || ^2.0.0",

drupal-9.0.x-dev and drupal-9.1.x-dev

composer/installers

  • ./composer/Metapackage/CoreRecommended/composer.json: "composer/installers": "v1.9.0",
  • ./composer/Plugin/Scaffold/README.md: "composer/installers": "^1.2",
  • ./composer/Template/RecommendedProject/composer.json: "composer/installers": "^1.9",
  • ./composer/Template/LegacyProject/composer.json: "composer/installers": "^1.9",
  • ./composer.json: "composer/installers": "^1.9",

composer/composer

  • ./composer/Metapackage/DevDependencies/composer.json: "composer/composer": "^1.9.1",
  • ./composer/Metapackage/PinnedDevDependencies/composer.json: "composer/composer": "1.10.5",
  • ./composer/Plugin/Scaffold/composer.json: "composer/composer": "^1.8@stable"
  • ./composer.json: "composer/composer": "^1.9.1",

composer-plugin-api

  • ./composer/Plugin/VendorHardening/composer.json: "composer-plugin-api": "^1.1 || ^2"
  • ./composer/Plugin/ProjectMessage/composer.json: "composer-plugin-api": "^1.1 || ^2"
  • ./composer/Plugin/Scaffold/composer.json: "composer-plugin-api": "^1 || ^2",

Do any of the dissimilar versions need to be changed?

greg.1.anderson’s picture

Thanks for the review, @Kristen Pol.

#67.1: Not sure why these are here. I'll look into it later. composer why will shed some insight.

#67.4: The tests should cover functionality pretty well, but if others want to do more ad-hoc testing, that certainly wouldn't hurt.

#68.1: composer/installers: Should probably be ^1.9 everywhere, for the avoidance of doubt.

#68.2: composer-plugin-api: There's no need to bump the minimum version of the plugin api to 1.1 where it was previously 1.0. Doing so wouldn't hurt, though.

#68.3: Not sure why PinnedDevDependencies didn't get regenerated. Maybe composer update --lock was omitted for this patch? I wouldn't think so. I'll need to investigate more late.

#68.4: It would be consistent to make the Scaffold plugin require composer/composer ^1.9 instead of ^1.8@stable, but this is not necessary.

I appreciate all the cross-checking you did.

jungle’s picture

Status: Needs work » Needs review

@Kristen Pol, many thanks for your detailed review, always!

#67.1, nothing to do to me. sylius and MantisBT exist in composer.lock of both two patches.

See vendor/composer/installers/src/Composer/Installers/Installer.php

'drupal'       => 'DrupalInstaller',
...
'sylius'       => 'SyliusInstaller',
'MantisBT' =>'MantisBTInstaller',

BTW. Composer version 1.9.0 2019-08-02 20:55:32 used to reroll the patch for 8.9.x

#68, to me, it's acceptable to have different versions in different branches. The min PHP version required in 8.x is 7.0.8, while it's 7.3.0 in 9.1.x. So I am setting back to NR.

greg.1.anderson’s picture

#68.3, the versions in PinnedDevDependencies, are cause to put this back to needs work if they are in fact inconsistent with what is in composer.lock.

Not putting this back to needs work myself just yet because I don't have a minute to verify that adjustment is needed. Running COMPOSER_ROOT_VERSION=8.9.x-dev composer update --lock is probably sufficient to fix this problem, if it exists.

hussainweb’s picture

In the D9 patch in #61, I noticed the composer.lock was not updated for projects in the path repositories. I updated it but my composer version added a bunch of additional values.

These were the changes I was looking for:

 ~/w/d/drupal   9.0.x ~  rg composer-plugin-api
composer.lock
76:                "composer-plugin-api": "^1.0 || ^2.0"
711:                "composer-plugin-api": "^1.1",
741:                "composer-plugin-api": "^1.1",

composer/Plugin/VendorHardening/composer.json
18:    "composer-plugin-api": "^1.1 || ^2"

composer/Plugin/ProjectMessage/composer.json
18:    "composer-plugin-api": "^1.1 || ^2"

composer/Plugin/Scaffold/composer.json
9:    "composer-plugin-api": "^1 || ^2",

Particularly, lines 711 and 741 were not updated in composer.lock. They were in sections for `drupal/core-project-message` and `drupal/core-vendor-hardening`. They are now updated in the new patch.

hussainweb’s picture

From #68, @Kristen Pol, I think that none of those versions contradicts each other and that's fine. (If I'm understanding your comment correctly.)

I don't think that reflects what's in composer.lock. It's still worth making the versions consistent but maybe in a separate issue?

hussainweb’s picture

I was doing more digging related to #68 and I thought I should clarify that I am only commenting on 9.0.x branch at the moment. Also, one of the versions for composer/installers is ^1.2, but that's in the README.md file. It makes sense to update it in this issue itself.

hussainweb’s picture

Oh, one more thing. Re #71, @greg.1.anderson, you're right that the version mismatch in PinnedDevDependencies would be cause to set to "needs work", but PinnedDevDependencies is not in the patch (for 9.0.x at least). It's already set to 1.10.5 in HEAD of the branch.

Sorry for the repeat posts. I think this is all for now. :)

Kristen Pol’s picture

Thanks. I see a lot of changes in the lock file so I'm going to hold off reviewing at least until @greg.1.anderson weighs in more on this.

Status: Needs review » Needs work

The last submitted patch, 72: 3126566-72.patch, failed testing. View results

hussainweb’s picture

This is the failure.

1) Drupal\Tests\ComposerIntegrationTest::testComposerLockHash
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'e3d324da27c1060f5f9982ee20aa7f9845303fd2'
+'5bd6798a64831fa08a343a14a0ee47127c4cb99f'

I am not sure what to think of this because the actual hash in composer.lock is `bf3f5b1c0d2f35255e03825589a1ee4f`. Also, running `composer validate` locally reports no errors.

greg.1.anderson’s picture

#78: @hussainweb: Downgrade your version of Composer to 1.9.x before running composer update on Drupal core. I think there's an issue for this somewhere.

hussainweb’s picture

Thanks @greg.1.anderson. I'll try that. Although, composer validate on 1.9.0 works fine.

jungle’s picture

Thanks @hussainweb!

Re #78

+++ b/composer.lock
@@ -183,6 +183,16 @@
+            "funding": [
+                {
+                    "url": "https://packagist.com",

@@ -1306,6 +1325,12 @@
+            "funding": [

Funding info were added, which introduced in composer 1.10. So you need to fallback to composer 1.9 to avoid adding funding info.

See #3127918: Add funding info in composer.lock for more

jungle’s picture

In 8.9.x patch, I updated the composer-plugin-api versions to allow coexistence with composer 2 manually, which should be automatically generated probably.

hussainweb’s picture

Status: Needs work » Needs review
FileSize
23.45 KB
11 KB

Thanks @jungle, I ran this now on composer 1.9.0 and the hash does look different. Here it goes.

hussainweb’s picture

#82, @jungle: I am not sure which comment you're responding to but if you're talking about the first patch I posted here, that was for 9.0.x. I haven't seen the 8.9.x patch yet. Am I missing anything here?

jungle’s picture

Re @hussainweb, #82 is just FYI :)

greg.1.anderson’s picture

OK, I looked at this a bit more closely.

./composer/Metapackage/DevDependencies/composer.json: "composer/composer": "^1.9.1",
./composer/Metapackage/PinnedDevDependencies/composer.json: "composer/composer": "1.10.5",

The dev dependencies copies whatever is in Drupal's composer.json file, recording the current constraint for each dependency. The version constraint for composer/composer in the 9.0.x branch is currently ^1.9.1, and this patch makes no changes to this value, so this is correct.

The pinned dev dependencies records the exact version of each dependency locked in Drupal's composer.lock file. Note that it is expected and common for this version to be later than the version constraint in the composer.lock file. Drupal has Composer 1.10.5 in the composer.lock file, and this patch does not change anything in this area, so this is also correct.

I also tried running COMPOSER_ROOT_VERSION=9.0.x-dev composer update --lock on #61, and found that no changes were made to the composer.lock file, so #72 and #83 are not necessary.

As previously mentioned, #67.1 is part of the update to composer/installers done in this patch. Composer copies part of every dependency's composer.json file into the composer.lock file of any application that uses it. Sylus and MantisDB are mentioned in the "keywords" of the new version of composer/installers.

The optional version consistency changes could be made as noted in #69, if desired, or we can just continue with #83 / #61 (these should be the same, I think) and #64 (for 8.9.x).

hussainweb’s picture

Thanks @greg.1.anderson, I am not sure if I understand. However, I still think that the changes in #72 (and hence #83) are necessary. IIRC, I got the changes just by running `composer install`. I'm trying to check my history to see if I ran something else but after that, the lock file was updated with the correct constraints for composer-plugin-api under `drupal/core-project-message` and `drupal/core-vendor-hardening`.

Kristen Pol’s picture

@hussainweb It would be easier to understand the changes in #83 if the interdiff was against #61. It's hard to read with all the funding stuff.

hussainweb’s picture

FileSize
1.47 KB

@Kristen Pol, I reran the command I used earlier but now on composer 1.9 and got this diff. As you can see, it updates the locked version references for composer-plugin-api.

This is the command: docker run --rm -v {$PWD}:/app -w /app composer:1.9.0 require composer/installers:^1.9 --ignore-platform-reqs

It's functionally same as composer require composer/installers:^1.9 if you have composer 1.9.0 installed locally.

Kristen Pol’s picture

Thanks @hussainweb. That's much easier to understand.

alexpott’s picture

Status: Needs review » Reviewed & tested by the community

Here's the diff between #61 and #82

15c15
< index 3dc5459a1c..645114adb5 100644
---
> index 3dc5459a1c..f18a746b44 100644
26a27,52
> @@ -705,10 +705,10 @@
>              "dist": {
>                  "type": "path",
>                  "url": "composer/Plugin/ProjectMessage",
> -                "reference": "0691ca6beba657e6da57a893b1c6da757d16afc8"
> +                "reference": "b4efdbe26634b41a1b89e4f3770a8074769088a6"
>              },
>              "require": {
> -                "composer-plugin-api": "^1.1",
> +                "composer-plugin-api": "^1.1 || ^2",
>                  "php": ">=7.3.0"
>              },
>              "type": "composer-plugin",
> @@ -735,10 +735,10 @@
>              "dist": {
>                  "type": "path",
>                  "url": "composer/Plugin/VendorHardening",
> -                "reference": "0b015340af38f90df46923a934d0b22026134f18"
> +                "reference": "d54f0b3cc8b4237f3a41a0860a808db242f9da9e"
>              },
>              "require": {
> -                "composer-plugin-api": "^1.1",
> +                "composer-plugin-api": "^1.1 || ^2",
>                  "php": ">=7.3.0"
>              },
>              "type": "composer-plugin",
214c240
< index 8cc5f1d488..8fb2b41290 100644
---
> index d6c5787ffc..63df934307 100644
217c243
< @@ -55,3 +55,9 @@ build:
---
> @@ -50,3 +50,9 @@ build:

That change looks acceptable to me. Note that reviewing the composer.lock is pretty fraught. In an ideal world we wouldn't ship it but that would require us to min/max dependency test and involve all sorts of complexities around releases so that's unlikely to happen in any sort of speed.

I've taken the latest patch #83 and run composer update --lock with composer 1.9.2 and no changes are made so the lock file is great. More importantly all the code changes are good and we are testing core's composer plugins and meta packages using composer 2 so that's great.

The Drupal 9 patch is therefore rtbc.

Doing the same test on the Drupal 8 patch results in changes to the lock file so #64 is not rtbc but we can fix that once this has landed in Drupal 9.

  • catch committed 6c767cd on 9.0.x
    Issue #3126566 by greg.1.anderson, jungle, tedbow, hussainweb, Kristen...
catch’s picture

Version: 9.0.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Needs work
+++ b/core/drupalci.yml
@@ -50,3 +50,9 @@ build:
       nightwatchjs:
+      # Re-run Composer plugin tests after installing Composer 2
+      container_command.composer-upgrade:
+        commands:
+          - "sudo composer self-update --snapshot"
+          - "./vendor/bin/phpunit -c core --group VendorHardening,ProjectMessage,Scaffold"
+        halt-on-fail: true
diff --git a/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php b/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php

Bit worried this might lead to testbot instability prior to composer 2 becoming stable. However we can address that if it happens (i.e. a new issue to remove this hunk and put it into a tracking patch or similar). OK with adding it for now.

Committed/pushed to 9.1.x and cherry-picked to 9.0.x.

Moving back to 8.9.x for backport. I think we want to update our composer plugins so they're composer 2 compatible, but keep 8.9's composer dependencies as they are.

  • catch committed 349ae78 on 9.1.x
    Issue #3126566 by greg.1.anderson, jungle, tedbow, hussainweb, Kristen...
greg.1.anderson’s picture

Thanks, all. #89 showed that the composer.lock changes described in #87 were necessary.

Moving back to 8.9.x for backport. I think we want to update our composer plugins so they're composer 2 compatible, but keep 8.9's composer dependencies as they are.

This seems reasonable. If we do this, then when using Drupal 8.9.x, it would be possible to use Composer 2 with drupal/recommended-project, but not with drupal/pinned-dev-dependencies, and not with Drupal core development. Using Composer 2 with Drupal core is dependent upon #3128631: Update dependencies composer/composer ^2 and composer/semver to ^3 anyway, though, so this is fine.

greg.1.anderson’s picture

Status: Needs work » Needs review
FileSize
5.16 KB
9.54 KB

Here's a backport to 8.9.x. This is essentially #64 with the composer/installers update omitted, and other composer.lock changes applied.

greg.1.anderson’s picture

Bit worried this might lead to testbot instability prior to composer 2 becoming stable.

Yeah, it is not great to have the testbot running with every dev commit of Composer 2 that gets pushed. I originally included this just to demonstrate that it works, but it is also good to have some assurance that future changes to our plugins do not break under Composer 2.

I'll make a follow-on issue to at least pin to a specific version of Composer 2.

greg.1.anderson’s picture

greg.1.anderson’s picture

Should we remove the Composer 2 re-test from #96? Anything committed to 8.9.x should at this point have already been tested on the 9.0.x branch, which includes Composer 2 testing. Perhaps maintaining this test on the Composer dev release is not necessary here. It would be less to track in #3133423: Test with a specific dev release of Composer 2 to omit it here.

alexpott’s picture

#99 sounds okay. We might choose to add composer 2 testing to 8.9.x once composer 2 is stable but I agree that having less to tracking is nice.

greg.1.anderson’s picture

Updating #96 to eliminate Composer 2 testing per #100

longwave’s picture

Status: Needs review » Needs work

#82 has this:

-    "composer-plugin-api": "^1.1"
+    "composer-plugin-api": "^1.1 || ^2"

#101 has this:

-    "composer-plugin-api": "^1.1"
+    "composer-plugin-api": "^1.1 || ^2.0.0"

I think we should be consistent, I guess by using ^2 in the 8.9.x patch.

greg.1.anderson’s picture

Status: Needs work » Needs review
FileSize
2.74 KB
8.6 KB

OK, ^2 and ^2.0.0 are semantically identical, but in the interest of minimizing diffs between the 8.9.x and 9.0.x branches, here's an update to the patch.

longwave’s picture

Status: Needs review » Reviewed & tested by the community

Diff between the 8.9.x and 9.0.x patches is as small as can be (including the change from #99), so this is good to go.

  • catch committed 53e445b on 8.9.x
    Issue #3126566 by greg.1.anderson, jungle, tedbow, hussainweb, Kristen...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed 53e445b and pushed to 8.9.x. Thanks!

  • catch committed e8dfaa7 on 8.9.x
    Revert "Issue #3126566 by greg.1.anderson, jungle, tedbow, hussainweb,...
catch’s picture

Status: Fixed » Needs work

Reverting this due to #3134648: [backport, needs scheduling] Don't pin the composer/installers version in drupal/core-recommended. From discussion on that issue it looks like we could recommit this with a pinned dependency on composer-installers 1.7 to workaround the composer update bug.

hussainweb’s picture

I might be missing something here. The patch for 8.9 didn't change the version at all. It happened in #3122112: Update dependencies for Drupal 8.9. Is there a need to revert this patch as well?

alexpott’s picture

One option here is to recommit #103 as it does not upgrade composer/installers and then handle the composer/installers update in #3134648: [backport, needs scheduling] Don't pin the composer/installers version in drupal/core-recommended

I've kicked off a test run.

alexpott’s picture

Doing #110 would mean that drupal/core-recommended is not composer2 compatible for now. But as @greg.1.anderson has pointed out you can

switch to drupal/core and upgrade your composer/installers.

catch’s picture

@hussainweb yeah I got a bit confused this morning and was over-zealous. I think we can probably land #3122112: Update dependencies for Drupal 8.9 again, then re-commit #103 as-is.

greg.1.anderson’s picture

Status: Needs review » Reviewed & tested by the community

You're right, it looks like we decided not to upgrade dependencies for the 8.9.x version of this patch. Yay.

+1 on re-committing #103.

xjm’s picture

Status: Reviewed & tested by the community » Postponed

This probably needs to be postponed on that, because otherwise we're back in the same situation?

greg.1.anderson’s picture

We believe that this patch will have no effect on our erstwhile Composer bug; however, it's hard to prove until we commit to 8.9.x-dev, at which point we've already inconvenienced a bunch of people if we get it wrong.

Postponing this on #3134648: [backport, needs scheduling] Don't pin the composer/installers version in drupal/core-recommended is therefore safer, and couldn't hurt.

alexpott’s picture

Status: Postponed » Reviewed & tested by the community

@greg.1.anderson I'm not sure that you're right about #3134648: [backport, needs scheduling] Don't pin the composer/installers version in drupal/core-recommended being safer. This doesn't touch the drupal/core-recommended meta-package so I think it is totally safe to commit. The other one is much more tricky. In fact given the timings I think are necessary I think we need to commit this to 8.9.x sooner rather than later.

Also for what it is worth we know this didn't cause the break for composer update on 8.8 because when catch revert this it was not fixed. It took the revert of #3122112: Update dependencies for Drupal 8.9 to fix the problem because that's the commit that made the 8.9.x drupal/core-recommended meta-package compatible with composer/installers 1.9.0 - and thus reverting it stopped composer update on 8.8 oddly selecting to update to 8.9.x-dev.

catch’s picture

Status: Reviewed & tested by the community » Fixed

So for clarity, this issue was fine and not responsible for breaking HEAD, it just went in at the same time as #3122112: Update dependencies for Drupal 8.9, which did break HEAD. I reverted it out of caution. Have now undone the revert since the general dependency update is fixed.

  • catch committed e66a9e0 on 8.9.x
    Issue #3126566 by greg.1.anderson, jungle, tedbow, hussainweb, Kristen...
greg.1.anderson’s picture

Confirmed that Composer is not upgrading prefer-stable sites to 8.9.x-dev this morning.

xjm’s picture

xjm’s picture

Issue summary: View changes

Status: Fixed » Closed (fixed)

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

rgpublic’s picture

Umm, is this really fixed? I'm using Drupal 9.0.2. When I run "composer update" with Composer 2 I get a fatal error in Composer::vendorTestCodeCleanup:

Fatal error: Uncaught Error: Call to undefined method Composer\DependencyResolver\Operation\UpdateOperation::getJobType() in /var/www/projects/aufsichtsrat-handbuch.de/htdocs/core/lib/Drupal/Core/Composer/Composer.php:189

I open that file and see in line 189:

if ($op->getJobType() == 'update')

But in the description of this issue it says:

Call OperationInterface:: getOperationType() instead of OperationInterface::getJobType() (renamed in Compsoer 2)

Now, I'm confused. Is this fixed? Am I doing sth. wrong?

rgpublic’s picture

Ah. Sorry for the spam. Just as I wrote this, I discovered the problem. The "scripts" section of my composer.json file causes these methods to be run. I removed the scripts section altogether as I doesn't seem to be in a fresh installation of drupal/legacy-project. Perhaps this useful for others and IMHO certainly sth. to be aware of. If you have older projects, you might want to check your composer.json script section if you still encounter problem with Composer 2.

maxilein’s picture

#124 helped! Thanks