Browse Source

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.
Nils Adermann 13 years ago
parent
commit
4140f08d9c

+ 20 - 17
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;
             }

+ 3 - 3
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);
 }

+ 7 - 1
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;
     }
 
     /**

+ 23 - 23
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;

+ 19 - 7
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;
+    }
 }