diff --git a/src/Composer/DependencyResolver/DefaultPolicy.php b/src/Composer/DependencyResolver/DefaultPolicy.php index d59a07d55..6be7dcc23 100644 --- a/src/Composer/DependencyResolver/DefaultPolicy.php +++ b/src/Composer/DependencyResolver/DefaultPolicy.php @@ -60,14 +60,14 @@ class DefaultPolicy implements PolicyInterface return $pool->getPriority($package->getRepository()); } - public function selectPreferedPackages(Pool $pool, array $installedMap, array $literals) + public function selectPreferedPackages(Pool $pool, array $installedMap, array $literals, Rule $rule = null) { $packages = $this->groupLiteralsByNamePreferInstalled($pool, $installedMap, $literals); foreach ($packages as &$literals) { $policy = $this; - usort($literals, function ($a, $b) use ($policy, $pool, $installedMap) { - return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b), true); + usort($literals, function ($a, $b) use ($policy, $pool, $installedMap, $rule) { + return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b), $rule, true); }); } @@ -82,8 +82,8 @@ class DefaultPolicy implements PolicyInterface $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, $installedMap) { - return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b)); + usort($selected, function ($a, $b) use ($policy, $pool, $installedMap, $rule) { + return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b), $rule); }); return $selected; @@ -109,7 +109,10 @@ class DefaultPolicy implements PolicyInterface return $packages; } - public function compareByPriorityPreferInstalled(Pool $pool, array $installedMap, PackageInterface $a, PackageInterface $b, $ignoreReplace = false) + /** + * @protected + */ + public function compareByPriorityPreferInstalled(Pool $pool, array $installedMap, PackageInterface $a, PackageInterface $b, Rule $rule = null, $ignoreReplace = false) { if ($a->getRepository() === $b->getRepository()) { // prefer aliases to the original package @@ -132,6 +135,26 @@ class DefaultPolicy implements PolicyInterface 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 ($rule) { + if ($rule->getReason() === Rule::RULE_JOB_INSTALL) { + $required = $rule->getReasonData(); + } elseif ($rule->getReason() === Rule::RULE_PACKAGE_REQUIRES) { + $required = $rule->getReasonData()->getTarget(); + } + + if ($required && (false !== ($pos = strpos($required, '/')))) { + $requiredVendor = substr($required, 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 diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index d249f6d37..13c253848 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -35,6 +35,8 @@ class Rule protected $literals; protected $type; protected $id; + protected $reason; + protected $reasonData; protected $job; @@ -80,6 +82,16 @@ class Rule return $this->job; } + public function getReason() + { + return $this->reason; + } + + public function getReasonData() + { + return $this->reasonData; + } + /** * Checks if this rule is equal to another one * diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 3c46c0c41..05bb0b400 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -321,7 +321,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->installedMap, $decisionQueue); + $literals = $this->policy->selectPreferedPackages($this->pool, $this->installedMap, $decisionQueue, $rule); $selectedLiteral = array_shift($literals); diff --git a/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php b/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php index 8b82cc54d..3a17bf7ba 100644 --- a/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php +++ b/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php @@ -16,6 +16,7 @@ use Composer\Repository\ArrayRepository; use Composer\Repository\RepositoryInterface; use Composer\DependencyResolver\DefaultPolicy; use Composer\DependencyResolver\Pool; +use Composer\DependencyResolver\Rule; use Composer\Package\Link; use Composer\Package\AliasPackage; use Composer\Package\LinkConstraint\VersionConstraint; @@ -177,7 +178,6 @@ class DefaultPolicyTest extends TestCase public function testPreferNonReplacingFromSameRepo() { - $this->repo->addPackage($packageA = $this->getPackage('A', '1.0')); $this->repo->addPackage($packageB = $this->getPackage('B', '2.0')); @@ -193,6 +193,32 @@ class DefaultPolicyTest extends TestCase $this->assertEquals($expected, $selected); } + public function testPreferReplacingPackageFromSameVendor() + { + $this->repo->addPackage($packageB = $this->getPackage('vendor-b/replacer', '1.0')); + $this->repo->addPackage($packageA = $this->getPackage('vendor-a/replacer', '1.0')); + + $packageA->setReplaces(array(new Link('vendor-a/replacer', 'vendor-a/package', new VersionConstraint('==', '1.0'), 'replaces'))); + $packageB->setReplaces(array(new Link('vendor-b/replacer', 'vendor-a/package', new VersionConstraint('==', '1.0'), 'replaces'))); + + $this->pool->addRepository($this->repo); + + $literals = array($packageA->getId(), $packageB->getId()); + $expected = $literals; + + // test with install rule + $rule = new Rule($this->pool, $literals, Rule::RULE_JOB_INSTALL, 'vendor-a/package'); + $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals, $rule); + + $this->assertEquals($expected, $selected); + + // test with requires rule + $rule = new Rule($this->pool, $literals, Rule::RULE_PACKAGE_REQUIRES, new Link('foo', 'vendor-a/package')); + $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals, $rule); + + $this->assertEquals($expected, $selected); + } + protected function mapFromRepo(RepositoryInterface $repo) { $map = array(); diff --git a/tests/Composer/Test/Fixtures/installer/replace-vendor-priorities.test b/tests/Composer/Test/Fixtures/installer/replace-vendor-priorities.test new file mode 100644 index 000000000..86c491feb --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/replace-vendor-priorities.test @@ -0,0 +1,21 @@ +--TEST-- +Replacer of the same vendor takes precedence if same prio repo +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "b/replacer", "version": "1.1.0", "replace": { "a/package": "1.1.0" } }, + { "name": "a/replacer", "version": "1.1.0", "replace": { "a/package": "1.1.0" } } + ] + } + ], + "require": { + "a/package": "1.*" + } +} +--RUN-- +install +--EXPECT-- +Installing a/replacer (1.1.0)