From c41df325d837b898da4aaea14a9c9a09e1db2bb2 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 30 Jan 2020 15:23:22 +0100 Subject: [PATCH] Remove RepositorySet from Solver and remove getPool from RepositorySet --- src/Composer/DependencyResolver/Problem.php | 12 +++++------- src/Composer/DependencyResolver/Rule.php | 7 +++---- src/Composer/DependencyResolver/RuleSet.php | 6 +++--- src/Composer/DependencyResolver/Solver.php | 14 ++------------ .../SolverProblemsException.php | 11 +++++------ src/Composer/Installer.php | 6 +++--- src/Composer/Repository/RepositorySet.php | 19 ++++++------------- .../Test/DependencyResolver/RuleSetTest.php | 5 +---- .../Test/DependencyResolver/RuleTest.php | 5 +---- .../Test/DependencyResolver/SolverTest.php | 12 +++++++----- 10 files changed, 36 insertions(+), 61 deletions(-) diff --git a/src/Composer/DependencyResolver/Problem.php b/src/Composer/DependencyResolver/Problem.php index 1070dc4f3..82e649e3f 100644 --- a/src/Composer/DependencyResolver/Problem.php +++ b/src/Composer/DependencyResolver/Problem.php @@ -63,7 +63,7 @@ class Problem * @param array $installedMap A map of all present packages * @return string */ - public function getPrettyString(RepositorySet $repositorySet, Request $request, array $installedMap = array(), array $learnedPool = array()) + public function getPrettyString(RepositorySet $repositorySet, Request $request, Pool $pool, array $installedMap = array(), array $learnedPool = array()) { // TODO doesn't this entirely defeat the purpose of the problem sections? what's the point of sections? $reasons = call_user_func_array('array_merge', array_reverse($this->reasons)); @@ -81,20 +81,20 @@ class Problem $constraint = $reasonData['constraint']; if (isset($constraint)) { - $packages = $repositorySet->getPool()->whatProvides($packageName, $constraint); + $packages = $pool->whatProvides($packageName, $constraint); } else { $packages = array(); } if (empty($packages)) { - return "\n ".implode(self::getMissingPackageReason($repositorySet, $request, $packageName, $constraint)); + return "\n ".implode(self::getMissingPackageReason($repositorySet, $request, $pool, $packageName, $constraint)); } } $messages = array(); foreach ($reasons as $rule) { - $messages[] = $rule->getPrettyString($repositorySet, $request, $installedMap, $learnedPool); + $messages[] = $rule->getPrettyString($repositorySet, $request, $pool, $installedMap, $learnedPool); } return "\n - ".implode("\n - ", $messages); @@ -125,10 +125,8 @@ class Problem /** * @internal */ - public static function getMissingPackageReason(RepositorySet $repositorySet, Request $request, $packageName, $constraint = null) + public static function getMissingPackageReason(RepositorySet $repositorySet, Request $request, Pool $pool, $packageName, $constraint = null) { - $pool = $repositorySet->getPool(); - // handle php/hhvm if ($packageName === 'php' || $packageName === 'php-64bit' || $packageName === 'hhvm') { $version = phpversion(); diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index 849268fed..7dcdc5711 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -123,9 +123,8 @@ abstract class Rule abstract public function isAssertion(); - public function getPrettyString(RepositorySet $repositorySet, Request $request, array $installedMap = array(), array $learnedPool = array()) + public function getPrettyString(RepositorySet $repositorySet, Request $request, Pool $pool, array $installedMap = array(), array $learnedPool = array()) { - $pool = $repositorySet->getPool(); $literals = $this->getLiterals(); $ruleText = ''; @@ -180,7 +179,7 @@ abstract class Rule } else { $targetName = $this->reasonData->getTarget(); - $reason = Problem::getMissingPackageReason($repositorySet, $request, $targetName, $this->reasonData->getConstraint()); + $reason = Problem::getMissingPackageReason($repositorySet, $request, $pool, $targetName, $this->reasonData->getConstraint()); return $text . ' -> ' . $reason[1]; } @@ -241,7 +240,7 @@ abstract class Rule $learnedString = ', learned rules:'."\n - "; $reasons = array(); foreach ($learnedPool[$this->reasonData] as $learnedRule) { - $reasons[] = $learnedRule->getPrettyString($repositorySet, $request, $installedMap, $learnedPool); + $reasons[] = $learnedRule->getPrettyString($repositorySet, $request, $pool, $installedMap, $learnedPool); } $learnedString .= implode("\n - ", array_unique($reasons)); } else { diff --git a/src/Composer/DependencyResolver/RuleSet.php b/src/Composer/DependencyResolver/RuleSet.php index 9834e002f..d37ca1e9f 100644 --- a/src/Composer/DependencyResolver/RuleSet.php +++ b/src/Composer/DependencyResolver/RuleSet.php @@ -157,13 +157,13 @@ class RuleSet implements \IteratorAggregate, \Countable return array_keys($types); } - public function getPrettyString(RepositorySet $repositorySet = null, Request $request = null) + public function getPrettyString(RepositorySet $repositorySet = null, Request $request = null, Pool $pool = null) { $string = "\n"; foreach ($this->rules as $type => $rules) { $string .= str_pad(self::$types[$type], 8, ' ') . ": "; foreach ($rules as $rule) { - $string .= ($repositorySet && $request ? $rule->getPrettyString($repositorySet, $request) : $rule)."\n"; + $string .= ($repositorySet && $request && $pool ? $rule->getPrettyString($repositorySet, $request, $pool) : $rule)."\n"; } $string .= "\n\n"; } @@ -173,6 +173,6 @@ class RuleSet implements \IteratorAggregate, \Countable public function __toString() { - return $this->getPrettyString(null, null); + return $this->getPrettyString(null, null, null); } } diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index d670c980a..e32fe2478 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -14,9 +14,7 @@ namespace Composer\DependencyResolver; use Composer\IO\IOInterface; use Composer\Package\PackageInterface; -use Composer\Repository\RepositoryInterface; use Composer\Repository\PlatformRepository; -use Composer\Repository\RepositorySet; /** * @author Nils Adermann @@ -30,8 +28,6 @@ class Solver protected $policy; /** @var Pool */ protected $pool; - /** @var RepositorySet */ - protected $repositorySet; /** @var RuleSet */ protected $rules; @@ -67,12 +63,11 @@ class Solver * @param Pool $pool * @param IOInterface $io */ - public function __construct(PolicyInterface $policy, Pool $pool, IOInterface $io, RepositorySet $repositorySet) + public function __construct(PolicyInterface $policy, Pool $pool, IOInterface $io) { $this->io = $io; $this->policy = $policy; $this->pool = $pool; - $this->repositorySet = $repositorySet; } /** @@ -88,11 +83,6 @@ class Solver return $this->pool; } - public function getRepositorySet() - { - return $this->repositorySet; - } - // aka solver_makeruledecisions private function makeAssertionRuleDecisions() @@ -222,7 +212,7 @@ class Solver $this->io->writeError(sprintf('Dependency resolution completed in %.3f seconds', microtime(true) - $before), true, IOInterface::VERBOSE); if ($this->problems) { - throw new SolverProblemsException($this->problems, $this->repositorySet, $request, $this->learnedPool); + throw new SolverProblemsException($this->problems, $this->learnedPool); } return new LockTransaction($this->pool, $request->getPresentMap(), $request->getUnlockableMap(), $this->decisions); diff --git a/src/Composer/DependencyResolver/SolverProblemsException.php b/src/Composer/DependencyResolver/SolverProblemsException.php index 37768f436..542fe0464 100644 --- a/src/Composer/DependencyResolver/SolverProblemsException.php +++ b/src/Composer/DependencyResolver/SolverProblemsException.php @@ -21,24 +21,23 @@ use Composer\Repository\RepositorySet; class SolverProblemsException extends \RuntimeException { protected $problems; - protected $installedMap; protected $learnedPool; - public function __construct(array $problems, RepositorySet $repositorySet, Request $request, array $learnedPool) + public function __construct(array $problems, array $learnedPool) { $this->problems = $problems; - $this->installedMap = $request->getPresentMap(true); $this->learnedPool = $learnedPool; - parent::__construct($this->createMessage($repositorySet, $request), 2); + parent::__construct('Failed resolving dependencies with '.count($problems).' problems, call getPrettyString to get formatted details', 2); } - protected function createMessage(RepositorySet $repositorySet, Request $request) + public function getPrettyString(RepositorySet $repositorySet, Request $request, Pool $pool) { + $installedMap = $request->getPresentMap(true); $text = "\n"; $hasExtensionProblems = false; foreach ($this->problems as $i => $problem) { - $text .= " Problem ".($i + 1).$problem->getPrettyString($repositorySet, $request, $this->installedMap, $this->learnedPool)."\n"; + $text .= " Problem ".($i + 1).$problem->getPrettyString($repositorySet, $request, $pool, $installedMap, $this->learnedPool)."\n"; if (!$hasExtensionProblems && $this->hasExtensionProblems($problem->getReasons())) { $hasExtensionProblems = true; diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index 542a9ee1c..b1ed2db2c 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -396,7 +396,7 @@ class Installer $solver = null; } catch (SolverProblemsException $e) { $this->io->writeError('Your requirements could not be resolved to an installable set of packages.', true, IOInterface::QUIET); - $this->io->writeError($e->getMessage()); + $this->io->writeError($e->getPrettyString($repositorySet, $request, $pool)); if (!$this->devMode) { $this->io->writeError('Running update with --no-dev does not mean require-dev is ignored, it just means the packages will not be installed. If dev requirements are blocking the update you have to resolve those problems.', true, IOInterface::QUIET); } @@ -536,7 +536,7 @@ class Installer $solver = null; } catch (SolverProblemsException $e) { $this->io->writeError('Unable to find a compatible set of packages based on your non-dev requirements alone.', true, IOInterface::QUIET); - $this->io->writeError($e->getMessage()); + $this->io->writeError($e->getPrettyString($repositorySet, $request, $pool)); return max(1, $e->getCode()); } @@ -602,7 +602,7 @@ class Installer } } catch (SolverProblemsException $e) { $this->io->writeError('Your lock file does not contain a compatible set of packages. Please run composer update.', true, IOInterface::QUIET); - $this->io->writeError($e->getMessage()); + $this->io->writeError($e->getPrettyString($repositorySet, $request, $pool)); return max(1, $e->getCode()); } diff --git a/src/Composer/Repository/RepositorySet.php b/src/Composer/Repository/RepositorySet.php index 96fe94df9..f931a17a2 100644 --- a/src/Composer/Repository/RepositorySet.php +++ b/src/Composer/Repository/RepositorySet.php @@ -54,8 +54,8 @@ class RepositorySet private $stabilityFlags; private $rootRequires; - /** @var Pool */ - private $pool; + /** @var bool */ + private $locked = false; public function __construct(array $rootAliases = array(), array $rootReferences = array(), $minimumStability = 'stable', array $stabilityFlags = array(), array $rootRequires = array()) { @@ -92,7 +92,7 @@ class RepositorySet */ public function addRepository(RepositoryInterface $repo) { - if ($this->pool) { + if ($this->locked) { throw new \RuntimeException("Pool has already been created from this repository set, it cannot be modified anymore."); } @@ -191,7 +191,9 @@ class RepositorySet } } - return $this->pool = $poolBuilder->buildPool($this->repositories, $request); + $this->locked = true; + + return $poolBuilder->buildPool($this->repositories, $request); } // TODO unify this with above in some simpler version without "request"? @@ -210,13 +212,4 @@ class RepositorySet return $this->createPool($request); } - - /** - * Access the pool object after it has been created, relevant for plugins which need to read info from the pool - * @return Pool - */ - public function getPool() - { - return $this->pool; - } } diff --git a/tests/Composer/Test/DependencyResolver/RuleSetTest.php b/tests/Composer/Test/DependencyResolver/RuleSetTest.php index 3617e08fd..5a89ddf79 100644 --- a/tests/Composer/Test/DependencyResolver/RuleSetTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleSetTest.php @@ -144,9 +144,6 @@ class RuleSetTest extends TestCase )); $repositorySetMock = $this->getMockBuilder('Composer\Repository\RepositorySet')->disableOriginalConstructor()->getMock(); - $repositorySetMock->expects($this->any()) - ->method('getPool') - ->willReturn($pool); $requestMock = $this->getMockBuilder('Composer\DependencyResolver\Request')->disableOriginalConstructor()->getMock(); $ruleSet = new RuleSet; @@ -155,6 +152,6 @@ class RuleSetTest extends TestCase $ruleSet->add($rule, RuleSet::TYPE_REQUEST); - $this->assertContains('REQUEST : No package found to satisfy root composer.json require foo/bar', $ruleSet->getPrettyString($repositorySetMock, $requestMock)); + $this->assertContains('REQUEST : No package found to satisfy root composer.json require foo/bar', $ruleSet->getPrettyString($repositorySetMock, $requestMock, $pool)); } } diff --git a/tests/Composer/Test/DependencyResolver/RuleTest.php b/tests/Composer/Test/DependencyResolver/RuleTest.php index 3ff6b765d..f819397fb 100644 --- a/tests/Composer/Test/DependencyResolver/RuleTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleTest.php @@ -100,13 +100,10 @@ class RuleTest extends TestCase )); $repositorySetMock = $this->getMockBuilder('Composer\Repository\RepositorySet')->disableOriginalConstructor()->getMock(); - $repositorySetMock->expects($this->any()) - ->method('getPool') - ->willReturn($pool); $requestMock = $this->getMockBuilder('Composer\DependencyResolver\Request')->disableOriginalConstructor()->getMock(); $rule = new GenericRule(array($p1->getId(), -$p2->getId()), Rule::RULE_PACKAGE_REQUIRES, new Link('baz', 'foo')); - $this->assertEquals('baz 1.1 relates to foo -> satisfiable by foo[2.1].', $rule->getPrettyString($repositorySetMock, $requestMock)); + $this->assertEquals('baz 1.1 relates to foo -> satisfiable by foo[2.1].', $rule->getPrettyString($repositorySetMock, $requestMock, $pool)); } } diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index 74f0a480f..e64002883 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/tests/Composer/Test/DependencyResolver/SolverTest.php @@ -34,6 +34,7 @@ class SolverTest extends TestCase protected $request; protected $policy; protected $solver; + protected $pool; public function setUp() { @@ -82,7 +83,7 @@ class SolverTest extends TestCase $problems = $e->getProblems(); $this->assertCount(1, $problems); $this->assertEquals(2, $e->getCode()); - $this->assertEquals("\n - Root composer.json requires b, it could not be found in any version, there may be a typo in the package name.", $problems[0]->getPrettyString($this->repoSet, $this->request)); + $this->assertEquals("\n - Root composer.json requires b, it could not be found in any version, there may be a typo in the package name.", $problems[0]->getPrettyString($this->repoSet, $this->request, $this->pool)); } } @@ -653,7 +654,7 @@ class SolverTest extends TestCase $msg .= " - Root composer.json requires a -> satisfiable by A[1.0].\n"; $msg .= " - B 1.0 conflicts with A[1.0].\n"; $msg .= " - Root composer.json requires b -> satisfiable by B[1.0].\n"; - $this->assertEquals($msg, $e->getMessage()); + $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool)); } } @@ -683,7 +684,7 @@ class SolverTest extends TestCase $msg .= " Problem 1\n"; $msg .= " - Root composer.json requires a -> satisfiable by A[1.0].\n"; $msg .= " - A 1.0 requires b >= 2.0 -> found B[1.0] but it does not match your constraint.\n"; - $this->assertEquals($msg, $e->getMessage()); + $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool)); } } @@ -728,7 +729,7 @@ class SolverTest extends TestCase $msg .= " - Only one of these can be installed: B[0.9, 1.0].\n"; $msg .= " - A 1.0 requires b >= 1.0 -> satisfiable by B[1.0].\n"; $msg .= " - Root composer.json requires a -> satisfiable by A[1.0].\n"; - $this->assertEquals($msg, $e->getMessage()); + $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool)); } } @@ -889,7 +890,8 @@ class SolverTest extends TestCase protected function createSolver() { - $this->solver = new Solver($this->policy, $this->repoSet->createPool($this->request), new NullIO(), $this->repoSet); + $this->pool = $this->repoSet->createPool($this->request); + $this->solver = new Solver($this->policy, $this->pool, new NullIO()); } protected function checkSolverResult(array $expected)