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.2: selective update breaks on changed requirement #10394

Closed
jrfnl opened this issue Dec 23, 2021 · 12 comments · Fixed by #10405
Closed

Composer 2.2: selective update breaks on changed requirement #10394

jrfnl opened this issue Dec 23, 2021 · 12 comments · Fixed by #10405
Labels
Milestone

Comments

@jrfnl
Copy link
Contributor

jrfnl commented Dec 23, 2021

Summary

There's a change in behaviour between Composer 2.1 and 2.2, where dependencies which were previously resolved without problem, now no longer get resolved.

Basic premise is a repo which has a committed composer.lock file and uses on-the-fly updates in the CI runs via GH Actions.
These updates used to work fine, but are now broken and failing builds.

I've set up a reproduction sample here: https://github.com/jrfnl/composer-2.2-bug-poc

And the changed behaviour and full -vvv output can be seen in the GH Actions output: https://github.com/jrfnl/composer-2.2-bug-poc/actions/runs/1616471281

My composer.json:

{
  "name": "jrfnl/composer-2.2-bug-poc",
  "description": "Proof of concept",
  "license": "GPL-2.0-or-later",
  "require": {
    "php": ">=5.6"
  },
  "require-dev": {
    "yoast/wp-test-utils": "^1.0.0"
  },
  "config": {
    "platform": {
      "php": "5.6.40"
    }
  }
}

When I run this command on PHP 8.0:

composer require --dev phpunit/phpunit:"^7.5" --no-update --ignore-platform-req=php
composer update yoast/wp-test-utils --with-dependencies --ignore-platform-req=php

I get the following output:

./composer.json has been updated
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - phpunit/phpunit[7.5.0, ..., 7.5.20] require sebastian/exporter ^3.1 -> found sebastian/exporter[3.1.0, ..., 3.1.4] but these were not loaded, likely because it conflicts with another require.
    - Root composer.json requires phpunit/phpunit ^7.5 -> satisfiable by phpunit/phpunit[7.5.0, ..., 7.5.20].

Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions.

And I expected this to happen:

The update to succeed without problems.

Other notes:

Additional things I've tried:

  • --with-dependencies vs --with-all-dependencies does not make a difference.
  • Specifying the versions in the update command didn't help.
  • Using --with ... in the update command doesn't help.
  • Using --no-install instead of --no-update in the first command doesn't help.

Work-around which works

... but may not play nice with caching in GH Actions as much:

composer remove --dev yoast/wp-test-utils
composer require --dev yoast/wp-test-utils:"^1.0.0" phpunit/phpunit:"^7.5" --update-with-dependencies --ignore-platform-reqs
@herndlm
Copy link
Contributor

herndlm commented Dec 23, 2021

Hmm I had to use -W to reproduce this because of the lockfile, but maybe I messed something up. I was in a hurry. Anyways, bisect pointed me to

