Merge pull request #1804 from Seldaek/replacerprio

Put a higher prio on replacers of the same vendor as the required package

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, $requiredPackage = null)
     {
         $packages = $this->groupLiteralsByNamePreferInstalled($pool, $installedMap, $literals);
 
         foreach ($packages as &$literals) {
             $policy = $this;
-            usort($literals, function ($a, $b) use ($policy, $pool, $installedMap) {
+            usort($literals, function ($a, $b) use ($policy, $pool, $installedMap, $requiredPackage) {
-                return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b), true);
+                return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b), $requiredPackage, 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) {
+        usort($selected, function ($a, $b) use ($policy, $pool, $installedMap, $requiredPackage) {
-            return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b));
+            return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b), $requiredPackage);
         });
 
         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, $requiredPackage = null, $ignoreReplace = false)
     {
         if ($a->getRepository() === $b->getRepository()) {
             // prefer aliases to the original package
@@ -132,6 +135,19 @@ 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 ($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

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,17 @@ class Rule
         return $this->job;
     }
 
+    public function getRequiredPackage()
+    {
+        if ($this->reason === self::RULE_JOB_INSTALL) {
+            return $this->reasonData;
+        }
+
+        if ($this->reason === self::RULE_PACKAGE_REQUIRES) {
+            return $this->reasonData->getTarget();
+        }
+    }
+
     /**
      * Checks if this rule is equal to another one
      *

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->getRequiredPackage());
 
         $selectedLiteral = array_shift($literals);
 

tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php

@@ -177,7 +177,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 +192,38 @@ class DefaultPolicyTest extends TestCase
         $this->assertEquals($expected, $selected);
     }
 
+    public function testPreferReplacingPackageFromSameVendor()
+    {
+        // test with default order
+        $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;
+
+        $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals, 'vendor-a/package');
+        $this->assertEquals($expected, $selected);
+
+        // test with reversed order in repo
+        $repo = new ArrayRepository;
+        $repo->addPackage($packageA = clone $packageA);
+        $repo->addPackage($packageB = clone $packageB);
+
+        $pool = new Pool('dev');
+        $pool->addRepository($this->repo);
+
+        $literals = array($packageA->getId(), $packageB->getId());
+        $expected = $literals;
+
+        $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals, 'vendor-a/package');
+        $this->assertEquals($expected, $selected);
+    }
+
     protected function mapFromRepo(RepositoryInterface $repo)
     {
         $map = array();

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)