Create the pool in the installer before giving it to the solver

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);

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();

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 <naderman@naderman.de>
@@ -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;
     }
 }

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());

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)