From f68731e6631f9a17ff011cf2d53abfeb9d04f81f Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 15 Jan 2020 14:52:44 +0100 Subject: [PATCH] Remove package/repo priority concept as it is enforced by the pool builder now --- .../DependencyResolver/DefaultPolicy.php | 105 ++++++------------ src/Composer/DependencyResolver/Pool.php | 9 +- .../DependencyResolver/PoolBuilder.php | 8 +- src/Composer/Repository/RepositorySet.php | 14 +-- .../DependencyResolver/DefaultPolicyTest.php | 43 +------ 5 files changed, 46 insertions(+), 133 deletions(-) diff --git a/src/Composer/DependencyResolver/DefaultPolicy.php b/src/Composer/DependencyResolver/DefaultPolicy.php index 7d241b482..0b978ec59 100644 --- a/src/Composer/DependencyResolver/DefaultPolicy.php +++ b/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 (!$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); - - $aIsSameVendor = substr($a->getName(), 0, $pos) === $requiredVendor; - $bIsSameVendor = substr($b->getName(), 0, $pos) === $requiredVendor; - - if ($bIsSameVendor !== $aIsSameVendor) { - return $aIsSameVendor ? -1 : 1; - } - } + if (!$aAliased && $bAliased) { + return 1; // use b } - - // priority equal, sort by package id to make reproducible - if ($a->id === $b->id) { - return 0; - } - - return ($a->id < $b->id) ? -1 : 1; } - return ($pool->getPriority($a->id) > $pool->getPriority($b->id)) ? -1 : 1; + 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); + + $aIsSameVendor = substr($a->getName(), 0, $pos) === $requiredVendor; + $bIsSameVendor = substr($b->getName(), 0, $pos) === $requiredVendor; + + 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; } /** @@ -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 * diff --git a/src/Composer/DependencyResolver/Pool.php b/src/Composer/DependencyResolver/Pool.php index 9470c6e35..ffb70c300 100644 --- a/src/Composer/DependencyResolver/Pool.php +++ b/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. * diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index c57e4fa43..05d029604 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/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; } diff --git a/src/Composer/Repository/RepositorySet.php b/src/Composer/Repository/RepositorySet.php index 21526b96f..6ed647297 100644 --- a/src/Composer/Repository/RepositorySet.php +++ b/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. * diff --git a/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php b/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php index b4df12310..827a9181d 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\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;