From 4140f08d9c48e80b95c0dcc6121c65aa25ed0ba4 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 19 Nov 2011 00:27:35 +0100 Subject: [PATCH] Use a map of installed package ids in the solver The assumption package.repo == installed no longer holds for installed packages because there are multiple wrapped installed repositories. --- .../DependencyResolver/DefaultPolicy.php | 37 ++++++++------- .../DependencyResolver/PolicyInterface.php | 6 +-- src/Composer/DependencyResolver/Pool.php | 8 +++- src/Composer/DependencyResolver/Solver.php | 46 +++++++++---------- .../DependencyResolver/DefaultPolicyTest.php | 26 ++++++++--- 5 files changed, 72 insertions(+), 51 deletions(-) diff --git a/src/Composer/DependencyResolver/DefaultPolicy.php b/src/Composer/DependencyResolver/DefaultPolicy.php index 0c5efe963..fb6c219a7 100644 --- a/src/Composer/DependencyResolver/DefaultPolicy.php +++ b/src/Composer/DependencyResolver/DefaultPolicy.php @@ -39,7 +39,7 @@ class DefaultPolicy implements PolicyInterface return $constraint->matchSpecific($version); } - public function findUpdatePackages(Solver $solver, Pool $pool, RepositoryInterface $repo, PackageInterface $package, $allowAll = false) + public function findUpdatePackages(Solver $solver, Pool $pool, array $installedMap, PackageInterface $package, $allowAll = false) { $packages = array(); @@ -57,7 +57,7 @@ class DefaultPolicy implements PolicyInterface return $packages; } - public function installable(Solver $solver, Pool $pool, RepositoryInterface $repo, PackageInterface $package) + public function installable(Solver $solver, Pool $pool, array $installedMap, PackageInterface $package) { // todo: package blacklist? return true; @@ -68,34 +68,34 @@ class DefaultPolicy implements PolicyInterface return $pool->getPriority($package->getRepository()); } - public function selectPreferedPackages(Pool $pool, RepositoryInterface $installed, array $literals) + public function selectPreferedPackages(Pool $pool, array $installedMap, array $literals) { - $packages = $this->groupLiteralsByNamePreferInstalled($installed, $literals); + $packages = $this->groupLiteralsByNamePreferInstalled($installedMap, $literals); foreach ($packages as &$literals) { $policy = $this; - usort($literals, function ($a, $b) use ($policy, $pool, $installed) { - return $policy->compareByPriorityPreferInstalled($pool, $installed, $a->getPackage(), $b->getPackage(), true); + usort($literals, function ($a, $b) use ($policy, $pool, $installedMap) { + return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $a->getPackage(), $b->getPackage(), true); }); } foreach ($packages as &$literals) { $literals = $this->pruneToBestVersion($literals); - $literals = $this->pruneToHighestPriorityOrInstalled($pool, $installed, $literals); + $literals = $this->pruneToHighestPriorityOrInstalled($pool, $installedMap, $literals); } $selected = call_user_func_array('array_merge', $packages); // now sort the result across all packages to respect replaces across packages - usort($selected, function ($a, $b) use ($policy, $pool, $installed) { - return $policy->compareByPriorityPreferInstalled($pool, $installed, $a->getPackage(), $b->getPackage()); + usort($selected, function ($a, $b) use ($policy, $pool, $installedMap) { + return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $a->getPackage(), $b->getPackage()); }); return $selected; } - protected function groupLiteralsByNamePreferInstalled(RepositoryInterface $installed, $literals) + protected function groupLiteralsByNamePreferInstalled(array $installedMap, $literals) { $packages = array(); foreach ($literals as $literal) { @@ -105,7 +105,7 @@ class DefaultPolicy implements PolicyInterface $packages[$packageName] = array(); } - if ($literal->getPackage()->getRepository() === $installed) { + if (isset($installedMap[$literal->getPackageId()])) { array_unshift($packages[$packageName], $literal); } else { $packages[$packageName][] = $literal; @@ -115,7 +115,7 @@ class DefaultPolicy implements PolicyInterface return $packages; } - public function compareByPriorityPreferInstalled(Pool $pool, RepositoryInterface $installed, PackageInterface $a, PackageInterface $b, $ignoreReplace = false) + public function compareByPriorityPreferInstalled(Pool $pool, array $installedMap, PackageInterface $a, PackageInterface $b, $ignoreReplace = false) { if ($a->getRepository() === $b->getRepository()) { @@ -137,11 +137,11 @@ class DefaultPolicy implements PolicyInterface return ($a->getId() < $b->getId()) ? -1 : 1; } - if ($a->getRepository() === $installed) { + if (isset($installedMap[$a->getId()])) { return -1; } - if ($b->getRepository() === $installed) { + if (isset($installedMap[$b->getId()])) { return 1; } @@ -181,7 +181,7 @@ class DefaultPolicy implements PolicyInterface return $bestLiterals; } - protected function selectNewestPackages(RepositoryInterface $installed, array $literals) + protected function selectNewestPackages(array $installedMap, array $literals) { $maxLiterals = array($literals[0]); $maxPackage = $literals[0]->getPackage(); @@ -201,7 +201,10 @@ class DefaultPolicy implements PolicyInterface return $maxLiterals; } - protected function pruneToHighestPriorityOrInstalled(Pool $pool, RepositoryInterface $installed, array $literals) + /** + * Assumes that installed packages come first and then all highest priority packages + */ + protected function pruneToHighestPriorityOrInstalled(Pool $pool, array $installedMap, array $literals) { $selected = array(); @@ -210,7 +213,7 @@ class DefaultPolicy implements PolicyInterface foreach ($literals as $literal) { $package = $literal->getPackage(); - if ($package->getRepository() === $installed) { + if (isset($installedMap[$package->getId()])) { $selected[] = $literal; continue; } diff --git a/src/Composer/DependencyResolver/PolicyInterface.php b/src/Composer/DependencyResolver/PolicyInterface.php index 41929c2ce..0271fdab2 100644 --- a/src/Composer/DependencyResolver/PolicyInterface.php +++ b/src/Composer/DependencyResolver/PolicyInterface.php @@ -23,7 +23,7 @@ interface PolicyInterface function allowUninstall(); function allowDowngrade(); function versionCompare(PackageInterface $a, PackageInterface $b, $operator); - function findUpdatePackages(Solver $solver, Pool $pool, RepositoryInterface $repo, PackageInterface $package, $allowAll); - function installable(Solver $solver, Pool $pool, RepositoryInterface $repo, PackageInterface $package); - function selectPreferedPackages(Pool $pool, RepositoryInterface $installed, array $literals); + function findUpdatePackages(Solver $solver, Pool $pool, array $installedMap, PackageInterface $package, $allowAll); + function installable(Solver $solver, Pool $pool, array $installedMap, PackageInterface $package); + function selectPreferedPackages(Pool $pool, array $installedMap, array $literals); } diff --git a/src/Composer/DependencyResolver/Pool.php b/src/Composer/DependencyResolver/Pool.php index 7d95f4d15..b5eeb26e7 100644 --- a/src/Composer/DependencyResolver/Pool.php +++ b/src/Composer/DependencyResolver/Pool.php @@ -47,7 +47,13 @@ class Pool public function getPriority(RepositoryInterface $repo) { - return array_search($repo, $this->repositories, true); + $priority = array_search($repo, $this->repositories, true); + + if (false === $priority) { + throw new \RuntimeException("Could not determine repository priority. The repository was not registered in the pool."); + } + + return $priority; } /** diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index f864273df..ba15144cd 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -50,7 +50,7 @@ class Solver protected $watches = array(); protected $removeWatches = array(); protected $decisionMap; - protected $installedPackageMap; + protected $installedMap; protected $packageToUpdateRule = array(); protected $packageToFeatureRule = array(); @@ -253,11 +253,11 @@ class Solver $this->addedMap[$package->getId()] = true; $dontFix = 0; - if (isset($this->installedPackageMap[$package->getId()]) && !isset($this->fixMap[$package->getId()])) { + if (isset($this->installedMap[$package->getId()]) && !isset($this->fixMap[$package->getId()])) { $dontFix = 1; } - if (!$dontFix && !$this->policy->installable($this, $this->pool, $this->installed, $package)) { + if (!$dontFix && !$this->policy->installable($this, $this->pool, $this->installedMap, $package)) { $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRemoveRule($package, self::RULE_NOT_INSTALLABLE, (string) $package)); continue; } @@ -272,7 +272,7 @@ class Solver if ($dontFix) { $foundInstalled = false; foreach ($possibleRequires as $require) { - if (isset($this->installedPackageMap[$require->getId()])) { + if (isset($this->installedMap[$require->getId()])) { $foundInstalled = true; break; } @@ -284,7 +284,7 @@ class Solver } } - $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRequireRule($package, $possibleRequires, self::RULE_PACKAGE_REQUIRES, (string) $link)); + $this->addRule(RuleSet::TYPE_PACKAGE, $rule = $this->createRequireRule($package, $possibleRequires, self::RULE_PACKAGE_REQUIRES, (string) $link)); foreach ($possibleRequires as $require) { $workQueue->enqueue($require); @@ -295,7 +295,7 @@ class Solver $possibleConflicts = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); foreach ($possibleConflicts as $conflict) { - if ($dontFix && isset($this->installedPackageMap[$conflict->getId()])) { + if ($dontFix && isset($this->installedMap[$conflict->getId()])) { continue; } @@ -310,7 +310,7 @@ class Solver /** @TODO: if ($this->noInstalledObsoletes) */ if (true) { $noObsoletes = isset($this->noObsoletes[$package->getId()]); - $isInstalled = (isset($this->installedPackageMap[$package->getId()])); + $isInstalled = (isset($this->installedMap[$package->getId()])); foreach ($package->getReplaces() as $link) { $obsoleteProviders = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); @@ -320,7 +320,7 @@ class Solver continue; } - if ($isInstalled && $dontFix && $this->installed === $provider->getRepository()) { + if ($isInstalled && $dontFix && isset($this->installedMap[$provider->getId()])) { continue; // don't repair installed/installed problems } @@ -341,7 +341,7 @@ class Solver continue; } - if ($isInstalled && $this->installed !== $provider->getRepository()) { + if ($isInstalled && !isset($this->installedMap[$provider->getId()])) { continue; } @@ -379,7 +379,7 @@ class Solver */ private function addRulesForUpdatePackages(PackageInterface $package, $allowAll) { - $updates = $this->policy->findUpdatePackages($this, $this->pool, $this->installed, $package, $allowAll); + $updates = $this->policy->findUpdatePackages($this, $this->pool, $this->installedMap, $package, $allowAll); $this->addRulesForPackage($package); @@ -759,7 +759,7 @@ class Solver switch ($job['cmd']) { case 'install': foreach ($job['packages'] as $package) { - if (isset($this->installedPackageMap[$package->getId()])) { + if (isset($this->installedMap[$package->getId()])) { $disableQueue[] = array('type' => 'update', 'package' => $package); } @@ -872,7 +872,7 @@ class Solver case 'remove': foreach ($job['packages'] as $package) { - if (isset($this->installedPackageMap[$package->getId()])) { + if (isset($this->installedMap[$package->getId()])) { $disableQueue[] = array('type' => 'update', 'package' => $package); } } @@ -934,9 +934,9 @@ class Solver { $this->jobs = $request->getJobs(); $installedPackages = $this->installed->getPackages(); - $this->installedPackageMap = array(); + $this->installedMap = array(); foreach ($installedPackages as $package) { - $this->installedPackageMap[$package->getId()] = $package; + $this->installedMap[$package->getId()] = $package; } $this->decisionMap = new \SplFixedArray($this->pool->getMaxId() + 1); @@ -959,12 +959,12 @@ class Solver foreach ($job['packages'] as $package) { switch ($job['cmd']) { case 'fix': - if (isset($this->installedPackageMap[$package->getId()])) { + if (isset($this->installedMap[$package->getId()])) { $this->fixMap[$package->getId()] = true; } break; case 'update': - if (isset($this->installedPackageMap[$package->getId()])) { + if (isset($this->installedMap[$package->getId()])) { $this->updateMap[$package->getId()] = true; } break; @@ -996,11 +996,11 @@ class Solver foreach ($installedPackages as $package) { // create a feature rule which allows downgrades - $updates = $this->policy->findUpdatePackages($this, $this->pool, $this->installed, $package, true); + $updates = $this->policy->findUpdatePackages($this, $this->pool, $this->installedMap, $package, true); $featureRule = $this->createUpdateRule($package, $updates, self::RULE_INTERNAL_ALLOW_UPDATE, (string) $package); // create an update rule which does not allow downgrades - $updates = $this->policy->findUpdatePackages($this, $this->pool, $this->installed, $package, false); + $updates = $this->policy->findUpdatePackages($this, $this->pool, $this->installedMap, $package, false); $rule = $this->createUpdateRule($package, $updates, self::RULE_INTERNAL_ALLOW_UPDATE, (string) $package); if ($rule->equals($featureRule)) { @@ -1044,7 +1044,7 @@ class Solver break; case 'lock': foreach ($job['packages'] as $package) { - if (isset($this->installedPackageMap[$package->getId()])) { + if (isset($this->installedMap[$package->getId()])) { $rule = $this->createInstallRule($package, self::RULE_JOB_LOCK); } else { $rule = $this->createRemoveRule($package, self::RULE_JOB_LOCK); @@ -1088,7 +1088,7 @@ class Solver $package = $literal->getPackage(); // !wanted & installed - if (!$literal->isWanted() && isset($this->installedPackageMap[$package->getId()])) { + if (!$literal->isWanted() && isset($this->installedMap[$package->getId()])) { $literals = array(); if (isset($this->packageToUpdateRule[$package->getId()])) { @@ -1110,7 +1110,7 @@ class Solver $package = $literal->getPackage(); // wanted & installed || !wanted & !installed - if ($literal->isWanted() == (isset($this->installedPackageMap[$package->getId()]))) { + if ($literal->isWanted() == (isset($this->installedMap[$package->getId()]))) { continue; } @@ -1418,7 +1418,7 @@ class Solver private function selectAndInstall($level, array $decisionQueue, $disableRules, Rule $rule) { // choose best package to install from decisionQueue - $literals = $this->policy->selectPreferedPackages($this->pool, $this->installed, $decisionQueue); + $literals = $this->policy->selectPreferedPackages($this->pool, $this->installedMap, $decisionQueue); $selectedLiteral = array_shift($literals); @@ -1779,7 +1779,7 @@ class Solver if (count($this->installed) != count($this->updateMap)) { $prunedQueue = array(); foreach ($decisionQueue as $literal) { - if ($this->installed === $literal->getPackage()->getRepository()) { + if (isset($this->installedMap[$literal->getPackageId()])) { $prunedQueue[] = $literal; if (isset($this->updateMap[$literal->getPackageId()])) { $prunedQueue = $decisionQueue; diff --git a/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php b/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php index 1688bb799..6975cf42e 100644 --- a/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php +++ b/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php @@ -13,6 +13,7 @@ namespace Composer\Test\DependencyResolver; use Composer\Repository\ArrayRepository; +use Composer\Repository\RepositoryInterface; use Composer\DependencyResolver\DefaultPolicy; use Composer\DependencyResolver\Pool; use Composer\DependencyResolver\Literal; @@ -45,7 +46,7 @@ class DefaultPolicyTest extends \PHPUnit_Framework_TestCase $literals = array(new Literal($packageA, true)); $expected = array(new Literal($packageA, true)); - $selected = $this->policy->selectPreferedPackages($this->pool, $this->repoInstalled, $literals); + $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals); $this->assertEquals($expected, $selected); } @@ -59,7 +60,7 @@ class DefaultPolicyTest extends \PHPUnit_Framework_TestCase $literals = array(new Literal($packageA1, true), new Literal($packageA2, true)); $expected = array(new Literal($packageA2, true)); - $selected = $this->policy->selectPreferedPackages($this->pool, $this->repoInstalled, $literals); + $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals); $this->assertEquals($expected, $selected); } @@ -68,13 +69,13 @@ class DefaultPolicyTest extends \PHPUnit_Framework_TestCase { $this->repo->addPackage($packageA = new MemoryPackage('A', '2.0')); $this->repoInstalled->addPackage($packageAInstalled = new MemoryPackage('A', '1.0')); - $this->pool->addRepository($this->repo); $this->pool->addRepository($this->repoInstalled); + $this->pool->addRepository($this->repo); $literals = array(new Literal($packageA, true), new Literal($packageAInstalled, true)); $expected = array(new Literal($packageA, true)); - $selected = $this->policy->selectPreferedPackages($this->pool, $this->repoInstalled, $literals); + $selected = $this->policy->selectPreferedPackages($this->pool, $this->mapFromRepo($this->repoInstalled), $literals); $this->assertEquals($expected, $selected); } @@ -86,13 +87,14 @@ class DefaultPolicyTest extends \PHPUnit_Framework_TestCase $this->repo->addPackage($packageA = new MemoryPackage('A', '1.0')); $this->repoImportant->addPackage($packageAImportant = new MemoryPackage('A', '1.0')); + $this->pool->addRepository($this->repoInstalled); $this->pool->addRepository($this->repo); $this->pool->addRepository($this->repoImportant); $literals = array(new Literal($packageA, true), new Literal($packageAImportant, true)); $expected = array(new Literal($packageAImportant, true)); - $selected = $this->policy->selectPreferedPackages($this->pool, $this->repoInstalled, $literals); + $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals); $this->assertEquals($expected, $selected); } @@ -110,7 +112,7 @@ class DefaultPolicyTest extends \PHPUnit_Framework_TestCase $literals = array(new Literal($packageA, true), new Literal($packageB, true)); $expected = $literals; - $selected = $this->policy->selectPreferedPackages($this->pool, $this->repoInstalled, $literals); + $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals); $this->assertEquals($expected, $selected); } @@ -128,8 +130,18 @@ class DefaultPolicyTest extends \PHPUnit_Framework_TestCase $literals = array(new Literal($packageA, true), new Literal($packageB, true)); $expected = array(new Literal($packageA, true), new Literal($packageB, true)); - $selected = $this->policy->selectPreferedPackages($this->pool, $this->repoInstalled, $literals); + $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals); $this->assertEquals($expected, $selected); } + + protected function mapFromRepo(RepositoryInterface $repo) + { + $map = array(); + foreach ($repo->getPackages() as $package) { + $map[$package->getId()] = true; + } + + return $map; + } }