From 67fde906665a9f7241985abaa9ce5ffab3159eb5 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 5 Jun 2012 01:05:41 +0200 Subject: [PATCH] Correctly sort operations within transactions using DFS Fixes #655 --- src/Composer/DependencyResolver/Solver.php | 7 + .../DependencyResolver/Transaction.php | 166 ++++++++++++++---- .../Test/DependencyResolver/SolverTest.php | 8 +- .../aliased-priority-conflicting.test | 4 +- .../Fixtures/installer/aliased-priority.test | 6 +- 5 files changed, 144 insertions(+), 47 deletions(-) diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 795c6d9ab..ff7e736df 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -171,6 +171,13 @@ class Solver $this->runSat(true); + // decide to remove everything that's installed and undecided + foreach ($this->installedMap as $packageId => $void) { + if ($this->decisions->undecided($packageId)) { + $this->decisions->decide(-$packageId, 1, null); + } + } + if ($this->problems) { throw new SolverProblemsException($this->problems); } diff --git a/src/Composer/DependencyResolver/Transaction.php b/src/Composer/DependencyResolver/Transaction.php index de1ee2192..ff801acd6 100644 --- a/src/Composer/DependencyResolver/Transaction.php +++ b/src/Composer/DependencyResolver/Transaction.php @@ -37,32 +37,42 @@ class Transaction public function getOperations() { - $installMeansUpdateMap = array(); + $installMeansUpdateMap = $this->findUpdates(); - foreach ($this->installedMap as $packageId => $void) { - if ($this->decisions->undecided($packageId)) { - $this->decisions->decide(-$packageId, 1, null); - } - } + $updateMap = array(); + $installMap = array(); + $uninstallMap = array(); foreach ($this->decisions as $i => $decision) { $literal = $decision[Decisions::DECISION_LITERAL]; + $reason = $decision[Decisions::DECISION_REASON]; + $package = $this->pool->literalToPackage($literal); - // !wanted & installed - if ($literal <= 0 && isset($this->installedMap[$package->getId()])) { - $updates = $this->policy->findUpdatePackages($this->pool, $this->installedMap, $package); + // wanted & installed || !wanted & !installed + if (($literal > 0) == (isset($this->installedMap[$package->getId()]))) { + continue; + } - $literals = array($package->getId()); + if ($literal > 0) { + if (isset($installMeansUpdateMap[abs($literal)]) && !$package instanceof AliasPackage) { - foreach ($updates as $update) { - $literals[] = $update->getId(); - } + $source = $installMeansUpdateMap[abs($literal)]; - foreach ($literals as $updateLiteral) { - if ($updateLiteral !== $literal) { - $installMeansUpdateMap[abs($updateLiteral)] = $package; - } + $updateMap[$package->getId()] = array( + 'package' => $package, + 'source' => $source, + 'reason' => $reason, + ); + + // avoid updates to one package from multiple origins + unset($installMeansUpdateMap[abs($literal)]); + $ignoreRemove[$source->getId()] = true; + } else { + $installMap[$package->getId()] = array( + 'package' => $package, + 'reason' => $reason, + ); } } } @@ -71,47 +81,127 @@ class Transaction $literal = $decision[Decisions::DECISION_LITERAL]; $package = $this->pool->literalToPackage($literal); - // wanted & installed || !wanted & !installed - if (($literal > 0) == (isset($this->installedMap[$package->getId()]))) { - continue; + if ($literal <= 0 && + isset($this->installedMap[$package->getId()]) && + !isset($ignoreRemove[$package->getId()])) { + $uninstallMap[$package->getId()] = array( + 'package' => $package, + 'reason' => $reason, + ); + } + } + + $this->transactionFromMaps($installMap, $updateMap, $uninstallMap); + + return $this->transaction; + } + + protected function transactionFromMaps($installMap, $updateMap, $uninstallMap) + { + $queue = array_map(function ($operation) { + return $operation['package']; + }, + $this->findRootPackages($installMap, $updateMap) + ); + + $visited = array(); + + while (!empty($queue)) { + $package = array_pop($queue); + $packageId = $package->getId(); + + if (!isset($visited[$packageId])) { + array_push($queue, $package); - if ($literal > 0) { if ($package instanceof AliasPackage) { - $this->markAliasInstalled($package, $decision[Decisions::DECISION_REASON]); - continue; + array_push($queue, $package->getAliasOf()); + } else { + foreach ($package->getRequires() as $link) { + $possibleRequires = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); + + foreach ($possibleRequires as $require) { + array_push($queue, $require); + } + } } - if (isset($installMeansUpdateMap[abs($literal)])) { + $visited[$package->getId()] = true; + } else { + if (isset($installMap[$packageId])) { + $this->install( + $installMap[$packageId]['package'], + $installMap[$packageId]['reason'] + ); + unset($installMap[$packageId]); + } + if (isset($updateMap[$packageId])) { + $this->update( + $updateMap[$packageId]['source'], + $updateMap[$packageId]['package'], + $updateMap[$packageId]['reason'] + ); + unset($updateMap[$packageId]); + } + } + } - $source = $installMeansUpdateMap[abs($literal)]; + foreach ($uninstallMap as $uninstall) { + $this->uninstall($uninstall['package'], $uninstall['reason']); + } + } - $this->update($source, $package, $decision[Decisions::DECISION_REASON]); + protected function findRootPackages($installMap, $updateMap) + { + $packages = $installMap + $updateMap; + $roots = $packages; - // avoid updates to one package from multiple origins - unset($installMeansUpdateMap[abs($literal)]); - $ignoreRemove[$source->getId()] = true; - } else { - $this->install($package, $decision[Decisions::DECISION_REASON]); + foreach ($packages as $packageId => $operation) { + $package = $operation['package']; + + if (!isset($roots[$packageId])) { + continue; + } + + foreach ($package->getRequires() as $link) { + $possibleRequires = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); + + foreach ($possibleRequires as $require) { + unset($roots[$require->getId()]); } } } + return $roots; + } + + protected function findUpdates() + { + $installMeansUpdateMap = array(); + foreach ($this->decisions as $i => $decision) { $literal = $decision[Decisions::DECISION_LITERAL]; $package = $this->pool->literalToPackage($literal); - // wanted & installed || !wanted & !installed - if (($literal > 0) == (isset($this->installedMap[$package->getId()]))) { - continue; - } + // !wanted & installed + if ($literal <= 0 && isset($this->installedMap[$package->getId()])) { + $updates = $this->policy->findUpdatePackages($this->pool, $this->installedMap, $package); + + $literals = array($package->getId()); - if ($literal <= 0 && !isset($ignoreRemove[$package->getId()])) { - $this->uninstall($package, $decision[Decisions::DECISION_REASON]); + foreach ($updates as $update) { + $literals[] = $update->getId(); + } + + foreach ($literals as $updateLiteral) { + if ($updateLiteral !== $literal) { + $installMeansUpdateMap[abs($updateLiteral)] = $package; + } + } } } - return $this->transaction; + return $installMeansUpdateMap; } protected function install($package, $reason) diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index f1d270f90..0cec2b50a 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/tests/Composer/Test/DependencyResolver/SolverTest.php @@ -505,8 +505,8 @@ class SolverTest extends TestCase $this->request->install('X'); $this->checkSolverResult(array( - array('job' => 'install', 'package' => $packageA), array('job' => 'install', 'package' => $newPackageB), + array('job' => 'install', 'package' => $packageA), array('job' => 'install', 'package' => $packageX), )); } @@ -548,9 +548,9 @@ class SolverTest extends TestCase $this->request->install('A'); $this->checkSolverResult(array( - array('job' => 'install', 'package' => $packageC), array('job' => 'install', 'package' => $packageB), array('job' => 'install', 'package' => $packageA), + array('job' => 'install', 'package' => $packageC), )); } @@ -718,8 +718,8 @@ class SolverTest extends TestCase $this->request->install('A', $this->getVersionConstraint('==', '1.1.0.0')); $this->checkSolverResult(array( - array('job' => 'install', 'package' => $packageB), array('job' => 'install', 'package' => $packageA2), + array('job' => 'install', 'package' => $packageB), array('job' => 'install', 'package' => $packageA2Alias), )); } @@ -741,9 +741,9 @@ class SolverTest extends TestCase $this->request->install('B'); $this->checkSolverResult(array( + array('job' => 'install', 'package' => $packageA), array('job' => 'install', 'package' => $packageAAlias), array('job' => 'install', 'package' => $packageB), - array('job' => 'install', 'package' => $packageA), )); } diff --git a/tests/Composer/Test/Fixtures/installer/aliased-priority-conflicting.test b/tests/Composer/Test/Fixtures/installer/aliased-priority-conflicting.test index 1ffa936b4..481604cc3 100644 --- a/tests/Composer/Test/Fixtures/installer/aliased-priority-conflicting.test +++ b/tests/Composer/Test/Fixtures/installer/aliased-priority-conflicting.test @@ -45,7 +45,7 @@ Aliases take precedence over default package even if default is selected --RUN-- install --EXPECT-- -Marking a/req (dev-master feat.f) as installed, alias of a/req (dev-feature-foo feat.f) Installing a/req (dev-feature-foo feat.f) -Installing a/b (dev-master) +Marking a/req (dev-master feat.f) as installed, alias of a/req (dev-feature-foo feat.f) Installing a/a (dev-master) +Installing a/b (dev-master) diff --git a/tests/Composer/Test/Fixtures/installer/aliased-priority.test b/tests/Composer/Test/Fixtures/installer/aliased-priority.test index 55da1ee71..0cde23361 100644 --- a/tests/Composer/Test/Fixtures/installer/aliased-priority.test +++ b/tests/Composer/Test/Fixtures/installer/aliased-priority.test @@ -47,9 +47,9 @@ Aliases take precedence over default package --RUN-- install --EXPECT-- -Marking a/b (1.0.x-dev forked) as installed, alias of a/b (dev-master forked) -Installing a/b (dev-master forked) +Installing a/c (dev-feature-foo feat.f) Marking a/c (dev-master feat.f) as installed, alias of a/c (dev-feature-foo feat.f) +Installing a/b (dev-master forked) Installing a/a (dev-master master) -Installing a/c (dev-feature-foo feat.f) Marking a/a (1.0.x-dev master) as installed, alias of a/a (dev-master master) +Marking a/b (1.0.x-dev forked) as installed, alias of a/b (dev-master forked)