Fix implementation & tweak test

src/Composer/DependencyResolver/PoolBuilder.php

@@ -165,24 +165,27 @@ public function buildPool(array $repositories, Request $request)
 
             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[$lockedName] = $lockedName;
+                    foreach ($lockedPackage->getReplaces() as $link) {
+                        $this->skippedLoad[$link->getTarget()] = $lockedName;
+                    }
+
                     // 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) {
-                            $this->pathRepoUnlocked[$lockedPackage->getName()] = true;
+                            $this->pathRepoUnlocked[$lockedPackage->getName()] = $lockedName;
                             continue;
                         }
                     }
 
                     $request->lockPackage($lockedPackage);
-                    $lockedName = $lockedPackage->getName();
-                    // remember which packages we skipped loading remote content for in this partial update
-                    $this->skippedLoad[$lockedName] = $lockedName;
-                    foreach ($lockedPackage->getReplaces() as $link) {
-                        $this->skippedLoad[$link->getTarget()] = $lockedName;
-                    }
                 }
             }
         }
@@ -205,7 +208,7 @@ public function buildPool(array $repositories, Request $request)
                 || $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;
             }
@@ -355,7 +358,7 @@ private function markPackageNameForLoading(Request $request, $name, ConstraintIn
      * @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;
@@ -382,16 +385,17 @@ private function loadPackagesMarkedForLoading(Request $request, $repositories)
             }
             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;
@@ -444,7 +448,7 @@ private function loadPackage(Request $request, BasePackage $package, $propagateU
                 // dependency of another package which we are trying to update, and then attempt to load it again
                 if ($propagateUpdate && $request->getUpdateAllowTransitiveDependencies()) {
                     if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$require])) {
-                        $this->unlockPackage($request, $require);
+                        $this->unlockPackage($request, $repositories, $require);
                         $this->markPackageNameForLoading($request, $require, $linkConstraint);
                     } elseif (!isset($this->updateAllowWarned[$this->skippedLoad[$require]])) {
                         $this->updateAllowWarned[$this->skippedLoad[$require]] = true;
@@ -463,7 +467,7 @@ private function loadPackage(Request $request, BasePackage $package, $propagateU
                 $replace = $link->getTarget();
                 if (isset($this->loadedPackages[$replace], $this->skippedLoad[$replace])) {
                     if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$replace])) {
-                        $this->unlockPackage($request, $replace);
+                        $this->unlockPackage($request, $repositories, $replace);
                         $this->markPackageNameForLoading($request, $replace, $link->getConstraint());
                     } elseif (!$request->getUpdateAllowTransitiveRootDependencies() && $this->isRootRequire($request, $replace) && !isset($this->updateAllowWarned[$replace])) {
                         $this->updateAllowWarned[$replace] = true;
@@ -535,10 +539,11 @@ private function warnAboutNonMatchingUpdateAllowList(Request $request)
      * 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)
     {
         if (
             // if we unfixed a replaced package name, we also need to unfix the replacer itself
@@ -547,7 +552,7 @@ private function unlockPackage(Request $request, $name)
             && isset($this->skippedLoad[$this->skippedLoad[$name]])
         ) {
             $replacerName = $this->skippedLoad[$name];
-            $this->unlockPackage($request, $replacerName);
+            $this->unlockPackage($request, $repositories, $replacerName);
 
             if ($this->isRootRequire($request, $replacerName)) {
                 $this->markPackageNameForLoading($request, $replacerName, new MatchAllConstraint);
@@ -561,14 +566,22 @@ private function unlockPackage(Request $request, $name)
             }
         }
 
+        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
         foreach ($request->getLockedPackages() as $lockedPackage) {
             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
@@ -591,15 +604,23 @@ private function unlockPackage(Request $request, $name)
     }
 
     /**
+     * @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)]);

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"
 ]

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"
+]

tests/Composer/Test/DependencyResolver/PoolBuilderTest.php

@@ -140,6 +140,8 @@ public function testPoolBuilder($file, $message, $expect, $expectOptimized, $roo
         $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 @@ private function getPackageResultSet(Pool $pool, $packageIds)
             $result[] = $pool->packageById($i);
         }
 
-        chdir($oldCwd);
-
         return array_map(function (BasePackage $package) use ($packageIds) {
             if ($id = array_search($package, $packageIds, true)) {
                 return $id;