From b700aa3d622895a79a7334b751b462c28a72b385 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 7 Sep 2019 06:03:39 +0200 Subject: [PATCH] Sort local repo transaction as topological as possible --- .../LocalRepoTransaction.php | 100 +++++++++++++++++- tests/Composer/Test/InstallerTest.php | 2 +- 2 files changed, 98 insertions(+), 4 deletions(-) diff --git a/src/Composer/DependencyResolver/LocalRepoTransaction.php b/src/Composer/DependencyResolver/LocalRepoTransaction.php index db224989d..8a38248f7 100644 --- a/src/Composer/DependencyResolver/LocalRepoTransaction.php +++ b/src/Composer/DependencyResolver/LocalRepoTransaction.php @@ -72,8 +72,6 @@ class LocalRepoTransaction $operations[] = new Operation\InstallOperation($package); unset($removeMap[$package->getName()]); } - - /* if (isset($lockedPackages[$package->getName()])) { die("Alias?"); @@ -85,7 +83,10 @@ class LocalRepoTransaction $operations[] = new Operation\UninstallOperation($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 $operations = $this->moveUninstallsToFront($operations); // TODO skip updates which don't update? is this needed? we shouldn't schedule this update in the first place? @@ -109,6 +110,99 @@ class LocalRepoTransaction return $operations; } + // TODO is there a more efficient / better way to do get a "good" install order? + public function sortOperations(array $operations) + { + $packageQueue = $this->lockedRepository->getPackages(); + + $packageQueue[] = null; // null is a cycle marker + + $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; + } + } + + $requires = array_filter(array_keys($package->getRequires()), function ($req) { + 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]); + } + if ($maxWeight !== null) { + $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; + } + /** * Workaround: if your packages depend on plugins, we must be sure * that those are installed / updated first; else it would lead to packages @@ -176,7 +270,7 @@ class LocalRepoTransaction { $uninstOps = array(); foreach ($operations as $idx => $op) { - if ($op instanceof UninstallOperation || $op instanceof MarkAliasUninstalledOperation) { + if ($op instanceof UninstallOperation) { $uninstOps[] = $op; unset($operations[$idx]); } diff --git a/tests/Composer/Test/InstallerTest.php b/tests/Composer/Test/InstallerTest.php index 89385c288..ce87d111f 100644 --- a/tests/Composer/Test/InstallerTest.php +++ b/tests/Composer/Test/InstallerTest.php @@ -97,7 +97,7 @@ class InstallerTest extends TestCase })); $tempLockData = null; - $locker = new Locker($io, $lockJsonMock, $repositoryManager, $installationManager, '{}'); + $locker = new Locker($io, $lockJsonMock, $installationManager, '{}'); $autoloadGenerator = $this->getMockBuilder('Composer\Autoload\AutoloadGenerator')->disableOriginalConstructor()->getMock();