diff --git a/src/Composer/DependencyResolver/DefaultPolicy.php b/src/Composer/DependencyResolver/DefaultPolicy.php index 190829213..a58cf6184 100644 --- a/src/Composer/DependencyResolver/DefaultPolicy.php +++ b/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; } diff --git a/src/Composer/DependencyResolver/Pool.php b/src/Composer/DependencyResolver/Pool.php index a1bba4f3a..56f356baa 100644 --- a/src/Composer/DependencyResolver/Pool.php +++ b/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; diff --git a/src/Composer/DependencyResolver/Problem.php b/src/Composer/DependencyResolver/Problem.php index 56ac867c4..765b74a19 100644 --- a/src/Composer/DependencyResolver/Problem.php +++ b/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) diff --git a/src/Composer/DependencyResolver/Request.php b/src/Composer/DependencyResolver/Request.php index 92c8aa175..bf74318f6 100644 --- a/src/Composer/DependencyResolver/Request.php +++ b/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() diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index b40ce1a60..f555f1205 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/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); diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 0fc860c72..3e101e0f3 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/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; diff --git a/tests/Composer/Test/DependencyResolver/RequestTest.php b/tests/Composer/Test/DependencyResolver/RequestTest.php index d8cb865a0..0ba43ca73 100644 --- a/tests/Composer/Test/DependencyResolver/RequestTest.php +++ b/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()); } } diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index 349f6e3b4..ac78b5a26 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/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), diff --git a/tests/Composer/Test/Fixtures/installer/broken-deps-do-not-replace.test b/tests/Composer/Test/Fixtures/installer/broken-deps-do-not-replace.test new file mode 100644 index 000000000..c626db198 --- /dev/null +++ b/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-- diff --git a/tests/Composer/Test/Fixtures/installer/provide-priorities.test b/tests/Composer/Test/Fixtures/installer/provide-priorities.test deleted file mode 100644 index f97e16e6c..000000000 --- a/tests/Composer/Test/Fixtures/installer/provide-priorities.test +++ /dev/null @@ -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) diff --git a/tests/Composer/Test/Fixtures/installer/replace-root-require.test b/tests/Composer/Test/Fixtures/installer/replace-root-require.test new file mode 100644 index 000000000..c00ac4fd5 --- /dev/null +++ b/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) diff --git a/tests/Composer/Test/InstallerTest.php b/tests/Composer/Test/InstallerTest.php index 0cc17cfb0..4e992c577 100644 --- a/tests/Composer/Test/InstallerTest.php +++ b/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.*?)\s* (?:--EXPECT-LOCK--\s*(?P'.$content.'))?\s* (?:--EXPECT-OUTPUT--\s*(?P'.$content.'))?\s* + (?:--EXPECT-EXIT-CODE--\s*(?P\d+))?\s* --EXPECT--\s*(?P.*?)\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;