[8c8d9ef] Filter impossible packages from the pool (#9620)

That might be the place to look for.

@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 23, 2021

@herndlm Nice detective work! So, I guess that change is filtering out too many "(not) impossible" packages now...

jrfnl added a commit to jdevalk/clicky that referenced this issue Dec 23, 2021
Composer 2.2 contains a a bug/over-optimization, which causes the install with changed, but compatible requirements to error out on "conflicting" requirements.

This commit puts a work-around for this in place, which still tries to use the caching for the Composer install as much as possible.

It is recommended to revert this commit if/when the Composer bug has been fixed and a new version has been released.

Ref:
* Bug report: composer/composer#10394
* Probable cause: composer/composer#9620
* Similar fix in YoastSEO: Yoast/wordpress-seo@b399942
@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 25, 2021

Just ran into another similar failure. The reproduction details are different, but I have the feeling it will have the same root cause:
https://github.com/sebastianbergmann/exporter/runs/4633739667?check_suite_focus=true

In this case, it's a package with a recursive dependency on itself, which was previously handled correctly, but isn't anymore.

@Seldaek Seldaek added this to the 2.2 milestone Dec 28, 2021
@Seldaek Seldaek added the Bug label Dec 28, 2021
@Seldaek
Copy link
Member

Seldaek commented Dec 28, 2021

OK the reason the packages gets deleted is that PHPUnit 5.x requires phpunit/phpunit-mock-objects, which has a require on sebastian/exporter ^1.2 || ^2. So we have that installed.. And updating phpunit/phpunit to 7.x (by way of yoast/wp-test-utils -W) ends up removing the phpunit/phpunit-mock-objects requirement entirely, so that package will be removed once resolved. But as it's not required anymore, it is not unlocked. And as it is locked, the optimizer assumes its requirements must still apply, so sebastian/exporter 3.x versions are filtered away as they don't match ^1.2 || ^2.

I am not sure how to best fix this.. maybe we can only apply the optimization to packages which are locked and still required by locked packages or the root package.. Or maybe we just need to remove this optimization pass entirely.

cc @driskell FYI

@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 28, 2021

Thanks for looking into this @Seldaek !

I am not sure how to best fix this.. maybe we can only apply the optimization to packages which are locked and still required by locked packages or the root package.. Or maybe we just need to remove this optimization pass entirely.

Or maybe only apply the optimization when the lock file is in sync with the json file ? (just spitballing, not sure if this helps)

@driskell
Copy link
Contributor

driskell commented Dec 29, 2021

I am not sure how to best fix this.. maybe we can only apply the optimization to packages which are locked and still required by locked packages or the root package.. Or maybe we just need to remove this optimization pass entirely.

cc @driskell FYI

I think ignoring a locked package that isn't required by anything in the pool would resolve it. If it were required by anything in the pool it would, by the transitive allow, get unlocked. Just writing up a test and will raise a potential PR shortly. Will test it with this exact case too if I can.

I think that's the only missed scenario - locked packages MAY actually get removed by an uninstall - if there's another scenario for locked package that needs to be taken into account let me know but racking my brain for a bit I can't think of another case where a locked package wouldn't end up still being locked after solver.

driskell added a commit to driskell/composer that referenced this issue Dec 29, 2021
driskell added a commit to driskell/composer that referenced this issue Dec 29, 2021
driskell added a commit to driskell/composer that referenced this issue Dec 29, 2021
driskell added a commit to driskell/composer that referenced this issue Dec 29, 2021
driskell added a commit to driskell/composer that referenced this issue Dec 29, 2021
driskell added a commit to driskell/composer that referenced this issue Dec 29, 2021
driskell added a commit to driskell/composer that referenced this issue Dec 29, 2021
driskell added a commit to driskell/composer that referenced this issue Dec 29, 2021
@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 29, 2021

Thanks @driskell for looking into this. I'll keep an eye out for more situations which could be related to this.

Seldaek added a commit that referenced this issue Dec 31, 2021
…ackage that will be uninstalled (#10405)

Fixes #10394

Co-authored-by: Jordi Boggiano <j.boggiano@seld.be>
jrfnl added a commit to jdevalk/clicky that referenced this issue Jan 3, 2022
This reverts commit 126ecf7.

Composer 2.2.3 has been released which contains a fix for the overly greedy optimization pass which was introduced in Composer 2.2.0.

With this fix in place, the previously introduced work-around for Composer 2.2 is no longer needed.

Refs:
* https://github.com/composer/composer/releases/tag/2.2.3
* composer/composer#10394
jrfnl added a commit to Yoast/wordpress-seo that referenced this issue Jan 3, 2022
This reverts commit b399942.

Composer 2.2.3 has been released which contains a fix for the overly greedy optimization pass which was introduced in Composer 2.2.0.

With this fix in place, the previously introduced work-around for Composer 2.2 is no longer needed.

Refs:
* https://github.com/composer/composer/releases/tag/2.2.3
* composer/composer#10394
jrfnl added a commit to Yoast/wordpress-seo that referenced this issue Jan 3, 2022
This reverts commit b399942.

Composer 2.2.3 has been released which contains a fix for the overly greedy optimization pass which was introduced in Composer 2.2.0.

With this fix in place, the previously introduced work-around for Composer 2.2 is no longer needed.

Refs:
* https://github.com/composer/composer/releases/tag/2.2.3
* composer/composer#10394
herregroen pushed a commit to Yoast-dist/wordpress-seo that referenced this issue Jan 3, 2022
This reverts commit b3999428193d5cdcdd0758dcb7dc060e6992c507.

Composer 2.2.3 has been released which contains a fix for the overly greedy optimization pass which was introduced in Composer 2.2.0.

With this fix in place, the previously introduced work-around for Composer 2.2 is no longer needed.

Refs:
* https://github.com/composer/composer/releases/tag/2.2.3
* composer/composer#10394
@hchonov
Copy link

hchonov commented Jan 3, 2022

I am still having issues when updating selectively with the flag --with-all-dependencies. I've tested with all the current composer 2.2.* versions 2.2.0, 2.2.1, 2.2.2 and 2.2.3 and all of them have the issue. Rolling back to 2.1.14 fixes the issue for me. Please consider re-opening the issue.

@Seldaek
Copy link
Member

Seldaek commented Jan 3, 2022

Please just open a new one with details if you want assistance. This issue here was definitely fixed, so you must be hitting another bug.

@hchonov
Copy link

hchonov commented Jan 4, 2022

I am having the very same bug as described in the issue here.

@hchonov
Copy link

hchonov commented Jan 4, 2022

I have a dependency x/y that requires Drupal ^8.9. After switching to a new branch that dependency starts requiring Drupal ^9.3. Running composer update x/y --with-all-dependencies leads to the error as described in the issue here. With composer 2.1.14 the dependenices are correctly resolved and the update succeeds and with 2.2.3 I get the following error:

Gathering patches for root package.
> DrupalProject\composer\ScriptHandler::checkComposerVersion
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - drupal/core-recommended 9.3.x-dev requires drupal/core 9.3.x-dev -> found drupal/core[9.3.x-dev] but these were not loaded, likely because it conflicts with another require.
    - drupal/core-recommended 9.4.x-dev requires drupal/core 9.4.x-dev -> found drupal/core[9.4.x-dev] but these were not loaded, likely because it conflicts with another require.
    - drupal/core-recommended 9.3.0-alpha1 requires drupal/core 9.3.0-alpha1 -> found drupal/core[9.3.0-alpha1] but these were not loaded, likely because it conflicts with another require.
    - drupal/core-recommended 9.3.0-beta1 requires drupal/core 9.3.0-beta1 -> found drupal/core[9.3.0-beta1] but these were not loaded, likely because it conflicts with another require.
    - drupal/core-recommended 9.3.0-beta2 requires drupal/core 9.3.0-beta2 -> found drupal/core[9.3.0-beta2] but these were not loaded, likely because it conflicts with another require.
    - drupal/core-recommended 9.3.0-beta3 requires drupal/core 9.3.0-beta3 -> found drupal/core[9.3.0-beta3] but these were not loaded, likely because it conflicts with another require.
    - drupal/core-recommended 9.3.0-rc1 requires drupal/core 9.3.0-rc1 -> found drupal/core[9.3.0-rc1] but these were not loaded, likely because it conflicts with another require.
    - drupal/core-recommended 9.3.0 requires drupal/core 9.3.0 -> found drupal/core[9.3.0] but these were not loaded, likely because it conflicts with another require.
    - x/y dev-feature/2753-d9 requires drupal/core-recommended ^9.3 -> satisfiable by drupal/core-recommended[9.3.0-alpha1, ..., 9.4.x-dev].
    - Root composer.json requires x/y dev-feature/2753-d9 -> satisfiable by x/y[dev-feature/2753-d9].

@Seldaek
Copy link
Member

Seldaek commented Jan 4, 2022

As I said, please create a new issue and include a composer.json + possibly composer.lock otherwise we cannot reproduce this.

@composer composer locked as resolved and limited conversation to collaborators Jan 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
5 participants