From 2fa58ccf963ca5f4f0e059d0be238d8f4226e519 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sat, 6 Jun 2020 17:18:42 +0200 Subject: [PATCH] Reduce amount of packages loaded by avoiding extensions of the constraint beyond the root constraint --- .../DependencyResolver/PoolBuilder.php | 29 ++++++++++++++++++- .../alias-priority-conflicting.test | 8 ++--- .../poolbuilder/alias-with-reference.test | 3 +- .../partial-update-transitive-deps-unfix.test | 17 +++++++---- 4 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index c6ea7d73d..91381bee2 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -80,6 +80,16 @@ class PoolBuilder private $unacceptableFixedPackages = array(); private $updateAllowList = array(); private $skippedLoad = array(); + + /** + * Keeps a list of dependencies which are root requirements, and as such + * have already their maximum required range loaded and can not be + * extended by markPackageNameForLoading + * + * Packages get cleared from this list if they get unfixed as in that case + * we need to actually load them + */ + private $maxExtendedReqs = array(); /** * @psalm-var array */ @@ -156,7 +166,8 @@ class PoolBuilder continue; } - $this->markPackageNameForLoading($request, $packageName, $constraint); + $this->packagesToLoad[$packageName] = $constraint; + $this->maxExtendedReqs[$packageName] = true; } // clean up packagesToLoad for anything we manually marked loaded above @@ -225,6 +236,21 @@ class PoolBuilder private function markPackageNameForLoading(Request $request, $name, ConstraintInterface $constraint) { + // Root require (which was not unfixed) already loaded the maximum range so no + // need to check anything here + if (isset($this->maxExtendedReqs[$name])) { + return; + } + + // Root requires can not be overruled by dependencies so there is no point in + // extending the loaded constraint for those. + // This is triggered when loading a root require which was fixed but got unfixed, then + // we make sure that we load at most the intervals covered by the root constraint. + $rootRequires = $request->getRequires(); + if (isset($rootRequires[$name]) && !Intervals::isSubsetOf($constraint, $rootRequires[$name])) { + $constraint = $rootRequires[$name]; + } + // Maybe it was already marked before but not loaded yet. In that case // we have to extend the constraint (we don't check if they match because // MultiConstraint::create() will optimize anyway) @@ -457,6 +483,7 @@ class PoolBuilder unset($this->skippedLoad[$name]); unset($this->loadedPackages[$name]); + unset($this->maxExtendedReqs[$name]); } private function removeLoadedPackage(Request $request, PackageInterface $package, $index) diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test index 68136d6c2..4386df5ac 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test @@ -50,12 +50,8 @@ Check root aliases are loaded --EXPECT-- [ - "package/a-dev-master", - "package/a-9999999-dev (alias of dev-master)", "req/pkg-dev-feature-foo#feat.f", "req/pkg-dev-master#feat.f (alias of dev-feature-foo)", - "req/pkg-dev-master#forked", - "req/pkg-dev-master#master", - "req/pkg-1.0.9999999.9999999-dev#forked (alias of dev-master)", - "req/pkg-1.0.9999999.9999999-dev#master (alias of dev-master)" + "package/a-dev-master", + "package/a-9999999-dev (alias of dev-master)" ] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test index 4401f869e..f7ef17b2f 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test @@ -49,9 +49,8 @@ Check root aliases get selected correctly --EXPECT-- [ - "b/requirer-1.0.0.0#1.0.0", "a/aliased-dev-master#abcd", "a/aliased-1.0.0.0#abcd (alias of dev-master)", - "a/aliased-1.0.0.0#abcd", + "b/requirer-1.0.0.0#1.0.0", "a/aliased-9999999-dev#abcd (alias of dev-master)" ] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test index e075c2df2..d3cfb0db1 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test @@ -5,12 +5,14 @@ Partially updating one root requirement with transitive deps fully updates anoth { "require": { "root/update": "*", - "root/dep": "*" + "root/dep": "1.0.*", + "root/dep2": "*" }, "locked": [ {"name": "root/update", "version": "1.0.1", "require": {"dep/dep": "1.*"}}, - {"name": "dep/dep", "version": "1.0.2", "require": {"root/dep": "1.*"}}, - {"name": "root/dep", "version": "1.0.3"} + {"name": "dep/dep", "version": "1.0.2", "require": {"root/dep": "1.*", "root/dep2": "1.*"}}, + {"name": "root/dep", "version": "1.0.3"}, + {"name": "root/dep2", "version": "1.0.3"} ], "allowList": [ "root/update" @@ -28,8 +30,10 @@ Partially updating one root requirement with transitive deps fully updates anoth {"name": "root/update", "version": "1.0.5", "require": {"dep/dep": "2.*"}}, {"name": "root/dep", "version": "1.0.6"}, {"name": "root/dep", "version": "2.0.7"}, - {"name": "dep/dep", "version": "1.0.8", "require": {"root/dep": "1.*"}}, - {"name": "dep/dep", "version": "2.0.9", "require": {"root/dep": "2.*"}} + {"name": "root/dep2", "version": "1.0.6"}, + {"name": "root/dep2", "version": "2.0.7"}, + {"name": "dep/dep", "version": "1.0.8", "require": {"root/dep": "1.*", "root/dep2": "1.*"}}, + {"name": "dep/dep", "version": "2.0.9", "require": {"root/dep": "2.*", "root/dep2": "2.*"}} ] --EXPECT-- @@ -39,5 +43,6 @@ Partially updating one root requirement with transitive deps fully updates anoth "dep/dep-1.0.8.0", "dep/dep-2.0.9.0", "root/dep-1.0.6.0", - "root/dep-2.0.7.0" + "root/dep2-1.0.6.0", + "root/dep2-2.0.7.0" ]