Skip to content

Commit 2928b87

Browse files
nadermannaderman
authored
Merge pull request #11449 from Toflar/fix/poolbuilder-replace-should-be-optional
Optimize PoolBuilder to not load replaced targets if not required
2 parents 9555ae1 + 9ced20f commit 2928b87

File tree

5 files changed

+142
-4
lines changed

5 files changed

+142
-4
lines changed

‎src/Composer/DependencyResolver/PoolBuilder.php‎

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ class PoolBuilder
9494
* @phpstan-var array<int, array<string, array<string, PackageInterface>>>
9595
*/
9696
private $loadedPerRepo = [];
97+
/**
98+
* @var array[]
99+
* @phpstan-var array<string, bool>
100+
*/
101+
private $optionalPackages = [];
97102
/**
98103
* @var BasePackage[]
99104
*/
@@ -235,8 +240,9 @@ public function buildPool(array $repositories, Request $request): Pool
235240
}
236241
}
237242

238-
while (!empty($this->packagesToLoad)) {
243+
while ([] !== $this->packagesToLoad || [] !== $this->optionalPackages) {
239244
$this->loadPackagesMarkedForLoading($request, $repositories);
245+
$this->loadOptionalPackages($request);
240246
}
241247

242248
if (\count($this->temporaryConstraints) > 0) {
@@ -483,7 +489,8 @@ private function loadPackage(Request $request, array $repositories, BasePackage
483489

484490
if ($request->getUpdateAllowTransitiveRootDependencies() || !$skippedRootRequires) {
485491
$this->unlockPackage($request, $repositories, $replace);
486-
$this->markPackageNameForLoading($request, $replace, $link->getConstraint());
492+
// Mark as optional - if no other package requires it, we don't need to load it
493+
$this->markPackageNameForOptionalLoading($replace);
487494
} else {
488495
foreach ($skippedRootRequires as $rootRequire) {
489496
if (!isset($this->updateAllowWarned[$rootRequire])) {
@@ -639,6 +646,8 @@ private function unlockPackage(Request $request, array $repositories, string $na
639646
// make sure that any requirements for this package by other locked or fixed packages are now
640647
// also loaded, as they were previously ignored because the locked (now unlocked) package already
641648
// satisfied their requirements
649+
// and if this package is replacing another that is required by a locked or fixed package, ensure
650+
// that we load that replaced package in case an update to this package removes the replacement
642651
foreach ($request->getFixedOrLockedPackages() as $fixedOrLockedPackage) {
643652
if ($fixedOrLockedPackage === $lockedPackage) {
644653
continue;
@@ -649,13 +658,45 @@ private function unlockPackage(Request $request, array $repositories, string $na
649658
if (isset($requires[$lockedPackage->getName()])) {
650659
$this->markPackageNameForLoading($request, $lockedPackage->getName(), $requires[$lockedPackage->getName()]->getConstraint());
651660
}
661+
662+
foreach ($lockedPackage->getReplaces() as $replace) {
663+
if (isset($requires[$replace->getTarget()], $this->skippedLoad[$replace->getTarget()])) {
664+
$this->unlockPackage($request, $repositories, $replace->getTarget());
665+
// Do not call markPackageNameForOptionalLoading() here, we know that $lockedPackage is already
666+
// part of $this->packages, and we check for $requires[$replace->getTarget()] so we're guaranteed
667+
// to require this package.
668+
$this->markPackageNameForLoading($request, $replace->getTarget(), $replace->getConstraint());
669+
}
670+
}
652671
}
653672
}
654673
}
655674
}
656675
}
657676
}
658677

678+
private function markPackageNameForOptionalLoading(string $name): void
679+
{
680+
$this->optionalPackages[$name] = true;
681+
}
682+
683+
private function loadOptionalPackages(Request $request): void
684+
{
685+
if ([] === $this->optionalPackages) {
686+
return;
687+
}
688+
689+
foreach ($this->packages as $package) {
690+
foreach ($package->getRequires() as $link) {
691+
if (isset($this->optionalPackages[$link->getTarget()])) {
692+
$this->markPackageNameForLoading($request, $link->getTarget(), $link->getConstraint());
693+
}
694+
}
695+
}
696+
697+
$this->optionalPackages = [];
698+
}
699+
659700
/**
660701
* @param RepositoryInterface[] $repositories
661702
*/

‎tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/load-replaced-package-if-replacer-dropped.test‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
--TEST--
2+
Ensure that a package gets loaded which was previously skipped due to replacement
3+
4+
--REQUEST--
5+
{
6+
"require": {
7+
"root/dep": "*",
8+
"root/no-update": "*"
9+
},
10+
"locked": [
11+
{"name": "root/dep", "version": "1.1.0", "require": {"replacer/pkg": "1.*"}},
12+
{"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "1.0.0"}},
13+
{"name": "root/no-update", "version": "1.0.0", "require": {"replaced/pkg": "1.0.0"}}
14+
],
15+
"allowList": [
16+
"root/dep"
17+
],
18+
"allowTransitiveDepsNoRootRequire": true
19+
}
20+
21+
--FIXED--
22+
[
23+
]
24+
25+
--PACKAGE-REPOS--
26+
[
27+
[
28+
{"name": "root/dep", "version": "1.2.0", "require": {"replacer/pkg": ">=1.1.0"}},
29+
{"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "1.0.0"}},
30+
{"name": "replacer/pkg", "version": "1.1.0"},
31+
{"name": "replaced/pkg", "version": "1.0.0"},
32+
{"name": "root/no-update", "version": "1.0.0", "require": {"replaced/pkg": "1.0.0"}}
33+
]
34+
]
35+
36+
--EXPECT--
37+
[
38+
"root/no-update-1.0.0.0 (locked)",
39+
"root/dep-1.2.0.0",
40+
"replaced/pkg-1.0.0.0",
41+
"replacer/pkg-1.1.0.0"
42+
]
43+
44+
--EXPECT-OPTIMIZED--
45+
[
46+
"root/no-update-1.0.0.0 (locked)",
47+
"root/dep-1.2.0.0",
48+
"replaced/pkg-1.0.0.0",
49+
"replacer/pkg-1.1.0.0"
50+
]

‎tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace-partial-update-all.test‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ Check that replacers from additional repositories are loaded when doing a partia
101101
"indirect/replacer-1.0.0.0",
102102
"replacer/package-1.2.0.0",
103103
"replacer/package-1.0.0.0",
104-
"base/package-1.0.0.0",
105104
"shared/dep-1.0.0.0",
106105
"shared/dep-1.2.0.0"
107106
]
@@ -112,6 +111,5 @@ Check that replacers from additional repositories are loaded when doing a partia
112111
"indirect/replacer-1.0.0.0",
113112
"replacer/package-1.2.0.0",
114113
"replacer/package-1.0.0.0",
115-
"base/package-1.0.0.0",
116114
"shared/dep-1.2.0.0"
117115
]

‎tests/Composer/Test/DependencyResolver/PoolBuilderTest.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,14 @@ public function testPoolBuilder(string $file, string $message, array $expect, ar
137137

138138
$result = $this->getPackageResultSet($pool, $packageIds);
139139

140+
sort($expect);
141+
sort($result);
140142
$this->assertSame($expect, $result, 'Unoptimized pool does not match expected package set');
141143

142144
$optimizer = new PoolOptimizer(new DefaultPolicy());
143145
$result = $this->getPackageResultSet($optimizer->optimize($request, $pool), $packageIds);
146+
sort($expectOptimized);
147+
sort($result);
144148
$this->assertSame($expectOptimized, $result, 'Optimized pool does not match expected package set');
145149

146150
chdir($oldCwd);

‎tests/Composer/Test/Fixtures/installer/load-replaced-package-if-replacer-dropped.test‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
--TEST--
2+
Ensure that a package gets loaded which was previously skipped due to replacement
3+
--COMPOSER--
4+
{
5+
"repositories": [
6+
{
7+
"type": "package",
8+
"package": [
9+
{"name": "root/dep", "version": "1.2.0", "require": {"replacer/pkg": ">=1.1.0"}},
10+
{"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "1.0.0"}},
11+
{"name": "replacer/pkg", "version": "1.1.0"},
12+
{"name": "replaced/pkg", "version": "1.0.0"},
13+
{"name": "root/no-update", "version": "1.0.0", "require": {"replaced/pkg": "1.0.0"}}
14+
]
15+
}
16+
],
17+
"require": {
18+
"root/dep": "*",
19+
"root/no-update": "*"
20+
}
21+
}
22+
--LOCK--
23+
{
24+
"packages": [
25+
{"name": "root/dep", "version": "1.1.0", "require": {"replacer/pkg": "1.*"}},
26+
{"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "1.0.0"}},
27+
{"name": "root/no-update", "version": "1.0.0", "require": {"replaced/pkg": "1.0.0"}}
28+
],
29+
"packages-dev": [],
30+
"aliases": [],
31+
"minimum-stability": "dev",
32+
"stability-flags": [],
33+
"prefer-stable": false,
34+
"prefer-lowest": false,
35+
"platform": [],
36+
"platform-dev": []
37+
}
38+
--RUN--
39+
update root/dep --with-all-dependencies
40+
--EXPECT--
41+
Installing replacer/pkg (1.1.0)
42+
Installing root/dep (1.2.0)
43+
Installing replaced/pkg (1.0.0)
44+
Installing root/no-update (1.0.0)
45+

0 commit comments

Comments
 (0)