fix: Do not optimise away packages due to a requirement by a locked p…

…ackage that will be uninstalled (#10405) Fixes #10394 Co-authored-by: Jordi Boggiano <j.boggiano@seld.be>
composer · Dec 31, 2021 · efc7a3f · efc7a3f
1 parent 75e4d30
commit efc7a3f
Show file tree

Hide file tree

Showing 8 changed files with 362 additions and 0 deletions.
diff --git a/src/Composer/DependencyResolver/PoolOptimizer.php b/src/Composer/DependencyResolver/PoolOptimizer.php
@@ -418,6 +418,20 @@ private function optimizeImpossiblePackagesAway(Request $request, Pool $pool)
         }
 
         foreach ($request->getLockedPackages() as $package) {
+            // If this locked package is no longer required by root or anything in the pool, it may get uninstalled so do not apply its requirements
+            // In a case where a requirement WERE to appear in the pool by a package that would not be used, it would've been unlocked and so not filtered still
+            $isUnusedPackage = true;
+            foreach ($package->getNames(false) as $packageName) {
+                if (isset($this->requireConstraintsPerPackage[$packageName])) {
+                    $isUnusedPackage = false;
+                    break;
+                }
+            }
+
+            if ($isUnusedPackage) {
+                continue;
+            }
+
             foreach ($package->getRequires() as $link) {
                 $require = $link->getTarget();
                 if (!isset($packageIndex[$require])) {

diff --git a/...t/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-locked-replacer.test b/...t/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-locked-replacer.test
@@ -0,0 +1,60 @@
+--TEST--
+Do not load packages into the pool that cannot meet the fixed/locked requirements, when a loose requirement is encountered during update
+(The locked replacer/pkg should restrict dependencies even though it is only referenced by its replaced name)
+
+--REQUEST--
+{
+    "require": {
+        "first/pkg": "*",
+        "second/pkg": "*",
+        "replacer/dep": "*"
+    },
+    "locked": [
+        {"name": "first/pkg", "version": "1.0.0", "require": {"replaced/pkg": "1.0.0"}},
+        {"name": "second/pkg", "version": "1.0.0"},
+        {"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "1.0.0"}, "require": {"replacer/dep": "1.*"}},
+        {"name": "replacer/dep", "version": "1.0.0"}
+    ],
+    "allowList": [
+        "second/pkg",
+        "replacer/dep"
+    ],
+    "allowTransitiveDeps": true
+}
+
+--FIXED--
+[
+]
+
+--PACKAGE-REPOS--
+[
+    [
+        {"name": "first/pkg", "version": "1.0.0", "require": {"replaced/pkg": "1.0.0"}},
+        {"name": "first/pkg", "version": "1.1.0", "require": {"replaced/pkg": "2.0.0"}},
+        {"name": "second/pkg", "version": "1.0.0"},
+        {"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "1.0.0"}, "require": {"replacer/dep": "1.*"}},
+        {"name": "replacer/pkg", "version": "1.1.0", "replace": {"replaced/pkg": "2.0.0"}, "require": {"replacer/dep": "1.*"}},
+        {"name": "replacer/pkg", "version": "1.2.0", "replace": {"replaced/pkg": "3.0.0"}, "require": {"replacer/dep": "1.*"}},
+        {"name": "replacer/dep", "version": "1.0.0"},
+        {"name": "replacer/dep", "version": "1.0.1"},
+        {"name": "replacer/dep", "version": "2.0.0"}
+    ]
+]
+
+--EXPECT--
+[
+    "first/pkg-1.0.0.0 (locked)",
+    "replacer/pkg-1.0.0.0 (locked)",
+    "second/pkg-1.0.0.0",
+    "replacer/dep-1.0.0.0",
+    "replacer/dep-1.0.1.0",
+    "replacer/dep-2.0.0.0"
+]
+
+--EXPECT-OPTIMIZED--
+[
+    "first/pkg-1.0.0.0 (locked)",
+    "replacer/pkg-1.0.0.0 (locked)",
+    "second/pkg-1.0.0.0",
+    "replacer/dep-1.0.1.0"
+]
diff --git a/...dencyResolver/Fixtures/poolbuilder/filter-impossible-packages-only-required-provides.test b/...dencyResolver/Fixtures/poolbuilder/filter-impossible-packages-only-required-provides.test
@@ -0,0 +1,58 @@
+--TEST--
+When filtering packages from the pool that cannot meet the fixed/locked requirements, ensure that the requirements for a package that is not required anywhere is not used to filter, as it will be ultimately be removed.
+(Variant where the requirement is on a provided package, the locked third/pkg must not restrict the provider)
+(NOTE: We are not optimising this scenario currently)
+
+--REQUEST--
+{
+    "require": {
+        "first/pkg": "*",
+        "second/pkg": "1.1.0",
+        "provider/pkg": "*"
+    },
+    "locked": [
+        {"name": "first/pkg", "version": "1.0.0", "require": {"second/pkg": "^1.0"}},
+        {"name": "second/pkg", "version": "1.0.0", "require": {"third/pkg": "1.0.0"}},
+        {"name": "third/pkg", "version": "1.0.0", "require": {"provided/pkg": "1.0.0"}},
+        {"name": "provider/pkg", "version": "1.0.0", "provide": {"provided/pkg": "1.0.0"}}
+    ],
+    "allowList": [
+        "first/pkg",
+        "provider/pkg"
+    ],
+    "allowTransitiveDeps": true
+}
+
+--FIXED--
+[
+]
+
+--PACKAGE-REPOS--
+[
+    [
+        {"name": "first/pkg", "version": "1.0.0", "require": {"second/pkg": "*"}},
+        {"name": "second/pkg", "version": "1.0.0", "require": {"third/pkg": "1.0.0"}},
+        {"name": "second/pkg", "version": "1.1.0", "require": {"provided/pkg": "2.0.0"}},
+        {"name": "third/pkg", "version": "1.0.0", "require": {"provided/pkg": "1.0.0"}},
+        {"name": "provider/pkg", "version": "1.0.0", "provide": {"provided/pkg": "1.0.0"}},
+        {"name": "provider/pkg", "version": "2.0.0", "provide": {"provided/pkg": "2.0.0"}}
+    ]
+]
+
+--EXPECT--
+[
+    "third/pkg-1.0.0.0 (locked)",
+    "first/pkg-1.0.0.0",
+    "provider/pkg-1.0.0.0",
+    "provider/pkg-2.0.0.0",
+    "second/pkg-1.1.0.0"
+]
+
+--EXPECT-OPTIMIZED--
+[
+    "third/pkg-1.0.0.0 (locked)",
+    "first/pkg-1.0.0.0",
+    "provider/pkg-1.0.0.0",
+    "provider/pkg-2.0.0.0",
+    "second/pkg-1.1.0.0"
+]
diff --git a/...dencyResolver/Fixtures/poolbuilder/filter-impossible-packages-only-required-replaces.test b/...dencyResolver/Fixtures/poolbuilder/filter-impossible-packages-only-required-replaces.test
@@ -0,0 +1,57 @@
+--TEST--
+When filtering packages from the pool that cannot meet the fixed/locked requirements, ensure that the requirements for a package that is not required anywhere is not used to filter, as it will be ultimately be removed.
+(Variant where the requirement is on a replaced package, the locked third/pkg must not restrict the replacer)
+(NOTE: We are not optimising this scenario currently)
+
+--REQUEST--
+{
+    "require": {
+        "first/pkg": "*",
+        "second/pkg": "1.1.0",
+        "replacer/pkg": "*"
+    },
+    "locked": [
+        {"name": "first/pkg", "version": "1.0.0", "require": {"second/pkg": "^1.0"}},
+        {"name": "second/pkg", "version": "1.0.0", "require": {"third/pkg": "1.0.0"}},
+        {"name": "third/pkg", "version": "1.0.0", "require": {"replaced/pkg": "1.0.0"}},
+        {"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "1.0.0"}}
+    ],
+    "allowList": [
+        "first/pkg"
+    ],
+    "allowTransitiveDeps": true
+}
+
+--FIXED--
+[
+]
+
+--PACKAGE-REPOS--
+[
+    [
+        {"name": "first/pkg", "version": "1.0.0", "require": {"second/pkg": "*"}},
+        {"name": "second/pkg", "version": "1.0.0", "require": {"third/pkg": "1.0.0"}},
+        {"name": "second/pkg", "version": "1.1.0", "require": {"replaced/pkg": "2.0.0"}},
+        {"name": "third/pkg", "version": "1.0.0", "require": {"replaced/pkg": "1.0.0"}},
+        {"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "1.0.0"}},
+        {"name": "replacer/pkg", "version": "2.0.0", "replace": {"replaced/pkg": "2.0.0"}}
+    ]
+]
+
+--EXPECT--
+[
+    "third/pkg-1.0.0.0 (locked)",
+    "first/pkg-1.0.0.0",
+    "second/pkg-1.1.0.0",
+    "replacer/pkg-1.0.0.0",
+    "replacer/pkg-2.0.0.0"
+]
+
+--EXPECT-OPTIMIZED--
+[
+    "third/pkg-1.0.0.0 (locked)",
+    "first/pkg-1.0.0.0",
+    "second/pkg-1.1.0.0",
+    "replacer/pkg-1.0.0.0",
+    "replacer/pkg-2.0.0.0"
+]
diff --git a/...est/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-only-required.test b/...est/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-only-required.test
@@ -0,0 +1,55 @@
+--TEST--
+When filtering packages from the pool that cannot meet the fixed/locked requirements, ensure that the requirements for a package that is not required anywhere is not used to filter, as it will be ultimately be removed.
+(The locked third/pkg is not required by any package so will be removed, so should not restrict the versions of fourth/pkg)
+
+--REQUEST--
+{
+    "require": {
+        "first/pkg": "*",
+        "second/pkg": "1.1.0"
+    },
+    "locked": [
+        {"name": "first/pkg", "version": "1.0.0", "require": {"second/pkg": "^1.0"}},
+        {"name": "second/pkg", "version": "1.0.0", "require": {"third/pkg": "1.0.0"}},
+        {"name": "third/pkg", "version": "1.0.0", "require": {"fourth/pkg": "1.0.0"}},
+        {"name": "fourth/pkg", "version": "1.0.0"}
+    ],
+    "allowList": [
+        "first/pkg"
+    ],
+    "allowTransitiveDeps": true
+}
+
+--FIXED--
+[
+]
+
+--PACKAGE-REPOS--
+[
+    [
+        {"name": "first/pkg", "version": "1.0.0", "require": {"second/pkg": "*"}},
+        {"name": "second/pkg", "version": "1.0.0", "require": {"third/pkg": "1.0.0"}},
+        {"name": "second/pkg", "version": "1.1.0", "require": {"fourth/pkg": "2.0.0"}},
+        {"name": "third/pkg", "version": "1.0.0", "require": {"fourth/pkg": "1.0.0"}},
+        {"name": "fourth/pkg", "version": "1.0.0"},
+        {"name": "fourth/pkg", "version": "2.0.0"}
+    ]
+]
+
+--EXPECT--
+[
+    "third/pkg-1.0.0.0 (locked)",
+    "first/pkg-1.0.0.0",
+    "second/pkg-1.1.0.0",
+    "fourth/pkg-1.0.0.0",
+    "fourth/pkg-2.0.0.0"
+]
+
+--EXPECT-OPTIMIZED--
+[
+    "third/pkg-1.0.0.0 (locked)",
+    "first/pkg-1.0.0.0",
+    "second/pkg-1.1.0.0",
+    "fourth/pkg-1.0.0.0",
+    "fourth/pkg-2.0.0.0"
+]
diff --git a/...ser/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-provides.test b/...ser/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-provides.test
@@ -0,0 +1,59 @@
+--TEST--
+Do not load packages into the pool that cannot meet the fixed/locked requirements, when a loose requirement is encountered during update
+(Variant where requirement is on a provided package, the locked second package should restrict the provider to those that provide < 3.0.0)
+(NOTE: We are not optimising this scenario currently)
+
+--REQUEST--
+{
+    "require": {
+        "first/pkg": "*",
+        "second/pkg": "*",
+        "provider/pkg": "*"
+    },
+    "locked": [
+        {"name": "first/pkg", "version": "1.0.0", "require": {"provided/pkg": "1.0.0"}},
+        {"name": "second/pkg", "version": "1.0.0", "require": {"provided/pkg": "< 3.0.0"}},
+        {"name": "provider/pkg", "version": "1.0.0", "provide": {"provided/pkg": "1.0.0"}}
+    ],
+    "allowList": [
+        "first/pkg",
+        "provider/pkg"
+    ],
+    "allowTransitiveDeps": true
+}
+
+--FIXED--
+[
+]
+
+--PACKAGE-REPOS--
+[
+    [
+        {"name": "first/pkg", "version": "1.0.0", "require": {"provided/pkg": "1.0.0"}},
+        {"name": "first/pkg", "version": "1.1.0", "require": {"provided/pkg": "2.0.0"}},
+        {"name": "second/pkg", "version": "1.0.0", "require": {"provided/pkg": "*"}},
+        {"name": "provider/pkg", "version": "1.0.0", "provide": {"provided/pkg": "1.0.0"}},
+        {"name": "provider/pkg", "version": "1.1.0", "provide": {"provided/pkg": "2.0.0"}},
+        {"name": "provider/pkg", "version": "1.2.0", "provide": {"provided/pkg": "3.0.0"}}
+    ]
+]
+
+--EXPECT--
+[
+    "second/pkg-1.0.0.0 (locked)",
+    "first/pkg-1.0.0.0",
+    "first/pkg-1.1.0.0",
+    "provider/pkg-1.0.0.0",
+    "provider/pkg-1.1.0.0",
+    "provider/pkg-1.2.0.0"
+]
+
+--EXPECT-OPTIMIZED--
+[
+    "second/pkg-1.0.0.0 (locked)",
+    "first/pkg-1.0.0.0",
+    "first/pkg-1.1.0.0",
+    "provider/pkg-1.0.0.0",
+    "provider/pkg-1.1.0.0",
+    "provider/pkg-1.2.0.0"
+]
diff --git a/...ser/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-replaces.test b/...ser/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-replaces.test
@@ -0,0 +1,58 @@
+--TEST--
+Do not load packages into the pool that cannot meet the fixed/locked requirements, when a loose requirement is encountered during update
+(Variant where requirement is on a replaced package, the locked second package should restrict the replacer to those that replace < 3.0.0)
+(NOTE: We are not optimising this scenario currently)
+
+--REQUEST--
+{
+    "require": {
+        "first/pkg": "*",
+        "second/pkg": "*",
+        "replacer/pkg": "*"
+    },
+    "locked": [
+        {"name": "first/pkg", "version": "1.0.0", "require": {"replaced/pkg": "1.0.0"}},
+        {"name": "second/pkg", "version": "1.0.0", "require": {"replaced/pkg": "< 3.0.0"}},
+        {"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "1.0.0"}}
+    ],
+    "allowList": [
+        "first/pkg"
+    ],
+    "allowTransitiveDeps": true
+}
+
+--FIXED--
+[
+]
+
+--PACKAGE-REPOS--
+[
+    [
+        {"name": "first/pkg", "version": "1.0.0", "require": {"replaced/pkg": "1.0.0"}},
+        {"name": "first/pkg", "version": "1.1.0", "require": {"replaced/pkg": "2.0.0"}},
+        {"name": "second/pkg", "version": "1.0.0", "require": {"replaced/pkg": "*"}},
+        {"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "1.0.0"}},
+        {"name": "replacer/pkg", "version": "1.1.0", "replace": {"replaced/pkg": "2.0.0"}},
+        {"name": "replacer/pkg", "version": "1.2.0", "replace": {"replaced/pkg": "3.0.0"}}
+    ]
+]
+
+--EXPECT--
+[
+    "second/pkg-1.0.0.0 (locked)",
+    "first/pkg-1.0.0.0",
+    "first/pkg-1.1.0.0",
+    "replacer/pkg-1.0.0.0",
+    "replacer/pkg-1.1.0.0",
+    "replacer/pkg-1.2.0.0"
+]
+
+--EXPECT-OPTIMIZED--
+[
+    "second/pkg-1.0.0.0 (locked)",
+    "first/pkg-1.0.0.0",
+    "first/pkg-1.1.0.0",
+    "replacer/pkg-1.0.0.0",
+    "replacer/pkg-1.1.0.0",
+    "replacer/pkg-1.2.0.0"
+]
diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages.test
@@ -1,5 +1,6 @@
 --TEST--
 Do not load packages into the pool that cannot meet the fixed/locked requirements, when a loose requirement is encountered during update
+(The locked root/req package should restrict dep/dep to only 2.* versions)
 
 --REQUEST--
 {