Skip to content

Commit 63c5a04

Browse files
ToflarToflar
committed
Re-organized PoolOptimizer - reverting composer#9620 to fix composer#9393 
1 parent 53a1f32 commit 63c5a04

5 files changed

+18
-100
lines changed

‎src/Composer/DependencyResolver/PoolOptimizer.php

+11-97
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,6 @@ class PoolOptimizer
4848
*/
4949
private $conflictConstraintsPerPackage = [];
5050

51-
/**
52-
* @var array<int, true>
53-
*/
54-
private $packagesToRemove = [];
55-
5651
/**
5752
* @var array<int, BasePackage[]>
5853
*/
@@ -74,8 +69,6 @@ public function optimize(Request $request, Pool $pool): Pool
7469

7570
$this->optimizeByIdenticalDependencies($request, $pool);
7671

77-
$this->optimizeImpossiblePackagesAway($request, $pool);
78-
7972
$optimizedPool = $this->applyRemovalsToPool($pool);
8073

8174
// No need to run this recursively at the moment
@@ -86,7 +79,6 @@ public function optimize(Request $request, Pool $pool): Pool
8679
$this->irremovablePackages = [];
8780
$this->requireConstraintsPerPackage = [];
8881
$this->conflictConstraintsPerPackage = [];
89-
$this->packagesToRemove = [];
9082
$this->aliasesPerPackage = [];
9183
$this->removedVersionsByPackage = [];
9284

@@ -145,6 +137,11 @@ private function prepare(Request $request, Pool $pool): void
145137

