Browse Source

Remove package/repo priority concept as it is enforced by the pool builder now

Jordi Boggiano 5 years ago
parent
commit
f68731e663

+ 31 - 62
src/Composer/DependencyResolver/DefaultPolicy.php

@@ -69,7 +69,6 @@ class DefaultPolicy implements PolicyInterface
         }
 
         foreach ($packages as &$sortedLiterals) {
-            $sortedLiterals = $this->pruneToHighestPriority($pool, $sortedLiterals);
             $sortedLiterals = $this->pruneToBestVersion($pool, $sortedLiterals);
             $sortedLiterals = $this->pruneRemoteAliases($pool, $sortedLiterals);
         }
@@ -104,51 +103,47 @@ class DefaultPolicy implements PolicyInterface
      */
     public function compareByPriority(Pool $pool, PackageInterface $a, PackageInterface $b, $requiredPackage = null, $ignoreReplace = false)
     {
-        if ($a->getRepository() === $b->getRepository()) {
-            // prefer aliases to the original package
-            if ($a->getName() === $b->getName()) {
-                $aAliased = $a instanceof AliasPackage;
-                $bAliased = $b instanceof AliasPackage;
-                if ($aAliased && !$bAliased) {
-                    return -1; // use a
-                }
-                if (!$aAliased && $bAliased) {
-                    return 1; // use b
-                }
+        // prefer aliases to the original package
+        if ($a->getName() === $b->getName()) {
+            $aAliased = $a instanceof AliasPackage;
+            $bAliased = $b instanceof AliasPackage;
+            if ($aAliased && !$bAliased) {
+                return -1; // use a
             }
+            if (!$aAliased && $bAliased) {
+                return 1; // use b
+            }
+        }
 
-            if (!$ignoreReplace) {
-                // return original, not replaced
-                if ($this->replaces($a, $b)) {
-                    return 1; // use b
-                }
-                if ($this->replaces($b, $a)) {
-                    return -1; // use a
-                }
+        if (!$ignoreReplace) {
+            // return original, not replaced
+            if ($this->replaces($a, $b)) {
+                return 1; // use b
+            }
+            if ($this->replaces($b, $a)) {
+                return -1; // use a
+            }
 
-                // for replacers not replacing each other, put a higher prio on replacing
-                // packages with the same vendor as the required package
-                if ($requiredPackage && false !== ($pos = strpos($requiredPackage, '/'))) {
-                    $requiredVendor = substr($requiredPackage, 0, $pos);
+            // for replacers not replacing each other, put a higher prio on replacing
+            // packages with the same vendor as the required package
+            if ($requiredPackage && false !== ($pos = strpos($requiredPackage, '/'))) {
+                $requiredVendor = substr($requiredPackage, 0, $pos);
 
-                    $aIsSameVendor = substr($a->getName(), 0, $pos) === $requiredVendor;
-                    $bIsSameVendor = substr($b->getName(), 0, $pos) === $requiredVendor;
+                $aIsSameVendor = substr($a->getName(), 0, $pos) === $requiredVendor;
+                $bIsSameVendor = substr($b->getName(), 0, $pos) === $requiredVendor;
 
-                    if ($bIsSameVendor !== $aIsSameVendor) {
-                        return $aIsSameVendor ? -1 : 1;
-                    }
+                if ($bIsSameVendor !== $aIsSameVendor) {
+                    return $aIsSameVendor ? -1 : 1;
                 }
             }
+        }
 
-            // priority equal, sort by package id to make reproducible
-            if ($a->id === $b->id) {
-                return 0;
-            }
-
-            return ($a->id < $b->id) ? -1 : 1;
+        // priority equal, sort by package id to make reproducible
+        if ($a->id === $b->id) {
+            return 0;
         }
 
-        return ($pool->getPriority($a->id) > $pool->getPriority($b->id)) ? -1 : 1;
+        return ($a->id < $b->id) ? -1 : 1;
     }
 
     /**
@@ -198,32 +193,6 @@ class DefaultPolicy implements PolicyInterface
         return $bestLiterals;
     }
 
-    /**
-     * Assumes that highest priority packages come first
-     */
-    protected function pruneToHighestPriority(Pool $pool, array $literals)
-    {
-        $selected = array();
-
-        $priority = null;
-
-        foreach ($literals as $literal) {
-            $package = $pool->literalToPackage($literal);
-
-            if (null === $priority) {
-                $priority = $pool->getPriority($package->id);
-            }
-
-            if ($pool->getPriority($package->id) != $priority) {
-                break;
-            }
-
-            $selected[] = $literal;
-        }
-
-        return $selected;
-    }
-
     /**
      * Assumes that locally aliased (in root package requires) packages take priority over branch-alias ones
      *

+ 1 - 8
src/Composer/DependencyResolver/Pool.php

@@ -36,7 +36,6 @@ class Pool implements \Countable
     protected $packages = array();
     protected $packageByName = array();
     protected $packageByExactName = array();
-    protected $priorities = array();
     protected $versionParser;
     protected $providerCache = array();
 
@@ -45,13 +44,12 @@ class Pool implements \Countable
         $this->versionParser = new VersionParser;
     }
 
-    public function setPackages(array $packages, array $priorities = array())
+    public function setPackages(array $packages)
     {
         $id = 1;
 
         foreach ($packages as $i => $package) {
             $this->packages[] = $package;
-            $this->priorities[] = isset($priorities[$i]) ? $priorities[$i] : 0;
 
             $package->id = $id++;
             $names = $package->getNames();
@@ -63,11 +61,6 @@ class Pool implements \Countable
         }
     }
 
-    public function getPriority($id)
-    {
-        return $this->priorities[$id - 1];
-    }
-
     /**
      * Retrieves the package object for a given package id.
      *

+ 2 - 6
src/Composer/DependencyResolver/PoolBuilder.php

@@ -38,7 +38,6 @@ class PoolBuilder
     private $loadedNames = array();
 
     private $packages = array();
-    private $priorities = array();
 
     public function __construct($isPackageAcceptableCallable, array $rootRequires = array())
     {
@@ -134,7 +133,6 @@ class PoolBuilder
                 if (!$found) {
                     foreach ($aliasedPackages as $index => $packageOrAlias) {
                         unset($this->packages[$index]);
-                        unset($this->priorities[$index]);
                     }
                 }
             }
@@ -149,7 +147,7 @@ class PoolBuilder
             }
         }
 
-        $pool->setPackages($this->packages, $this->priorities);
+        $pool->setPackages($this->packages);
 
         unset($this->aliasMap);
         unset($this->loadedNames);
@@ -158,11 +156,10 @@ class PoolBuilder
         return $pool;
     }
 
-    private function loadPackage(Request $request, PackageInterface $package, $repoIndex)
+    private function loadPackage(Request $request, PackageInterface $package)
     {
         $index = count($this->packages);
         $this->packages[] = $package;
-        $this->priorities[] = -$repoIndex;
 
         if ($package instanceof AliasPackage) {
             $this->aliasMap[spl_object_hash($package->getAliasOf())][$index] = $package;
@@ -192,7 +189,6 @@ class PoolBuilder
 
             $package->getRepository()->addPackage($aliasPackage); // TODO do we need this?
             $this->packages[] = $aliasPackage;
-            $this->priorities[] = -$repoIndex;
             $this->aliasMap[spl_object_hash($aliasPackage->getAliasOf())][$index+1] = $aliasPackage;
         }
 

+ 3 - 11
src/Composer/Repository/RepositorySet.php

@@ -65,6 +65,9 @@ class RepositorySet
     /**
      * Adds a repository to this repository set
      *
+     * The first repos added have a higher priority. As soon as a package is found in any
+     * repository the search for that package ends, and following repos will not be consulted.
+     *
      * @param RepositoryInterface $repo        A package repository
      */
     public function addRepository(RepositoryInterface $repo)
@@ -134,17 +137,6 @@ class RepositorySet
         return $candidates;
     }
 
-    public function getPriority(RepositoryInterface $repo)
-    {
-        $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;
-    }
-
     /**
      * Create a pool for dependency resolution from the packages in this repository set.
      *

+ 3 - 40
tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php

@@ -13,6 +13,7 @@
 namespace Composer\Test\DependencyResolver;
 
 use Composer\Repository\ArrayRepository;
+use Composer\Repository\LockArrayRepository;
 use Composer\Repository\RepositoryInterface;
 use Composer\DependencyResolver\DefaultPolicy;
 use Composer\DependencyResolver\Pool;
@@ -28,7 +29,7 @@ class DefaultPolicyTest extends TestCase
     protected $repositorySet;
     /** @var ArrayRepository */
     protected $repo;
-    /** @var ArrayRepository */
+    /** @var LockArrayRepository */
     protected $repoLocked;
     /** @var DefaultPolicy */
     protected $policy;
@@ -37,7 +38,7 @@ class DefaultPolicyTest extends TestCase
     {
         $this->repositorySet = new RepositorySet(array(), array(), 'dev');
         $this->repo = new ArrayRepository;
-        $this->repoLocked = new ArrayRepository;
+        $this->repoLocked = new LockArrayRepository;
 
         $this->policy = new DefaultPolicy;
     }
@@ -122,44 +123,6 @@ class DefaultPolicyTest extends TestCase
         $this->assertSame($expected, $selected);
     }
 
-    public function testSelectNewestOverLocked()
-    {
-        $this->repo->addPackage($packageA = $this->getPackage('A', '2.0'));
-        $this->repoLocked->addPackage($packageAInstalled = $this->getPackage('A', '1.0'));
-        $this->repositorySet->addRepository($this->repo);
-        $this->repositorySet->addRepository($this->repoLocked);
-
-        $pool = $this->repositorySet->createPoolForPackage('A');
-
-        $literals = array($packageA->getId(), $packageAInstalled->getId());
-        $expected = array($packageA->getId());
-
-        $selected = $this->policy->selectPreferredPackages($pool, $literals);
-
-        $this->assertSame($expected, $selected);
-    }
-
-    public function testSelectFirstRepo()
-    {
-        $otherRepository = new ArrayRepository;
-
-        $this->repo->addPackage($packageA = $this->getPackage('A', '1.0'));
-        $otherRepository->addPackage($packageAImportant = $this->getPackage('A', '1.0'));
-
-        $this->repositorySet->addRepository($otherRepository);
-        $this->repositorySet->addRepository($this->repo);
-        $this->repositorySet->addRepository($this->repoLocked);
-
-        $pool = $this->repositorySet->createPoolForPackage('A');
-
-        $literals = array($packageA->getId(), $packageAImportant->getId());
-        $expected = array($packageAImportant->getId());
-
-        $selected = $this->policy->selectPreferredPackages($pool, $literals);
-
-        $this->assertSame($expected, $selected);
-    }
-
     public function testRepositoryOrderingAffectsPriority()
     {
         $repo1 = new ArrayRepository;