diff --git a/src/Composer/DependencyResolver/PoolOptimizer.php b/src/Composer/DependencyResolver/PoolOptimizer.php index c53891fb63e8..ef0857485842 100644 --- a/src/Composer/DependencyResolver/PoolOptimizer.php +++ b/src/Composer/DependencyResolver/PoolOptimizer.php @@ -48,11 +48,6 @@ class PoolOptimizer */ private $conflictConstraintsPerPackage = []; - /** - * @var array - */ - private $packagesToRemove = []; - /** * @var array */ @@ -74,8 +69,6 @@ public function optimize(Request $request, Pool $pool): Pool $this->optimizeByIdenticalDependencies($request, $pool); - $this->optimizeImpossiblePackagesAway($request, $pool); - $optimizedPool = $this->applyRemovalsToPool($pool); // No need to run this recursively at the moment @@ -86,7 +79,6 @@ public function optimize(Request $request, Pool $pool): Pool $this->irremovablePackages = []; $this->requireConstraintsPerPackage = []; $this->conflictConstraintsPerPackage = []; - $this->packagesToRemove = []; $this->aliasesPerPackage = []; $this->removedVersionsByPackage = []; @@ -145,6 +137,11 @@ private function prepare(Request $request, Pool $pool): void private function markPackageIrremovable(BasePackage $package): void { + // Already marked to keep + if (isset($this->irremovablePackages[$package->id])) { + return; + } + $this->irremovablePackages[$package->id] = true; if ($package instanceof AliasPackage) { // recursing here so aliasesPerPackage for the aliasOf can be checked @@ -166,7 +163,7 @@ private function applyRemovalsToPool(Pool $pool): Pool $packages = []; $removedVersions = []; foreach ($pool->getPackages() as $package) { - if (!isset($this->packagesToRemove[$package->id])) { + if (isset($this->irremovablePackages[$package->id])) { $packages[] = $package; } else { $removedVersions[$package->getName()][$package->getVersion()] = $package->getPrettyVersion(); @@ -191,8 +188,6 @@ private function optimizeByIdenticalDependencies(Request $request, Pool $pool): continue; } - $this->markPackageForRemoval($package->id); - $dependencyHash = $this->calculateDependencyHash($package); foreach ($package->getNames(false) as $packageName) { @@ -240,7 +235,7 @@ private function optimizeByIdenticalDependencies(Request $request, Pool $pool): foreach ($constraintGroup as $packages) { // Only one package in this constraint group has the same requirements, we're not allowed to remove that package if (1 === \count($packages)) { - $this->keepPackage($packages[0], $identicalDefinitionsPerPackage, $packageIdenticalDefinitionLookup); + $this->keepPackageWithinDependencyGroups($packages[0], $identicalDefinitionsPerPackage, $packageIdenticalDefinitionLookup); continue; } @@ -253,7 +248,7 @@ private function optimizeByIdenticalDependencies(Request $request, Pool $pool): } foreach ($this->policy->selectPreferredPackages($pool, $literals) as $preferredLiteral) { - $this->keepPackage($pool->literalToPackage($preferredLiteral), $identicalDefinitionsPerPackage, $packageIdenticalDefinitionLookup); + $this->keepPackageWithinDependencyGroups($pool->literalToPackage($preferredLiteral), $identicalDefinitionsPerPackage, $packageIdenticalDefinitionLookup); } } } @@ -300,33 +295,18 @@ private function calculateDependencyHash(BasePackage $package): string return $hash; } - private function markPackageForRemoval(int $id): void - { - // We are not allowed to remove packages if they have been marked as irremovable - if (isset($this->irremovablePackages[$id])) { - throw new \LogicException('Attempted removing a package which was previously marked irremovable'); - } - - $this->packagesToRemove[$id] = true; - } - /** * @param array>>> $identicalDefinitionsPerPackage * @param array> $packageIdenticalDefinitionLookup */ - private function keepPackage(BasePackage $package, array $identicalDefinitionsPerPackage, array $packageIdenticalDefinitionLookup): void + private function keepPackageWithinDependencyGroups(BasePackage $package, array $identicalDefinitionsPerPackage, array $packageIdenticalDefinitionLookup): void { - // Already marked to keep - if (!isset($this->packagesToRemove[$package->id])) { - return; - } - - unset($this->packagesToRemove[$package->id]); + $this->markPackageIrremovable($package); if ($package instanceof AliasPackage) { // recursing here so aliasesPerPackage for the aliasOf can be checked // and all its aliases marked to be kept as well - $this->keepPackage($package->getAliasOf(), $identicalDefinitionsPerPackage, $packageIdenticalDefinitionLookup); + $this->keepPackageWithinDependencyGroups($package->getAliasOf(), $identicalDefinitionsPerPackage, $packageIdenticalDefinitionLookup); } // record all the versions of the package group so we can list them later in Problem output @@ -345,8 +325,6 @@ private function keepPackage(BasePackage $package, array $identicalDefinitionsPe if (isset($this->aliasesPerPackage[$package->id])) { foreach ($this->aliasesPerPackage[$package->id] as $aliasPackage) { - unset($this->packagesToRemove[$aliasPackage->id]); - // record all the versions of the package group so we can list them later in Problem output foreach ($aliasPackage->getNames(false) as $name) { if (isset($packageIdenticalDefinitionLookup[$aliasPackage->id][$name])) { @@ -364,70 +342,6 @@ private function keepPackage(BasePackage $package, array $identicalDefinitionsPe } } - /** - * Use the list of locked packages to constrain the loaded packages - * This will reduce packages with significant numbers of historical versions to a smaller number - * and reduce the resulting rule set that is generated - */ - private function optimizeImpossiblePackagesAway(Request $request, Pool $pool): void - { - if (count($request->getLockedPackages()) === 0) { - return; - } - - $packageIndex = []; - - foreach ($pool->getPackages() as $package) { - $id = $package->id; - - // Do not remove irremovable packages - if (isset($this->irremovablePackages[$id])) { - continue; - } - // Do not remove a package aliased by another package, nor aliases - if (isset($this->aliasesPerPackage[$id]) || $package instanceof AliasPackage) { - continue; - } - // Do not remove locked packages - if ($request->isFixedPackage($package) || $request->isLockedPackage($package)) { - continue; - } - - $packageIndex[$package->getName()][$package->id] = $package; - } - - 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])) { - continue; - } - - $linkConstraint = $link->getConstraint(); - foreach ($packageIndex[$require] as $id => $requiredPkg) { - if (false === CompilingMatcher::match($linkConstraint, Constraint::OP_EQ, $requiredPkg->getVersion())) { - $this->markPackageForRemoval($id); - unset($packageIndex[$require][$id]); - } - } - } - } - } - /** * Disjunctive require constraints need to be considered in their own group. E.g. "^2.14 || ^3.3" needs to generate * two require constraint groups in order for us to keep the best matching package for "^2.14" AND "^3.3" as otherwise, we'd diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-locked-replacer.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-locked-replacer.test index d72d8610543f..5d286a38e013 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-locked-replacer.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-locked-replacer.test @@ -56,5 +56,6 @@ Do not load packages into the pool that cannot meet the fixed/locked requirement "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" + "replacer/dep-1.0.1.0", + "replacer/dep-2.0.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 index edbb8b6d0650..7ab542364e22 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages.test @@ -51,5 +51,6 @@ Do not load packages into the pool that cannot meet the fixed/locked requirement [ 2, "some/pkg-1.0.4.0", + "dep/dep-1.0.2.0", "dep/dep-2.0.1.0" ] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixes-path-repos-always-but-not-their-transitive-deps.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixes-path-repos-always-but-not-their-transitive-deps.test index f1c5487ca7ad..10f2d7fd5df6 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixes-path-repos-always-but-not-their-transitive-deps.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixes-path-repos-always-but-not-their-transitive-deps.test @@ -86,5 +86,6 @@ Partially updating one root requirement with transitive deps fully updates trans "symlinked/path-pkg-2.0.0.0", "root/update-1.0.4.0", "symlinked/transitive2-2.0.4.0", - "mirrored/transitive2-1.0.7.0" + "mirrored/transitive2-1.0.7.0", + "mirrored/transitive2-2.0.8.0" ] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-locked-deps.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-locked-deps.test index ed4372ca6fc0..984590843771 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-locked-deps.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-locked-deps.test @@ -55,5 +55,6 @@ locked packages still need to be taking into account for loading all necessary v "root/req2-1.0.0.0 (locked)", "dep/pkg2-1.0.0.0", "dep/pkg2-1.2.0.0", - "dep/pkg1-1.0.1.0" + "dep/pkg1-1.0.1.0", + "dep/pkg1-2.0.0.0" ]