146138
private function markPackageIrremovable(BasePackage $package): void
147139
{
140+
// Already marked to keep
141+
if (isset($this->irremovablePackages[$package->id])) {
142+
return;
143+
}
144+
148145
$this->irremovablePackages[$package->id] = true;
149146
if ($package instanceof AliasPackage) {
150147
// recursing here so aliasesPerPackage for the aliasOf can be checked
@@ -166,7 +163,7 @@ private function applyRemovalsToPool(Pool $pool): Pool
166163
$packages = [];
167164
$removedVersions = [];
168165
foreach ($pool->getPackages() as $package) {
169-
if (!isset($this->packagesToRemove[$package->id])) {
166+
if (isset($this->irremovablePackages[$package->id])) {
170167
$packages[] = $package;
171168
} else {
172169
$removedVersions[$package->getName()][$package->getVersion()] = $package->getPrettyVersion();
@@ -191,8 +188,6 @@ private function optimizeByIdenticalDependencies(Request $request, Pool $pool):
191188
continue;
192189
}
193190

194-
$this->markPackageForRemoval($package->id);
195-
196191
$dependencyHash = $this->calculateDependencyHash($package);
197192

198193
foreach ($package->getNames(false) as $packageName) {
@@ -240,7 +235,7 @@ private function optimizeByIdenticalDependencies(Request $request, Pool $pool):
240235
foreach ($constraintGroup as $packages) {
241236
// Only one package in this constraint group has the same requirements, we're not allowed to remove that package
242237
if (1 === \count($packages)) {
243-
$this->keepPackage($packages[0], $identicalDefinitionsPerPackage, $packageIdenticalDefinitionLookup);
238+
$this->keepPackageWithinDependencyGroups($packages[0], $identicalDefinitionsPerPackage, $packageIdenticalDefinitionLookup);
244239
continue;
245240
}
246241

@@ -253,7 +248,7 @@ private function optimizeByIdenticalDependencies(Request $request, Pool $pool):
253248
}
254249

255250
foreach ($this->policy->selectPreferredPackages($pool, $literals) as $preferredLiteral) {
256-
$this->keepPackage($pool->literalToPackage($preferredLiteral), $identicalDefinitionsPerPackage, $packageIdenticalDefinitionLookup);
251+
$this->keepPackageWithinDependencyGroups($pool->literalToPackage($preferredLiteral), $identicalDefinitionsPerPackage, $packageIdenticalDefinitionLookup);
257252
}
258253
}
259254
}
@@ -300,33 +295,18 @@ private function calculateDependencyHash(BasePackage $package): string
300295
return $hash;
301296
}
302297

303-
private function markPackageForRemoval(int $id): void
304-
{
305-
// We are not allowed to remove packages if they have been marked as irremovable
306-
if (isset($this->irremovablePackages[$id])) {
307-
throw new \LogicException('Attempted removing a package which was previously marked irremovable');
308-
}
309-
310-
$this->packagesToRemove[$id] = true;
311-
}
312-
313298
/**
314299
* @param array<string, array<string, array<string, list<BasePackage>>>> $identicalDefinitionsPerPackage
315300
* @param array<int, array<string, array{groupHash: string, dependencyHash: string}>> $packageIdenticalDefinitionLookup
316301
*/
317-
private function keepPackage(BasePackage $package, array $identicalDefinitionsPerPackage, array $packageIdenticalDefinitionLookup): void
302+
private function keepPackageWithinDependencyGroups(BasePackage $package, array $identicalDefinitionsPerPackage, array $packageIdenticalDefinitionLookup): void
318303
{
319-
// Already marked to keep
320-
if (!isset($this->packagesToRemove[$package->id])) {
321-
return;
322-
}
323-
324-
unset($this->packagesToRemove[$package->id]);
304+
$this->markPackageIrremovable($package);
325305

326306
if ($package instanceof AliasPackage) {
327307
// recursing here so aliasesPerPackage for the aliasOf can be checked
328308
// and all its aliases marked to be kept as well
329-
$this->keepPackage($package->getAliasOf(), $identicalDefinitionsPerPackage, $packageIdenticalDefinitionLookup);
309+
$this->keepPackageWithinDependencyGroups($package->getAliasOf(), $identicalDefinitionsPerPackage, $packageIdenticalDefinitionLookup);
330310
}
331311

332312
// 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
345325

346326
if (isset($this->aliasesPerPackage[$package->id])) {
347327
foreach ($this->aliasesPerPackage[$package->id] as $aliasPackage) {
348-
unset($this->packagesToRemove[$aliasPackage->id]);
349-
350328
// record all the versions of the package group so we can list them later in Problem output
351329
foreach ($aliasPackage->getNames(false) as $name) {
352330
if (isset($packageIdenticalDefinitionLookup[$aliasPackage->id][$name])) {
@@ -364,70 +342,6 @@ private function keepPackage(BasePackage $package, array $identicalDefinitionsPe
364342
}
365343
}
366344

367-
/**
368-
* Use the list of locked packages to constrain the loaded packages
369-
* This will reduce packages with significant numbers of historical versions to a smaller number
370-
* and reduce the resulting rule set that is generated
371-
*/
372-
private function optimizeImpossiblePackagesAway(Request $request, Pool $pool): void
373-
{
374-
if (count($request->getLockedPackages()) === 0) {
375-
return;
376-
}
377-
378-
$packageIndex = [];
379-
380-
foreach ($pool->getPackages() as $package) {
381-
$id = $package->id;
382-
383-
// Do not remove irremovable packages
384-
if (isset($this->irremovablePackages[$id])) {
385-
continue;
386-
}
387-
// Do not remove a package aliased by another package, nor aliases
388-
if (isset($this->aliasesPerPackage[$id]) || $package instanceof AliasPackage) {
389-
continue;
390-
}
391-
// Do not remove locked packages
392-
if ($request->isFixedPackage($package) || $request->isLockedPackage($package)) {
393-
continue;
394-
}
395-
396-
$packageIndex[$package->getName()][$package->id] = $package;
397-
}
398-
399-
foreach ($request->getLockedPackages() as $package) {
400-
// 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
401-
// 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
402-
$isUnusedPackage = true;
403-
foreach ($package->getNames(false) as $packageName) {
404-
if (isset($this->requireConstraintsPerPackage[$packageName])) {
405-
$isUnusedPackage = false;
406-
break;
407-
}
408-
}
409-
410-
if ($isUnusedPackage) {
411-
continue;
412-
}
413-
414-
foreach ($package->getRequires() as $link) {
415-
$require = $link->getTarget();
416-
if (!isset($packageIndex[$require])) {
417-
continue;
418-
}
419-
420-
$linkConstraint = $link->getConstraint();
421-
foreach ($packageIndex[$require] as $id => $requiredPkg) {
422-
if (false === CompilingMatcher::match($linkConstraint, Constraint::OP_EQ, $requiredPkg->getVersion())) {
423-
$this->markPackageForRemoval($id);
424-
unset($packageIndex[$require][$id]);
425-
}
426-
}
427-
}
428-
}
429-
}
430-
431345
/**
432346
* Disjunctive require constraints need to be considered in their own group. E.g. "^2.14 || ^3.3" needs to generate
433347
* two require constraint groups in order for us to keep the best matching package for "^2.14" AND "^3.3" as otherwise, we'd

‎tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages-locked-replacer.test

+2-1
Original file line numberDiff line numberDiff line change
@@ -56,5 +56,6 @@ Do not load packages into the pool that cannot meet the fixed/locked requirement
5656
"first/pkg-1.0.0.0 (locked)",
5757
"replacer/pkg-1.0.0.0 (locked)",
5858
"second/pkg-1.0.0.0",
59-
"replacer/dep-1.0.1.0"
59+
"replacer/dep-1.0.1.0",
60+
"replacer/dep-2.0.0.0"
6061
]

‎tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/filter-impossible-packages.test

+1
Original file line numberDiff line numberDiff line change
@@ -51,5 +51,6 @@ Do not load packages into the pool that cannot meet the fixed/locked requirement
5151
[
5252
2,
5353
"some/pkg-1.0.4.0",
54+
"dep/dep-1.0.2.0",
5455
"dep/dep-2.0.1.0"
5556
]

‎tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixes-path-repos-always-but-not-their-transitive-deps.test

+2-1
Original file line numberDiff line numberDiff line change
@@ -86,5 +86,6 @@ Partially updating one root requirement with transitive deps fully updates trans
8686
"symlinked/path-pkg-2.0.0.0",
8787
"root/update-1.0.4.0",
8888
"symlinked/transitive2-2.0.4.0",
89-
"mirrored/transitive2-1.0.7.0"
89+
"mirrored/transitive2-1.0.7.0",
90+
"mirrored/transitive2-2.0.8.0"
9091
]

‎tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-locked-deps.test

+2-1
Original file line numberDiff line numberDiff line change
@@ -55,5 +55,6 @@ locked packages still need to be taking into account for loading all necessary v
5555
"root/req2-1.0.0.0 (locked)",
5656
"dep/pkg2-1.0.0.0",
5757
"dep/pkg2-1.2.0.0",
58-
"dep/pkg1-1.0.1.0"
58+
"dep/pkg1-1.0.1.0",
59+
"dep/pkg1-2.0.0.0"
5960
]

0 commit comments

Comments
 (0)