From 5b80144ad007ef1a77d14d37fd78745dbce0b8b0 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 21 Feb 2014 13:41:21 +0100 Subject: [PATCH] Resolve job packages after whitelist generation --- src/Composer/DependencyResolver/Problem.php | 21 +++++++++++++++---- src/Composer/DependencyResolver/Request.php | 4 +--- .../DependencyResolver/RuleSetGenerator.php | 17 ++++++++------- src/Composer/DependencyResolver/Solver.php | 5 +++-- .../Test/DependencyResolver/RequestTest.php | 10 ++++----- .../Test/DependencyResolver/SolverTest.php | 9 ++++---- tests/Composer/Test/InstallerTest.php | 4 ++-- 7 files changed, 41 insertions(+), 29 deletions(-) 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 85f83b4f5..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, true); $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 d203832db..f555f1205 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -287,10 +287,9 @@ class RuleSetGenerator foreach ($this->jobs as $job) { switch ($job['cmd']) { case 'install': - if ($job['packages']) { - foreach ($job['packages'] as $package) { - $this->whitelistFromPackage($package); - } + $packages = $this->pool->whatProvides($job['packageName'], $job['constraint'], true); + foreach ($packages as $package) { + $this->whitelistFromPackage($package); } break; } @@ -302,21 +301,23 @@ class RuleSetGenerator 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); } 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 14f9c0943..ac78b5a26 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/tests/Composer/Test/DependencyResolver/SolverTest.php @@ -464,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')); @@ -475,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() @@ -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/InstallerTest.php b/tests/Composer/Test/InstallerTest.php index 9f2a1bc4b..4e992c577 100644 --- a/tests/Composer/Test/InstallerTest.php +++ b/tests/Composer/Test/InstallerTest.php @@ -250,7 +250,7 @@ class InstallerTest extends TestCase $tests = array(); foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($fixturesDir), \RecursiveIteratorIterator::LEAVES_ONLY) as $file) { - if (!preg_match('/replace-root-require\.test$/', $file)) { + if (!preg_match('/\.test$/', $file)) { continue; } @@ -296,7 +296,7 @@ class InstallerTest extends TestCase } $expectOutput = $match['expectOutput']; $expect = $match['expect']; - $expectExitCode = $match['expectExitCode']; + $expectExitCode = (int) $match['expectExitCode']; } catch (\Exception $e) { die(sprintf('Test "%s" is not valid: '.$e->getMessage(), str_replace($fixturesDir.'/', '', $file))); }