Skip to content

Commit

Permalink
Re-organized PoolOptimizer - reverting composer#9620 to fix composer#…
Browse files Browse the repository at this point in the history 
…9393
  • Loading branch information
@Toflar
Toflar committed Dec 20, 2023
1 parent 53a1f32 commit 63c5a04
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 100 deletions.
108 changes: 11 additions & 97 deletions src/Composer/DependencyResolver/PoolOptimizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,6 @@ class PoolOptimizer
*/
private $conflictConstraintsPerPackage = [];

/**
* @var array<int, true>
*/
private $packagesToRemove = [];

/**
* @var array<int, BasePackage[]>
*/
Expand All @@ -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
Expand All @@ -86,7 +79,6 @@ public function optimize(Request $request, Pool $pool): Pool
$this->irremovablePackages = [];
$this->requireConstraintsPerPackage = [];
$this->conflictConstraintsPerPackage = [];
$this->packagesToRemove = [];
$this->aliasesPerPackage = [];
$this->removedVersionsByPackage = [];

Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}

Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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<string, array<string, array<string, list<BasePackage>>>> $identicalDefinitionsPerPackage
* @param array<int, array<string, array{groupHash: string, dependencyHash: string}>> $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
Expand All @@ -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])) {
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion ...t/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-locked-replacer.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
1 change: 1 addition & 0 deletions tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
3 changes: 2 additions & 1 deletion ...s/poolbuilder/partial-update-unfixes-path-repos-always-but-not-their-transitive-deps.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
3 changes: 2 additions & 1 deletion ...ser/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-locked-deps.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]

0 comments on commit 63c5a04

Please sign in to comment.