diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index b421af96a..0a81bdde9 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -165,9 +165,18 @@ class PoolBuilder foreach ($request->getLockedRepository()->getPackages() as $lockedPackage) { if (!$this->isUpdateAllowed($lockedPackage)) { + $lockedName = $lockedPackage->getName(); + + // remember which packages we skipped loading remote content for in this partial update + $this->skippedLoad[$lockedPackage->getName()][] = $lockedPackage; + foreach ($lockedPackage->getReplaces() as $link) { + $this->skippedLoad[$link->getTarget()][] = $lockedPackage; + } + // Path repo packages are never loaded from lock, to force them to always remain in sync // unless symlinking is disabled in which case we probably should rather treat them like - // regular packages + // regular packages. We mark them specially so they can be reloaded fully including update propagation + // if they do get unlocked, but by default they are unlocked without update propagation. if ($lockedPackage->getDistType() === 'path') { $transportOptions = $lockedPackage->getTransportOptions(); if (!isset($transportOptions['symlink']) || $transportOptions['symlink'] !== false) { @@ -177,11 +186,6 @@ class PoolBuilder } $request->lockPackage($lockedPackage); - $this->skippedLoad[$lockedPackage->getName()][] = $lockedPackage; - // remember which packages we skipped loading remote content for in this partial update - foreach ($lockedPackage->getReplaces() as $link) { - $this->skippedLoad[$link->getTarget()][] = $lockedPackage; - } } } } @@ -204,7 +208,7 @@ class PoolBuilder || $package->getRepository() instanceof PlatformRepository || StabilityFilter::isPackageAcceptable($this->acceptableStabilities, $this->stabilityFlags, $package->getNames(), $package->getStability()) ) { - $this->loadPackage($request, $package, false); + $this->loadPackage($request, $repositories, $package, false); } else { $this->unacceptableFixedOrLockedPackages[] = $package; } @@ -354,7 +358,7 @@ class PoolBuilder * @param RepositoryInterface[] $repositories * @return void */ - private function loadPackagesMarkedForLoading(Request $request, $repositories) + private function loadPackagesMarkedForLoading(Request $request, array $repositories) { foreach ($this->packagesToLoad as $name => $constraint) { $this->loadedPackages[$name] = $constraint; @@ -381,16 +385,17 @@ class PoolBuilder } foreach ($result['packages'] as $package) { $this->loadedPerRepo[$repoIndex][$package->getName()][$package->getVersion()] = $package; - $this->loadPackage($request, $package, !isset($this->pathRepoUnlocked[$package->getName()])); + $this->loadPackage($request, $repositories, $package, !isset($this->pathRepoUnlocked[$package->getName()])); } } } /** * @param bool $propagateUpdate + * @param RepositoryInterface[] $repositories * @return void */ - private function loadPackage(Request $request, BasePackage $package, $propagateUpdate) + private function loadPackage(Request $request, array $repositories, BasePackage $package, $propagateUpdate) { $index = $this->indexCounter++; $this->packages[$index] = $package; @@ -445,7 +450,7 @@ class PoolBuilder $skippedRootRequires = $this->getSkippedRootRequires($request, $require); if ($request->getUpdateAllowTransitiveRootDependencies() || !$skippedRootRequires) { - $this->unlockPackage($request, $require); + $this->unlockPackage($request, $repositories, $require); $this->markPackageNameForLoading($request, $require, $linkConstraint); } else { foreach ($skippedRootRequires as $rootRequire) { @@ -470,7 +475,7 @@ class PoolBuilder $skippedRootRequires = $this->getSkippedRootRequires($request, $replace); if ($request->getUpdateAllowTransitiveRootDependencies() || !$skippedRootRequires) { - $this->unlockPackage($request, $replace); + $this->unlockPackage($request, $repositories, $replace); $this->markPackageNameForLoading($request, $replace, $link->getConstraint()); } else { foreach ($skippedRootRequires as $rootRequire) { @@ -588,10 +593,11 @@ class PoolBuilder * Reverts the decision to use a locked package if a partial update with transitive dependencies * found that this package actually needs to be updated * + * @param RepositoryInterface[] $repositories * @param string $name * @return void */ - private function unlockPackage(Request $request, $name) + private function unlockPackage(Request $request, array $repositories, $name) { foreach ($this->skippedLoad[$name] as $packageOrReplacer) { // if we unfixed a replaced package name, we also need to unfix the replacer itself @@ -599,7 +605,7 @@ class PoolBuilder if ($packageOrReplacer->getName() !== $name && isset($this->skippedLoad[$packageOrReplacer->getName()])) { $replacerName = $packageOrReplacer->getName(); if ($request->getUpdateAllowTransitiveRootDependencies() || (!$this->isRootRequire($request, $name) && !$this->isRootRequire($request, $replacerName))) { - $this->unlockPackage($request, $replacerName); + $this->unlockPackage($request, $repositories, $replacerName); if ($this->isRootRequire($request, $replacerName)) { $this->markPackageNameForLoading($request, $replacerName, new MatchAllConstraint); @@ -615,6 +621,14 @@ class PoolBuilder } } + if (isset($this->pathRepoUnlocked[$name])) { + foreach ($this->packages as $index => $package) { + if ($package->getName() === $name) { + $this->removeLoadedPackage($request, $repositories, $package, $index, false); + } + } + } + unset($this->skippedLoad[$name], $this->loadedPackages[$name], $this->maxExtendedReqs[$name], $this->pathRepoUnlocked[$name]); // remove locked package by this name which was already initialized @@ -622,7 +636,7 @@ class PoolBuilder if (!($lockedPackage instanceof AliasPackage) && $lockedPackage->getName() === $name) { if (false !== $index = array_search($lockedPackage, $this->packages, true)) { $request->unlockPackage($lockedPackage); - $this->removeLoadedPackage($request, $lockedPackage, $index); + $this->removeLoadedPackage($request, $repositories, $lockedPackage, $index, true); // make sure that any requirements for this package by other locked or fixed packages are now // also loaded, as they were previously ignored because the locked (now unlocked) package already @@ -645,15 +659,23 @@ class PoolBuilder } /** + * @param RepositoryInterface[] $repositories * @param int $index + * @param bool $unlock * @return void */ - private function removeLoadedPackage(Request $request, BasePackage $package, $index) + private function removeLoadedPackage(Request $request, array $repositories, BasePackage $package, $index, $unlock) { + $repoIndex = array_search($package->getRepository(), $repositories, true); + + unset($this->loadedPerRepo[$repoIndex][$package->getName()][$package->getVersion()]); unset($this->packages[$index]); if (isset($this->aliasMap[spl_object_hash($package)])) { foreach ($this->aliasMap[spl_object_hash($package)] as $aliasIndex => $aliasPackage) { - $request->unlockPackage($aliasPackage); + if ($unlock) { + $request->unlockPackage($aliasPackage); + } + unset($this->loadedPerRepo[$repoIndex][$aliasPackage->getName()][$aliasPackage->getVersion()]); unset($this->packages[$aliasIndex]); } unset($this->aliasMap[spl_object_hash($package)]); diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixes-path-repo-replacer-with-transitive-deps.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixes-path-repo-replacer-with-transitive-deps.test index 54cb48b83..4fc0f4a7b 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixes-path-repo-replacer-with-transitive-deps.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixes-path-repo-replacer-with-transitive-deps.test @@ -6,7 +6,8 @@ Partially updating with deps a root requirement which depends on packages in a s "require": { "root/update": "*", "symlinked/transitive3": "*", - "symlinked/transitive5": "*" + "symlinked/transitive5": "*", + "symlinked/path-pkg-replace": "*" }, "locked": [ {"name": "root/update", "version": "1.0.1", "require": {"symlinked/path-pkg": ">=1.0.1", "symlinked/replaced-pkg": "*"}}, @@ -69,14 +70,15 @@ Partially updating with deps a root requirement which depends on packages in a s --EXPECT-- [ - "symlinked/transitive-1.0.0.0", - "symlinked/transitive-2.0.2.0", - "symlinked/path-pkg-2.0.0.0", "root/update-1.0.4.0", + "symlinked/path-pkg-2.0.0.0", + "symlinked/path-pkg-replace-2.0.0.0", + "symlinked/transitive-2.0.2.0", + "symlinked/transitive3-1.0.0.0", "symlinked/transitive3-1.0.3.0", "symlinked/transitive3-2.0.4.0", - "symlinked/transitive4-1.0.0.0", "symlinked/transitive4-2.0.2.0", + "symlinked/transitive5-1.0.0.0", "symlinked/transitive5-1.0.3.0", "symlinked/transitive5-2.0.4.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 8e7c8068f..10f2d7fd5 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 @@ -77,3 +77,15 @@ Partially updating one root requirement with transitive deps fully updates trans "mirrored/transitive2-1.0.7.0", "mirrored/transitive2-2.0.8.0" ] + +--EXPECT-OPTIMIZED-- +[ + "symlinked/transitive-1.0.0.0 (locked)", + "mirrored/transitive-1.0.0.0 (locked)", + "mirrored/path-pkg-1.0.0.0 (locked)", + "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-2.0.8.0" +] diff --git a/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php b/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php index 6be0d3016..58517af28 100644 --- a/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php +++ b/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php @@ -140,6 +140,8 @@ class PoolBuilderTest extends TestCase $optimizer = new PoolOptimizer(new DefaultPolicy()); $result = $this->getPackageResultSet($optimizer->optimize($request, $pool), $packageIds); $this->assertSame($expectOptimized, $result, 'Optimized pool does not match expected package set'); + + chdir($oldCwd); } /** @@ -153,8 +155,6 @@ class PoolBuilderTest extends TestCase $result[] = $pool->packageById($i); } - chdir($oldCwd); - return array_map(function (BasePackage $package) use ($packageIds) { if ($id = array_search($package, $packageIds, true)) { return $id;