From b6e2d60c9eee1a75ac5d1b8a1bb621ae4baf6b81 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 11 Sep 2018 15:49:08 +0200 Subject: [PATCH] Create the pool in the installer before giving it to the solver --- src/Composer/DependencyResolver/Solver.php | 14 +++---- src/Composer/Installer.php | 42 ++++++++++--------- src/Composer/Repository/RepositorySet.php | 28 ++++++------- .../DependencyResolver/DefaultPolicyTest.php | 30 ++++++------- .../Test/DependencyResolver/SolverTest.php | 5 ++- 5 files changed, 60 insertions(+), 59 deletions(-) diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index f5226fca5..959f1dc4c 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -27,8 +27,8 @@ class Solver /** @var PolicyInterface */ protected $policy; - /** @var RepositorySet */ - protected $repositorySet = null; + /** @var Pool */ + protected $pool = null; /** @var RepositoryInterface */ protected $installed; /** @var RuleSet */ @@ -37,8 +37,6 @@ class Solver protected $ruleSetGenerator; /** @var array */ protected $jobs; - /** @var Pool */ - protected $pool = null; /** @var int[] */ protected $updateMap = array(); @@ -65,15 +63,15 @@ class Solver /** * @param PolicyInterface $policy - * @param RepositorySet $repositorySet + * @param Pool $pool * @param RepositoryInterface $installed * @param IOInterface $io */ - public function __construct(PolicyInterface $policy, RepositorySet $repositorySet, RepositoryInterface $installed, IOInterface $io) + public function __construct(PolicyInterface $policy, Pool $pool, RepositoryInterface $installed, IOInterface $io) { $this->io = $io; $this->policy = $policy; - $this->repositorySet = $repositorySet; + $this->pool = $pool; $this->installed = $installed; } @@ -217,8 +215,6 @@ class Solver { $this->jobs = $request->getJobs(); - $this->pool = $this->repositorySet->createPool(); - $this->setupInstalledMap(); $this->ruleSetGenerator = new RuleSetGenerator($this->policy, $this->pool); diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index bb27bda3c..c6f888493 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -20,6 +20,7 @@ use Composer\DependencyResolver\Operation\UninstallOperation; use Composer\DependencyResolver\Operation\MarkAliasUninstalledOperation; use Composer\DependencyResolver\Operation\OperationInterface; use Composer\DependencyResolver\PolicyInterface; +use Composer\DependencyResolver\Pool; use Composer\DependencyResolver\Request; use Composer\DependencyResolver\Rule; use Composer\DependencyResolver\Solver; @@ -464,14 +465,15 @@ class Installer } } - $repositorySet->getPoolTemp(); // TODO remove this, but ensures ids are set before dev packages are processed in advance of solver + $this->eventDispatcher->dispatchInstallerEvent(InstallerEvents::PRE_DEPENDENCIES_SOLVING, $this->devMode, $policy, $repositorySet, $installedRepo, $request); + + $pool = $repositorySet->createPool(); // force dev packages to have the latest links if we update or install from a (potentially new) lock - $this->processDevPackages($localRepo, $repositorySet, $policy, $repositories, $installedRepo, $lockedRepository, 'force-links'); + $this->processDevPackages($localRepo, $pool, $policy, $repositories, $installedRepo, $lockedRepository, 'force-links'); // solve dependencies - $this->eventDispatcher->dispatchInstallerEvent(InstallerEvents::PRE_DEPENDENCIES_SOLVING, $this->devMode, $policy, $repositorySet, $installedRepo, $request); - $solver = new Solver($policy, $repositorySet, $installedRepo, $this->io); + $solver = new Solver($policy, $pool, $installedRepo, $this->io); try { $operations = $solver->solve($request, $this->ignorePlatformReqs); } catch (SolverProblemsException $e) { @@ -485,7 +487,7 @@ class Installer } // force dev packages to be updated if we update or install from a (potentially new) lock - $operations = $this->processDevPackages($localRepo, $repositorySet, $policy, $repositories, $installedRepo, $lockedRepository, 'force-updates', $operations); + $operations = $this->processDevPackages($localRepo, $pool, $policy, $repositories, $installedRepo, $lockedRepository, 'force-updates', $operations); $this->eventDispatcher->dispatchInstallerEvent(InstallerEvents::POST_DEPENDENCIES_SOLVING, $this->devMode, $policy, $repositorySet, $installedRepo, $request, $operations); @@ -600,11 +602,11 @@ class Installer if ($reason instanceof Rule) { switch ($reason->getReason()) { case Rule::RULE_JOB_INSTALL: - $this->io->writeError(' REASON: Required by the root package: '.$reason->getPrettyString($solver->getPool())); + $this->io->writeError(' REASON: Required by the root package: '.$reason->getPrettyString($pool)); $this->io->writeError(''); break; case Rule::RULE_PACKAGE_REQUIRES: - $this->io->writeError(' REASON: '.$reason->getPrettyString($solver->getPool())); + $this->io->writeError(' REASON: '.$reason->getPrettyString($pool)); $this->io->writeError(''); break; } @@ -623,7 +625,7 @@ class Installer if ($this->executeOperations) { // force source/dist urls to be updated for all packages - $this->processPackageUrls($repositorySet, $policy, $localRepo, $repositories); + $this->processPackageUrls($pool, $policy, $localRepo, $repositories); $localRepo->write(); } @@ -699,7 +701,7 @@ class Installer // solve deps to see which get removed $this->eventDispatcher->dispatchInstallerEvent(InstallerEvents::PRE_DEPENDENCIES_SOLVING, false, $policy, $repositorySet, $installedRepo, $request); - $solver = new Solver($policy, $repositorySet, $installedRepo, $this->io); + $solver = new Solver($policy, $repositorySet->createPool(), $installedRepo, $this->io); $ops = $solver->solve($request, $this->ignorePlatformReqs); $this->eventDispatcher->dispatchInstallerEvent(InstallerEvents::POST_DEPENDENCIES_SOLVING, false, $policy, $repositorySet, $installedRepo, $request, $ops); @@ -948,7 +950,7 @@ class Installer /** * @param WritableRepositoryInterface $localRepo - * @param RepositorySet $repositorySet + * @param Pool $pool * @param PolicyInterface $policy * @param array $repositories * @param RepositoryInterface $installedRepo @@ -957,7 +959,7 @@ class Installer * @param array|null $operations * @return array */ - private function processDevPackages($localRepo, $repositorySet, $policy, $repositories, $installedRepo, $lockedRepository, $task, array $operations = null) + private function processDevPackages($localRepo, Pool $pool, $policy, $repositories, $installedRepo, $lockedRepository, $task, array $operations = null) { if ($task === 'force-updates' && null === $operations) { throw new \InvalidArgumentException('Missing operations argument'); @@ -1012,7 +1014,7 @@ class Installer } // find similar packages (name/version) in all repositories - $matches = $repositorySet->findPackages($package->getName(), new Constraint('=', $package->getVersion())); + $matches = $pool->whatProvides($package->getName(), new Constraint('=', $package->getVersion()), true); foreach ($matches as $index => $match) { // skip local packages if (!in_array($match->getRepository(), $repositories, true)) { @@ -1024,8 +1026,8 @@ class Installer } // select preferred package according to policy rules - if ($matches && $matches = $policy->selectPreferredPackages($repositorySet->getPoolTemp(), array(), $matches)) { // TODO remove temp call - $newPackage = $repositorySet->getPoolTemp()->literalToPackage($matches[0]); + if ($matches && $matches = $policy->selectPreferredPackages($pool, array(), $matches)) { + $newPackage = $pool->literalToPackage($matches[0]); if ($task === 'force-links' && $newPackage) { $package->setRequires($newPackage->getRequires()); @@ -1126,12 +1128,12 @@ class Installer } /** - * @param RepositorySet $repositorySet + * @param Pool $pool * @param PolicyInterface $policy * @param WritableRepositoryInterface $localRepo * @param array $repositories */ - private function processPackageUrls($repositorySet, $policy, $localRepo, $repositories) + private function processPackageUrls(Pool $pool, $policy, $localRepo, $repositories) { if (!$this->update) { return; @@ -1140,8 +1142,8 @@ class Installer $rootRefs = $this->package->getReferences(); foreach ($localRepo->getCanonicalPackages() as $package) { - // find similar packages (name/version) in all repositories - $matches = $repositorySet->findPackages($package->getName(), new Constraint('=', $package->getVersion())); + // find similar packages (name/version) in pool + $matches = $pool->whatProvides($package->getName(), new Constraint('=', $package->getVersion()), true); foreach ($matches as $index => $match) { // skip local packages if (!in_array($match->getRepository(), $repositories, true)) { @@ -1153,8 +1155,8 @@ class Installer } // select preferred package according to policy rules - if ($matches && $matches = $policy->selectPreferredPackages($repositorySet->getPoolTemp(), array(), $matches)) { // TODO get rid of pool - $newPackage = $repositorySet->getPoolTemp()->literalToPackage($matches[0]); + if ($matches && $matches = $policy->selectPreferredPackages($pool, array(), $matches)) { + $newPackage = $pool->literalToPackage($matches[0]); // update the dist and source URLs $sourceUrl = $package->getSourceUrl(); diff --git a/src/Composer/Repository/RepositorySet.php b/src/Composer/Repository/RepositorySet.php index f83de3b14..4bea054f9 100644 --- a/src/Composer/Repository/RepositorySet.php +++ b/src/Composer/Repository/RepositorySet.php @@ -18,6 +18,7 @@ use Composer\Package\Version\VersionParser; use Composer\Repository\CompositeRepository; use Composer\Repository\PlatformRepository; use Composer\Semver\Constraint\ConstraintInterface; +use Composer\Test\DependencyResolver\PoolTest; /** * @author Nils Adermann @@ -28,17 +29,17 @@ class RepositorySet private $rootAliases; /** @var RepositoryInterface[] */ - private $repositories; + private $repositories = array(); /** @var ComposerRepository[] */ - private $providerRepos; + private $providerRepos = array(); private $acceptableStabilities; private $stabilityFlags; protected $filterRequires; /** @var Pool */ - private $pool; // TODO remove this + private $pool; public function __construct(array $rootAliases = array(), $minimumStability = 'stable', array $stabilityFlags = array(), array $filterRequires = array()) { @@ -66,6 +67,10 @@ class RepositorySet */ public function addRepository(RepositoryInterface $repo) { + if ($this->pool) { + throw new \RuntimeException("Pool has already been created from this repository set, it cannot be modified anymore."); + } + if ($repo instanceof CompositeRepository) { $repos = $repo->getRepositories(); } else { @@ -135,10 +140,6 @@ class RepositorySet */ public function createPool() { - if ($this->pool) { - return $this->pool; - } - $this->pool = new Pool($this->acceptableStabilities, $this->stabilityFlags, $this->filterRequires); foreach ($this->repositories as $repository) { @@ -148,13 +149,12 @@ class RepositorySet return $this->pool; } - // TODO get rid of this function - public function getPoolTemp() + /** + * 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() { - if (!$this->pool) { - return $this->createPool(); - } else { - return $this->pool; - } + return $this->pool; } } diff --git a/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php b/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php index 34a1c092b..dedb452e2 100644 --- a/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php +++ b/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php @@ -47,7 +47,7 @@ class DefaultPolicyTest extends TestCase $this->repo->addPackage($packageA = $this->getPackage('A', '1.0')); $this->repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($packageA->getId()); $expected = array($packageA->getId()); @@ -63,7 +63,7 @@ class DefaultPolicyTest extends TestCase $this->repo->addPackage($packageA2 = $this->getPackage('A', '2.0')); $this->repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($packageA1->getId(), $packageA2->getId()); $expected = array($packageA2->getId()); @@ -79,7 +79,7 @@ class DefaultPolicyTest extends TestCase $this->repo->addPackage($packageA2 = $this->getPackage('A', '1.0.1-alpha')); $this->repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($packageA1->getId(), $packageA2->getId()); $expected = array($packageA2->getId()); @@ -95,7 +95,7 @@ class DefaultPolicyTest extends TestCase $this->repo->addPackage($packageA2 = $this->getPackage('A', '1.0.1-alpha')); $this->repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($packageA1->getId(), $packageA2->getId()); $expected = array($packageA1->getId()); @@ -112,7 +112,7 @@ class DefaultPolicyTest extends TestCase $this->repo->addPackage($packageA2 = $this->getPackage('A', '1.0.0')); $this->repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($packageA1->getId(), $packageA2->getId()); $expected = array($packageA2->getId()); @@ -129,7 +129,7 @@ class DefaultPolicyTest extends TestCase $this->repositorySet->addRepository($this->repoInstalled); $this->repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($packageA->getId(), $packageAInstalled->getId()); $expected = array($packageA->getId()); @@ -150,7 +150,7 @@ class DefaultPolicyTest extends TestCase $this->repositorySet->addRepository($otherRepository); $this->repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($packageA->getId(), $packageAImportant->getId()); $expected = array($packageAImportant->getId()); @@ -173,7 +173,7 @@ class DefaultPolicyTest extends TestCase $this->repositorySet->addRepository($repo1); $this->repositorySet->addRepository($repo2); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($package1->getId(), $package2->getId(), $package3->getId(), $package4->getId()); $expected = array($package2->getId()); @@ -185,7 +185,7 @@ class DefaultPolicyTest extends TestCase $this->repositorySet->addRepository($repo2); $this->repositorySet->addRepository($repo1); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $expected = array($package4->getId()); $selected = $this->policy->selectPreferredPackages($pool, array(), $literals); @@ -209,7 +209,7 @@ class DefaultPolicyTest extends TestCase $this->repositorySet->addRepository($repoImportant); $this->repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $packages = $pool->whatProvides('a', new Constraint('=', '2.1.9999999.9999999-dev')); $literals = array(); @@ -234,7 +234,7 @@ class DefaultPolicyTest extends TestCase $this->repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($packageA->getId(), $packageB->getId()); $expected = $literals; @@ -253,7 +253,7 @@ class DefaultPolicyTest extends TestCase $this->repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($packageA->getId(), $packageB->getId()); $expected = $literals; @@ -274,7 +274,7 @@ class DefaultPolicyTest extends TestCase $this->repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($packageA->getId(), $packageB->getId()); $expected = $literals; @@ -290,7 +290,7 @@ class DefaultPolicyTest extends TestCase $repositorySet = new RepositorySet(array(), 'dev'); $repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($packageA->getId(), $packageB->getId()); $expected = $literals; @@ -317,7 +317,7 @@ class DefaultPolicyTest extends TestCase $this->repo->addPackage($packageA2 = $this->getPackage('A', '2.0')); $this->repositorySet->addRepository($this->repo); - $pool = $this->repositorySet->getPoolTemp(); + $pool = $this->repositorySet->createPool(); $literals = array($packageA1->getId(), $packageA2->getId()); $expected = array($packageA1->getId()); diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index 1a43c5b60..e4b02968f 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/tests/Composer/Test/DependencyResolver/SolverTest.php @@ -40,7 +40,6 @@ class SolverTest extends TestCase $this->request = new Request($this->repoSet); $this->policy = new DefaultPolicy; - $this->solver = new Solver($this->policy, $this->repoSet, $this->repoInstalled, new NullIO()); } public function testSolverInstallSingle() @@ -95,6 +94,8 @@ class SolverTest extends TestCase $this->repoSet->addRepository($repo1); $this->repoSet->addRepository($repo2); + $this->solver = new Solver($this->policy, $this->repoSet->createPool(), $this->repoInstalled, new NullIO()); + $this->request->install('foo'); $this->checkSolverResult(array( @@ -842,6 +843,8 @@ class SolverTest extends TestCase { $this->repoSet->addRepository($this->repoInstalled); $this->repoSet->addRepository($this->repo); + + $this->solver = new Solver($this->policy, $this->repoSet->createPool(), $this->repoInstalled, new NullIO()); } protected function checkSolverResult(array $expected)