Merge pull request #2733 from naderman/limit-replace-provide

Whitelist packages with names matching those specified before generating rules

src/Composer/DependencyResolver/DefaultPolicy.php

@@ -42,11 +42,11 @@ class DefaultPolicy implements PolicyInterface
         return $constraint->matchSpecific($version, true);
     }
 
-    public function findUpdatePackages(Pool $pool, array $installedMap, PackageInterface $package)
+    public function findUpdatePackages(Pool $pool, array $installedMap, PackageInterface $package, $mustMatchName = false)
     {
         $packages = array();
 
-        foreach ($pool->whatProvides($package->getName()) as $candidate) {
+        foreach ($pool->whatProvides($package->getName(), null, $mustMatchName) as $candidate) {
             if ($candidate !== $package) {
                 $packages[] = $candidate;
             }

src/Composer/DependencyResolver/Pool.php

@@ -50,6 +50,7 @@ class Pool
     protected $versionParser;
     protected $providerCache = array();
     protected $filterRequires;
+    protected $whitelist = null;
     protected $id = 1;
 
     public function __construct($minimumStability = 'stable', array $stabilityFlags = array(), array $filterRequires = array())
@@ -66,6 +67,11 @@ class Pool
         $this->filterRequires = $filterRequires;
     }
 
+    public function setWhitelist($whitelist)
+    {
+        $this->whitelist = $whitelist;
+    }
+
     /**
      * Adds a repository and its packages to this package pool
      *
@@ -223,21 +229,24 @@ class Pool
      * @param string                  $name       The package name to be searched for
      * @param LinkConstraintInterface $constraint A constraint that all returned
      *                                            packages must match or null to return all
+     * @param bool                    $mustMatchName Whether the name of returned packages
+     *                                            must match the given name
      * @return array A set of packages
      */
-    public function whatProvides($name, LinkConstraintInterface $constraint = null)
+    public function whatProvides($name, LinkConstraintInterface $constraint = null, $mustMatchName = false)
     {
-        if (isset($this->providerCache[$name][(string) $constraint])) {
-            return $this->providerCache[$name][(string) $constraint];
+        $key = ((string) (int) $mustMatchName).((string) $constraint);
+        if (isset($this->providerCache[$name][$key])) {
+            return $this->providerCache[$name][$key];
         }
 
-        return $this->providerCache[$name][(string) $constraint] = $this->computeWhatProvides($name, $constraint);
+        return $this->providerCache[$name][$key] = $this->computeWhatProvides($name, $constraint, $mustMatchName);
     }
 
     /**
      * @see whatProvides
      */
-    private function computeWhatProvides($name, $constraint)
+    private function computeWhatProvides($name, $constraint, $mustMatchName = false)
     {
         $candidates = array();
 
@@ -259,6 +268,9 @@ class Pool
         $nameMatch = false;
 
         foreach ($candidates as $candidate) {
+            if ($this->whitelist !== null && !isset($this->whitelist[$candidate->getId()])) {
+                continue;
+            }
             switch ($this->match($candidate, $name, $constraint)) {
                 case self::MATCH_NONE:
                     break;
@@ -288,6 +300,12 @@ class Pool
             }
         }
 
+        if ($mustMatchName) {
+            return array_filter($matches, function ($match) use ($name) {
+                return $match->getName() == $name;
+            });
+        }
+
         // if a package with the required name exists, we ignore providers
         if ($nameMatch) {
             return $matches;

src/Composer/DependencyResolver/Problem.php

@@ -80,7 +80,13 @@ class Problem
             $rule = $reason['rule'];
             $job = $reason['job'];
 
-            if ($job && $job['cmd'] === 'install' && empty($job['packages'])) {
+            if (isset($job['constraint'])) {
+                $packages = $this->pool->whatProvides($job['packageName'], $job['constraint']);
+            } else {
+                $packages = array();
+            }
+
+            if ($job && $job['cmd'] === 'install' && empty($packages)) {
                 // handle php extensions
                 if (0 === stripos($job['packageName'], 'ext-')) {
                     $ext = substr($job['packageName'], 4);
@@ -161,18 +167,25 @@ class Problem
     {
         switch ($job['cmd']) {
             case 'install':
-                if (!$job['packages']) {
+                $packages = $this->pool->whatProvides($job['packageName'], $job['constraint']);
+                if (!$packages) {
                     return 'No package found to satisfy install request for '.$job['packageName'].$this->constraintToText($job['constraint']);
                 }
 
-                return 'Installation request for '.$job['packageName'].$this->constraintToText($job['constraint']).' -> satisfiable by '.$this->getPackageList($job['packages']).'.';
+                return 'Installation request for '.$job['packageName'].$this->constraintToText($job['constraint']).' -> satisfiable by '.$this->getPackageList($packages).'.';
             case 'update':
                 return 'Update request for '.$job['packageName'].$this->constraintToText($job['constraint']).'.';
             case 'remove':
                 return 'Removal request for '.$job['packageName'].$this->constraintToText($job['constraint']).'';
         }
 
-        return 'Job(cmd='.$job['cmd'].', target='.$job['packageName'].', packages=['.$this->getPackageList($job['packages']).'])';
+        if (isset($job['constraint'])) {
+            $packages = $this->pool->whatProvides($job['packageName'], $job['constraint']);
+        } else {
+            $packages = array();
+        }
+
+        return 'Job(cmd='.$job['cmd'].', target='.$job['packageName'].', packages=['.$this->getPackageList($packages).'])';
     }
 
     protected function getPackageList($packages)

src/Composer/DependencyResolver/Request.php

@@ -46,10 +46,8 @@ class Request
     protected function addJob($packageName, $cmd, LinkConstraintInterface $constraint = null)
     {
         $packageName = strtolower($packageName);
-        $packages = $this->pool->whatProvides($packageName, $constraint);
 
         $this->jobs[] = array(
-            'packages' => $packages,
             'cmd' => $cmd,
             'packageName' => $packageName,
             'constraint' => $constraint,
@@ -58,7 +56,7 @@ class Request
 
     public function updateAll()
     {
-        $this->jobs[] = array('cmd' => 'update-all', 'packages' => array());
+        $this->jobs[] = array('cmd' => 'update-all');
     }
 
     public function getJobs()

src/Composer/DependencyResolver/RuleSetGenerator.php

@@ -25,6 +25,8 @@ class RuleSetGenerator
     protected $rules;
     protected $jobs;
     protected $installedMap;
+    protected $whitelistedMap;
+    protected $addedMap;
 
     public function __construct(PolicyInterface $policy, Pool $pool)
     {
@@ -141,6 +143,41 @@ class RuleSetGenerator
         $this->rules->add($newRule, $type);
     }
 
+    protected function whitelistFromPackage(PackageInterface $package)
+    {
+        $workQueue = new \SplQueue;
+        $workQueue->enqueue($package);
+
+        while (!$workQueue->isEmpty()) {
+            $package = $workQueue->dequeue();
+            if (isset($this->whitelistedMap[$package->getId()])) {
+                continue;
+            }
+
+            $this->whitelistedMap[$package->getId()] = true;
+
+            foreach ($package->getRequires() as $link) {
+                $possibleRequires = $this->pool->whatProvides($link->getTarget(), $link->getConstraint(), true);
+
+                foreach ($possibleRequires as $require) {
+                    $workQueue->enqueue($require);
+                }
+            }
+
+            $obsoleteProviders = $this->pool->whatProvides($package->getName(), null, true);
+
+            foreach ($obsoleteProviders as $provider) {
+                if ($provider === $package) {
+                    continue;
+                }
+
+                if (($package instanceof AliasPackage) && $package->getAliasOf() === $provider) {
+                    $workQueue->enqueue($provider);
+                }
+            }
+        }
+    }
+
     protected function addRulesForPackage(PackageInterface $package)
     {
         $workQueue = new \SplQueue;
@@ -236,26 +273,51 @@ class RuleSetGenerator
         }
     }
 
+    private function whitelistFromUpdatePackages(PackageInterface $package)
+    {
+        $updates = $this->policy->findUpdatePackages($this->pool, $this->installedMap, $package, true);
+
+        foreach ($updates as $update) {
+            $this->whitelistFromPackage($update);
+        }
+    }
+
+    protected function whitelistFromJobs()
+    {
+        foreach ($this->jobs as $job) {
+            switch ($job['cmd']) {
+                case 'install':
+                    $packages = $this->pool->whatProvides($job['packageName'], $job['constraint'], true);
+                    foreach ($packages as $package) {
+                        $this->whitelistFromPackage($package);
+                    }
+                    break;
+            }
+        }
+    }
+
     protected function addRulesForJobs()
     {
         foreach ($this->jobs as $job) {
             switch ($job['cmd']) {
                 case 'install':
-                    if ($job['packages']) {
-                        foreach ($job['packages'] as $package) {
+                    $packages = $this->pool->whatProvides($job['packageName'], $job['constraint']);
+                    if ($packages) {
+                        foreach ($packages as $package) {
                             if (!isset($this->installedMap[$package->getId()])) {
                                 $this->addRulesForPackage($package);
                             }
                         }
 
-                        $rule = $this->createInstallOneOfRule($job['packages'], Rule::RULE_JOB_INSTALL, $job);
+                        $rule = $this->createInstallOneOfRule($packages, Rule::RULE_JOB_INSTALL, $job);
                         $this->addRule(RuleSet::TYPE_JOB, $rule);
                     }
                     break;
                 case 'remove':
                     // remove all packages with this name including uninstalled
                     // ones to make sure none of them are picked as replacements
-                    foreach ($job['packages'] as $package) {
+                    $packages = $this->pool->whatProvides($job['packageName'], $job['constraint']);
+                    foreach ($packages as $package) {
                         $rule = $this->createRemoveRule($package, Rule::RULE_JOB_REMOVE, $job);
                         $this->addRule(RuleSet::TYPE_JOB, $rule);
                     }
@@ -270,6 +332,16 @@ class RuleSetGenerator
         $this->rules = new RuleSet;
         $this->installedMap = $installedMap;
 
+        $this->whitelistedNames = array();
+        foreach ($this->installedMap as $package) {
+            $this->whitelistFromPackage($package);
+            $this->whitelistFromUpdatePackages($package);
+        }
+        $this->whitelistFromJobs();
+
+        $this->pool->setWhitelist($this->whitelistedMap);
+
+        $this->addedMap = array();
         foreach ($this->installedMap as $package) {
             $this->addRulesForPackage($package);
             $this->addRulesForUpdatePackages($package);

src/Composer/DependencyResolver/Solver.php

@@ -131,7 +131,8 @@ class Solver
         foreach ($this->jobs as $job) {
             switch ($job['cmd']) {
                 case 'update':
-                    foreach ($job['packages'] as $package) {
+                    $packages = $this->pool->whatProvides($job['packageName'], $job['constraint']);
+                    foreach ($packages as $package) {
                         if (isset($this->installedMap[$package->getId()])) {
                             $this->updateMap[$package->getId()] = true;
                         }
@@ -145,7 +146,7 @@ class Solver
                     break;
 
                 case 'install':
-                    if (!$job['packages']) {
+                    if (!$this->pool->whatProvides($job['packageName'], $job['constraint'])) {
                         $problem = new Problem($this->pool);
                         $problem->addRule(new Rule($this->pool, array(), null, null, $job));
                         $this->problems[] = $problem;

tests/Composer/Test/DependencyResolver/RequestTest.php

@@ -39,9 +39,9 @@ class RequestTest extends TestCase
 
         $this->assertEquals(
             array(
-                array('packages' => array($foo), 'cmd' => 'install', 'packageName' => 'foo', 'constraint' => null),
-                array('packages' => array($bar), 'cmd' => 'install', 'packageName' => 'bar', 'constraint' => null),
-                array('packages' => array($foobar), 'cmd' => 'remove', 'packageName' => 'foobar', 'constraint' => null),
+                array('cmd' => 'install', 'packageName' => 'foo', 'constraint' => null),
+                array('cmd' => 'install', 'packageName' => 'bar', 'constraint' => null),
+                array('cmd' => 'remove', 'packageName' => 'foobar', 'constraint' => null),
             ),
             $request->getJobs());
     }
@@ -66,7 +66,7 @@ class RequestTest extends TestCase
 
         $this->assertEquals(
             array(
-                    array('packages' => array($foo1, $foo2), 'cmd' => 'install', 'packageName' => 'foo', 'constraint' => $constraint),
+                    array('cmd' => 'install', 'packageName' => 'foo', 'constraint' => $constraint),
             ),
             $request->getJobs()
         );
@@ -80,7 +80,7 @@ class RequestTest extends TestCase
         $request->updateAll();
 
         $this->assertEquals(
-            array(array('cmd' => 'update-all', 'packages' => array())),
+            array(array('cmd' => 'update-all')),
             $request->getJobs());
     }
 }

tests/Composer/Test/DependencyResolver/SolverTest.php

@@ -441,10 +441,9 @@ class SolverTest extends TestCase
 
         $this->request->install('A');
 
-        $this->checkSolverResult(array(
-            array('job' => 'install', 'package' => $packageQ),
-            array('job' => 'install', 'package' => $packageA),
-        ));
+        // must explicitly pick the provider, so error in this case
+        $this->setExpectedException('Composer\DependencyResolver\SolverProblemsException');
+        $this->solver->solve($this->request);
     }
 
     public function testSkipReplacerOfExistingPackage()
@@ -465,7 +464,7 @@ class SolverTest extends TestCase
         ));
     }
 
-    public function testInstallReplacerOfMissingPackage()
+    public function testNoInstallReplacerOfMissingPackage()
     {
         $this->repo->addPackage($packageA = $this->getPackage('A', '1.0'));
         $this->repo->addPackage($packageQ = $this->getPackage('Q', '1.0'));
@@ -476,10 +475,8 @@ class SolverTest extends TestCase
 
         $this->request->install('A');
 
-        $this->checkSolverResult(array(
-            array('job' => 'install', 'package' => $packageQ),
-            array('job' => 'install', 'package' => $packageA),
-        ));
+        $this->setExpectedException('Composer\DependencyResolver\SolverProblemsException');
+        $this->solver->solve($this->request);
     }
 
     public function testSkipReplacedPackageIfReplacerIsSelected()
@@ -574,11 +571,12 @@ class SolverTest extends TestCase
         $this->reposComplete();
 
         $this->request->install('A');
+        $this->request->install('C');
 
         $this->checkSolverResult(array(
-            array('job' => 'install', 'package' => $packageB),
             array('job' => 'install', 'package' => $packageA),
             array('job' => 'install', 'package' => $packageC),
+            array('job' => 'install', 'package' => $packageB),
         ));
     }
 
@@ -611,6 +609,7 @@ class SolverTest extends TestCase
         $this->reposComplete();
 
         $this->request->install('A');
+        $this->request->install('D');
 
         $this->checkSolverResult(array(
             array('job' => 'install', 'package' => $packageD2),

tests/Composer/Test/Fixtures/installer/broken-deps-do-not-replace.test

@@ -0,0 +1,25 @@
+--TEST--
+Broken dependencies should not lead to a replacer being installed which is not mentioned by name
+--COMPOSER--
+{
+    "repositories": [
+        {
+            "type": "package",
+            "package": [
+                { "name": "a/a", "version": "1.0.0" },
+                { "name": "b/b", "version": "1.0.0", "require": {"c/c": "1.*"} },
+                { "name": "c/c", "version": "1.0.0", "replace": {"a/a": "1.0.0" },"require":{"x/x": "1.0"}},
+                { "name": "d/d", "version": "1.0.0", "replace": {"a/a": "1.0.0", "c/c":"1.0.0" }}
+            ]
+        }
+    ],
+    "require": {
+        "a/a": "1.*",
+        "b/b": "1.*"
+    }
+}
+--RUN--
+install
+--EXPECT-EXIT-CODE--
+2
+--EXPECT--

tests/Composer/Test/Fixtures/installer/provide-priorities.test

@@ -1,34 +0,0 @@
---TEST--
-Provide only applies when no existing package has the given name
---COMPOSER--
-{
-    "repositories": [
-        {
-            "type": "package",
-            "package": [
-                { "name": "higher-prio-hijacker", "version": "1.1.0", "provide": { "package": "1.0.0" } },
-                { "name": "provider2", "version": "1.1.0", "provide": { "package2": "1.0.0" } }
-            ]
-        },
-        {
-            "type": "package",
-            "package": [
-                { "name": "package", "version": "0.9.0" },
-                { "name": "package", "version": "1.0.0" },
-                { "name": "hijacker", "version": "1.1.0", "provide": { "package": "1.0.0" } },
-                { "name": "provider3", "version": "1.1.0", "provide": { "package3": "1.0.0" } }
-            ]
-        }
-    ],
-    "require": {
-        "package": "1.*",
-        "package2": "1.*",
-        "provider3": "1.1.0"
-    }
-}
---RUN--
-install
---EXPECT--
-Installing package (1.0.0)
-Installing provider2 (1.1.0)
-Installing provider3 (1.1.0)

tests/Composer/Test/Fixtures/installer/replace-root-require.test

@@ -0,0 +1,24 @@
+--TEST--
+Ensure a transiently required replacer can replace root requirements
+--COMPOSER--
+{
+    "repositories": [
+        {
+            "type": "package",
+            "package": [
+                { "name": "a/a", "version": "1.0.0" },
+                { "name": "b/b", "version": "1.0.0", "require": {"c/c": "1.*"} },
+                { "name": "c/c", "version": "1.0.0", "replace": {"a/a": "1.0.0" }}
+            ]
+        }
+    ],
+    "require": {
+        "a/a": "1.*",
+        "b/b": "1.*"
+    }
+}
+--RUN--
+install
+--EXPECT--
+Installing c/c (1.0.0)
+Installing b/b (1.0.0)

tests/Composer/Test/InstallerTest.php

@@ -138,7 +138,7 @@ class InstallerTest extends TestCase
     /**
      * @dataProvider getIntegrationTests
      */
-    public function testIntegration($file, $message, $condition, $composerConfig, $lock, $installed, $run, $expectLock, $expectOutput, $expect)
+    public function testIntegration($file, $message, $condition, $composerConfig, $lock, $installed, $run, $expectLock, $expectOutput, $expect, $expectExitCode)
     {
         if ($condition) {
             eval('$res = '.$condition.';');
@@ -228,7 +228,7 @@ class InstallerTest extends TestCase
         $appOutput = fopen('php://memory', 'w+');
         $result = $application->run(new StringInput($run), new StreamOutput($appOutput));
         fseek($appOutput, 0);
-        $this->assertEquals(0, $result, $output . stream_get_contents($appOutput));
+        $this->assertEquals($expectExitCode, $result, $output . stream_get_contents($appOutput));
 
         if ($expectLock) {
             unset($actualLock['hash']);
@@ -266,6 +266,7 @@ class InstallerTest extends TestCase
                 --RUN--\s*(?P<run>.*?)\s*
                 (?:--EXPECT-LOCK--\s*(?P<expectLock>'.$content.'))?\s*
                 (?:--EXPECT-OUTPUT--\s*(?P<expectOutput>'.$content.'))?\s*
+                (?:--EXPECT-EXIT-CODE--\s*(?P<expectExitCode>\d+))?\s*
                 --EXPECT--\s*(?P<expect>.*?)\s*
             $}xs';
 
@@ -273,6 +274,7 @@ class InstallerTest extends TestCase
             $installedDev = array();
             $lock = array();
             $expectLock = array();
+            $expectExitCode = 0;
 
             if (preg_match($pattern, $test, $match)) {
                 try {
@@ -294,6 +296,7 @@ class InstallerTest extends TestCase
                     }
                     $expectOutput = $match['expectOutput'];
                     $expect = $match['expect'];
+                    $expectExitCode = (int) $match['expectExitCode'];
                 } catch (\Exception $e) {
                     die(sprintf('Test "%s" is not valid: '.$e->getMessage(), str_replace($fixturesDir.'/', '', $file)));
                 }
@@ -301,7 +304,7 @@ class InstallerTest extends TestCase
                 die(sprintf('Test "%s" is not valid, did not match the expected format.', str_replace($fixturesDir.'/', '', $file)));
             }
 
-            $tests[] = array(str_replace($fixturesDir.'/', '', $file), $message, $condition, $composer, $lock, $installed, $run, $expectLock, $expectOutput, $expect);
+            $tests[] = array(str_replace($fixturesDir.'/', '', $file), $message, $condition, $composer, $lock, $installed, $run, $expectLock, $expectOutput, $expect, $expectExitCode);
         }
 
         return $tests;