Use the old root stack based approach to sorting operations in the transaction

src/Composer/DependencyResolver/LocalRepoTransaction.php

@@ -15,28 +15,54 @@ namespace Composer\DependencyResolver;
 use Composer\DependencyResolver\Operation\MarkAliasUninstalledOperation;
 use Composer\DependencyResolver\Operation\UninstallOperation;
 use Composer\Package\AliasPackage;
+use Composer\Package\Link;
+use Composer\Package\PackageInterface;
 use Composer\Repository\PlatformRepository;
 use Composer\Repository\RepositoryInterface;
+use Composer\Semver\Constraint\Constraint;
 
 /**
  * @author Nils Adermann <naderman@naderman.de>
  */
 class LocalRepoTransaction
 {
-    /** @var RepositoryInterface */
+    /** @var array */
-    protected $lockedRepository;
+    protected $lockedPackages;
+    protected $lockedPackagesByName = array();
 
     /** @var RepositoryInterface */
     protected $localRepository;
 
-    public function __construct($lockedRepository, $localRepository)
+    public function __construct(RepositoryInterface $lockedRepository, $localRepository)
     {
-        $this->lockedRepository = $lockedRepository;
         $this->localRepository = $localRepository;
-
+        $this->setLockedPackageMaps($lockedRepository);
         $this->operations = $this->calculateOperations();
     }
 
+    private function setLockedPackageMaps($lockedRepository)
+    {
+        $packageSort = function (PackageInterface $a, PackageInterface $b) {
+            // sort alias packages by the same name behind their non alias version
+            if ($a->getName() == $b->getName() && $a instanceof AliasPackage != $b instanceof AliasPackage) {
+                return $a instanceof AliasPackage ? -1 : 1;
+            }
+            return strcmp($b->getName(), $a->getName());
+        };
+
+        foreach ($lockedRepository->getPackages() as $package) {
+            $this->lockedPackages[$package->id] = $package;
+            foreach ($package->getNames() as $name) {
+                $this->lockedPackagesByName[$name][] = $package;
+            }
+        }
+
+        uasort($this->lockedPackages, $packageSort);
+        foreach ($this->lockedPackagesByName as $name => $packages) {
+            uasort($this->lockedPackagesByName[$name], $packageSort);
+        }
+    }
+
     public function getOperations()
     {
         return $this->operations;
@@ -48,16 +74,56 @@ class LocalRepoTransaction
 
         $localPackageMap = array();
         $removeMap = array();
+        $localAliasMap = array();
+        $removeAliasMap = array();
         foreach ($this->localRepository->getPackages() as $package) {
-            if (isset($localPackageMap[$package->getName()])) {
+            if ($package instanceof AliasPackage) {
-                die("Alias?");
+                $localAliasMap[$package->getName().'::'.$package->getVersion()] = $package;
-            }
+                $removeAliasMap[$package->getName().'::'.$package->getVersion()] = $package;
+            } else {
                 $localPackageMap[$package->getName()] = $package;
                 $removeMap[$package->getName()] = $package;
             }
+        }
 
-        $lockedPackages = array();
+        $stack = $this->getRootPackages();
-        foreach ($this->lockedRepository->getPackages() as $package) {
+
+        $visited = array();
+        $processed = array();
+
+        while (!empty($stack)) {
+            $package = array_pop($stack);
+
+            if (isset($processed[$package->id])) {
+                continue;
+            }
+
+            if (!isset($visited[$package->id])) {
+                $visited[$package->id] = true;
+
+                $stack[] = $package;
+                if ($package instanceof AliasPackage) {
+                    $stack[] = $package->getAliasOf();
+                } else {
+                    foreach ($package->getRequires() as $link) {
+                        $possibleRequires = $this->getLockedProviders($link);
+
+                        foreach ($possibleRequires as $require) {
+                            $stack[] = $require;
+                        }
+                    }
+                }
+            } elseif (!isset($processed[$package->id])) {
+                $processed[$package->id] = true;
+
+                if ($package instanceof AliasPackage) {
+                    $aliasKey = $package->getName().'::'.$package->getVersion();
+                    if (isset($localAliasMap[$aliasKey])) {
+                        unset($removeAliasMap[$aliasKey]);
+                    } else {
+                        $operations[] = new Operation\MarkAliasInstalledOperation($package);
+                    }
+                } else {
                     if (isset($localPackageMap[$package->getName()])) {
                         $source = $localPackageMap[$package->getName()];
 
@@ -72,21 +138,20 @@ class LocalRepoTransaction
                         $operations[] = new Operation\InstallOperation($package);
                         unset($removeMap[$package->getName()]);
                     }
-/*
-            if (isset($lockedPackages[$package->getName()])) {
-                die("Alias?");
                 }
-            $lockedPackages[$package->getName()] = $package;*/
+            }
         }
 
         foreach ($removeMap as $name => $package) {
-            $operations[] = new Operation\UninstallOperation($package, null);
+            array_unshift($operations, new Operation\UninstallOperation($package, null));
+        }
+        foreach ($removeAliasMap as $nameVersion => $package) {
+            $operations[] = new Operation\MarkAliasUninstalledOperation($package, null);
         }
 
-        $operations = $this->sortOperations($operations);
         $operations = $this->movePluginsToFront($operations);
         // TODO fix this:
-        // we have to do this again here even though sortOperations did it because moving plugins moves them before uninstalls
+        // we have to do this again here even though the above stack code did it because moving plugins moves them before uninstalls
         $operations = $this->moveUninstallsToFront($operations);
 
         // TODO skip updates which don't update? is this needed? we shouldn't schedule this update in the first place?
@@ -110,97 +175,43 @@ class LocalRepoTransaction
         return $operations;
     }
 
-    // TODO is there a more efficient / better way to do get a "good" install order?
+    /**
-    public function sortOperations(array $operations)
+     * Determine which packages in the lock file are not required by any other packages in the lock file.
+     *
+     * These serve as a starting point to enumerate packages in a topological order despite potential cycles.
+     * If there are packages with a cycle on the top level the package with the lowest name gets picked
+     *
+     * @return array
+     */
+    private function getRootPackages()
     {
-        $packageQueue = $this->lockedRepository->getPackages();
+        $roots = $this->lockedPackages;
 
-        $packageQueue[] = null; // null is a cycle marker
+        foreach ($this->lockedPackages as $packageId => $package) {
-
+            if (!isset($roots[$packageId])) {
-        $weights = array();
-        $foundWeighables = false;
-
-        // This is sort of a topological sort, the weight represents the distance from a leaf (1 == is leaf)
-        // Since we can have cycles in the dep graph, any node which doesn't have an acyclic connection to all
-        // leaves it's connected to, cannot be assigned a weight and will be unsorted
-        while (!empty($packageQueue)) {
-            $package = array_shift($packageQueue);
-
-            // one full cycle
-            if ($package === null) {
-                // if we were able to assign some weights, keep going
-                if ($foundWeighables) {
-                    $foundWeighables = false;
-                    $packageQueue[] = null;
                 continue;
-                } else {
-                    foreach ($packageQueue as $package) {
-                        $weights[$package->getName()] = PHP_INT_MAX;
             }
-                    // no point in continuing, we are in a cycle
+
-                    break;
+            foreach ($package->getRequires() as $link) {
+                $possibleRequires = $this->getLockedProviders($link);
+
+                foreach ($possibleRequires as $require) {
+                    if ($require !== $package) {
+                        unset($roots[$require->id]);
+                    }
+                }
             }
         }
 
-            $requires = array_filter(array_keys($package->getRequires()), function ($req) {
+        return $roots;
-                return $req !== 'composer-plugin-api' && !preg_match(PlatformRepository::PLATFORM_PACKAGE_REGEX, $req);
-            });
-
-            $maxWeight = 0;
-            foreach ($requires as $require) {
-                if (!isset($weights[$require])) {
-                    $maxWeight = null;
-
-                    // needs more calculation, so add to end of queue
-                    $packageQueue[] = $package;
-                    break;
     }
 
-                $maxWeight = max((int) $maxWeight, $weights[$require]);
+    private function getLockedProviders(Link $link)
+    {
+        if (!isset($this->lockedPackagesByName[$link->getTarget()])) {
+            return array();
         }
-            if ($maxWeight !== null) {
+        return $this->lockedPackagesByName[$link->getTarget()];
-                $foundWeighables = true;
-                $weights[$package->getName()] = $maxWeight + 1;
-            }
-        }
-
-        // TODO do we have any alias ops in the local repo transaction?
-        usort($operations, function ($opA, $opB) use ($weights) {
-            // uninstalls come first, if there are multiple, sort by name
-            if ($opA instanceof Operation\UninstallOperation) {
-                $packageA = $opA->getPackage();
-                if ($opB instanceof Operation\UninstallOperation) {
-                    return strcmp($packageA->getName(), $opB->getPackage()->getName());
-                }
-                return -1;
-            } elseif ($opB instanceof Operation\UninstallOperation) {
-                return 1;
-            }
-
-
-            if ($opA instanceof Operation\InstallOperation) {
-                $packageA = $opA->getPackage();
-            } elseif ($opA instanceof Operation\UpdateOperation) {
-                $packageA = $opA->getTargetPackage();
-            }
-
-            if ($opB instanceof Operation\InstallOperation) {
-                $packageB = $opB->getPackage();
-            } elseif ($opB instanceof Operation\UpdateOperation) {
-                $packageB = $opB->getTargetPackage();
-            }
-
-            $weightA = $weights[$packageA->getName()];
-            $weightB = $weights[$packageB->getName()];
-
-            if ($weightA === $weightB) {
-                return strcmp($packageA->getName(),  $packageB->getName());
-            } else {
-                return $weightA < $weightB ? -1 : 1;
-            }
-        });
-
-        return $operations;
     }
 
     /**

src/Composer/Installer.php

@@ -575,6 +575,12 @@ class Installer
 
             // TODO should we warn people / error if plugins in vendor folder do not match contents of lock file before update?
             //$this->eventDispatcher->dispatchInstallerEvent(InstallerEvents::POST_DEPENDENCIES_SOLVING, $this->devMode, $policy, $repositorySet, $installedRepo, $request, $lockTransaction);
+        } else {
+            // we still need to ensure all packages have an id for correct functionality
+            $id = 1;
+            foreach ($lockedRepository->getPackages() as $package) {
+                $package->id = $id++;
+            }
         }
 
         // TODO in how far do we need to do anything here to ensure dev packages being updated to latest in lock without version change are treated correctly?