Browse Source

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

Jordi Boggiano 12 years ago
parent
commit
db4055b778

+ 29 - 6
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

+ 12 - 0
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
      *

+ 1 - 1
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);
 

+ 27 - 1
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();

+ 21 - 0
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)