Refactor require-dev handling to use one single repository and a one pass solving, fixes #719, fixes #1185, fixes #1330, fixes #789, fixes #640

src/Composer/Factory.php

@@ -291,7 +291,6 @@ class Factory
     protected function addLocalRepository(RepositoryManager $rm, $vendorDir)
     {
         $rm->setLocalRepository(new Repository\InstalledFilesystemRepository(new JsonFile($vendorDir.'/composer/installed.json')));
-        $rm->setLocalDevRepository(new Repository\InstalledFilesystemRepository(new JsonFile($vendorDir.'/composer/installed_dev.json')));
     }
 
     /**
@@ -345,12 +344,10 @@ class Factory
      */
     protected function purgePackages(Repository\RepositoryManager $rm, Installer\InstallationManager $im)
     {
-        foreach ($rm->getLocalRepositories() as $repo) {
+        $repo = $rm->getLocalRepository();
-            /* @var $repo   Repository\WritableRepositoryInterface */
+        foreach ($repo->getPackages() as $package) {
-            foreach ($repo->getPackages() as $package) {
+            if (!$im->isPackageInstalled($repo, $package)) {
-                if (!$im->isPackageInstalled($repo, $package)) {
+                $repo->removePackage($package);
-                    $repo->removePackage($package);
-                }
             }
         }
     }

src/Composer/Installer.php

@@ -160,13 +160,12 @@ class Installer
         $installedRootPackage->setRequires(array());
         $installedRootPackage->setDevRequires(array());
 
+        $localRepo = $this->repositoryManager->getLocalRepository();
         $platformRepo = new PlatformRepository();
-        $repos = array_merge(
+        $repos = array(
-            $this->repositoryManager->getLocalRepositories(),
+            $localRepo,
-            array(
+            new InstalledArrayRepository(array($installedRootPackage)),
-                new InstalledArrayRepository(array($installedRootPackage)),
+            $platformRepo,
-                $platformRepo,
-            )
         );
         $installedRepo = new CompositeRepository($repos);
         if ($this->additionalInstalledRepository) {
@@ -184,14 +183,9 @@ class Installer
 
         try {
             $this->suggestedPackages = array();
-            if (!$this->doInstall($this->repositoryManager->getLocalRepository(), $installedRepo, $aliases)) {
+            if (!$this->doInstall($localRepo, $installedRepo, $platformRepo, $aliases, $this->devMode)) {
                 return false;
             }
-            if ($this->devMode) {
-                if (!$this->doInstall($this->repositoryManager->getLocalDevRepository(), $installedRepo, $aliases, true)) {
-                    return false;
-                }
-            }
         } catch (\Exception $e) {
             $this->installationManager->notifyInstalls();
 
@@ -214,9 +208,34 @@ class Installer
         if (!$this->dryRun) {
             // write lock
             if ($this->update || !$this->locker->isLocked()) {
+                $devPackages = $this->devMode ? array() : null;
+                $localRepo->reload();
+
+                // split dev and non-dev requirements by checking what would be removed if we update without the dev requirements
+                if ($this->devMode && $this->package->getDevRequires()) {
+                    $policy = new DefaultPolicy();
+                    $pool = $this->createPool();
+                    $pool->addRepository($installedRepo, $aliases);
+
+                    // creating requirements request
+                    $request = $this->createRequest($pool, $this->package, $platformRepo);
+                    $request->updateAll();
+                    foreach ($this->package->getRequires() as $link) {
+                        $request->install($link->getTarget(), $link->getConstraint());
+                    }
+
+                    $solver = new Solver($policy, $pool, $installedRepo);
+                    $ops = $solver->solve($request);
+                    foreach ($ops as $op) {
+                        if ($op->getJobType() === 'uninstall') {
+                            $devPackages[] = $op->getPackage();
+                        }
+                    }
+                }
+
                 $updatedLock = $this->locker->setLockData(
-                    $this->repositoryManager->getLocalRepository()->getPackages(),
+                    array_diff($localRepo->getPackages(), (array) $devPackages),
-                    $this->devMode ? $this->repositoryManager->getLocalDevRepository()->getPackages() : null,
+                    $devPackages,
                     $aliases,
                     $this->package->getMinimumStability(),
                     $this->package->getStabilityFlags()
@@ -228,8 +247,7 @@ class Installer
 
             // write autoloader
             $this->io->write('<info>Generating autoload files</info>');
-            $localRepos = new CompositeRepository($this->repositoryManager->getLocalRepositories());
+            $this->autoloadGenerator->dump($this->config, $localRepo, $this->package, $this->installationManager, 'composer', $this->optimizeAutoloader);
-            $this->autoloadGenerator->dump($this->config, $localRepos, $this->package, $this->installationManager, 'composer', $this->optimizeAutoloader);
 
             if ($this->runScripts) {
                 // dispatch post event
@@ -241,27 +259,22 @@ class Installer
         return true;
     }
 
-    protected function doInstall($localRepo, $installedRepo, $aliases, $devMode = false)
+    protected function doInstall($localRepo, $installedRepo, $platformRepo, $aliases, $withDevReqs)
     {
-        $minimumStability = $this->package->getMinimumStability();
-        $stabilityFlags = $this->package->getStabilityFlags();
-
         // init vars
         $lockedRepository = null;
         $repositories = null;
 
         // initialize locker to create aliased packages
         $installFromLock = false;
-        if (!$this->update && $this->locker->isLocked($devMode)) {
+        if (!$this->update && $this->locker->isLocked()) {
             $installFromLock = true;
-            $lockedRepository = $this->locker->getLockedRepository($devMode);
+            $lockedRepository = $this->locker->getLockedRepository($withDevReqs);
-            $minimumStability = $this->locker->getMinimumStability();
-            $stabilityFlags = $this->locker->getStabilityFlags();
         }
 
         $this->whitelistUpdateDependencies(
             $localRepo,
-            $devMode,
+            $withDevReqs,
             $this->package->getRequires(),
             $this->package->getDevRequires()
         );
@@ -270,13 +283,13 @@ class Installer
 
         // creating repository pool
         $policy = new DefaultPolicy();
-        $pool = new Pool($minimumStability, $stabilityFlags);
+        $pool = $this->createPool();
         $pool->addRepository($installedRepo, $aliases);
         if ($installFromLock) {
             $pool->addRepository($lockedRepository, $aliases);
         }
 
-        if (!$installFromLock || !$this->locker->isCompleteFormat($devMode)) {
+        if (!$installFromLock || !$this->locker->isCompleteFormat()) {
             $repositories = $this->repositoryManager->getRepositories();
             foreach ($repositories as $repository) {
                 $pool->addRepository($repository, $aliases);
@@ -284,30 +297,30 @@ class Installer
         }
 
         // creating requirements request
-        $request = new Request($pool);
+        $request = $this->createRequest($pool, $this->package, $platformRepo);
-
-        $constraint = new VersionConstraint('=', $this->package->getVersion());
-        $constraint->setPrettyString($this->package->getPrettyVersion());
-        $request->install($this->package->getName(), $constraint);
 
         if ($this->update) {
-            $this->io->write('<info>Updating '.($devMode ? 'dev ': '').'dependencies</info>');
+            $this->io->write('<info>Updating dependencies'.($withDevReqs?' (including require-dev)':'').'</info>');
 
             $request->updateAll();
 
-            $links = $devMode ? $this->package->getDevRequires() : $this->package->getRequires();
+            if ($withDevReqs) {
+                $links = array_merge($this->package->getRequires(), $this->package->getDevRequires());
+            } else {
+                $links = $this->package->getRequires();
+            }
 
             foreach ($links as $link) {
                 $request->install($link->getTarget(), $link->getConstraint());
             }
         } elseif ($installFromLock) {
-            $this->io->write('<info>Installing '.($devMode ? 'dev ': '').'dependencies from lock file</info>');
+            $this->io->write('<info>Installing dependencies'.($withDevReqs?' (including require-dev)':'').' from lock file</info>');
 
-            if (!$this->locker->isCompleteFormat($devMode)) {
+            if (!$this->locker->isCompleteFormat($withDevReqs)) {
                 $this->io->write('<warning>Warning: Your lock file is in a deprecated format. It will most likely take a *long* time for composer to install dependencies, and may cause dependency solving issues.</warning>');
             }
 
-            if (!$this->locker->isFresh() && !$devMode) {
+            if (!$this->locker->isFresh()) {
                 $this->io->write('<warning>Warning: The lock file is not up to date with the latest changes in composer.json. You may be getting outdated dependencies. Run update to update them.</warning>');
             }
 
@@ -321,40 +334,24 @@ class Installer
                 $request->install($package->getName(), $constraint);
             }
         } else {
-            $this->io->write('<info>Installing '.($devMode ? 'dev ': '').'dependencies</info>');
+            $this->io->write('<info>Installing dependencies'.($withDevReqs?' (including require-dev)':'').'</info>');
 
-            $links = $devMode ? $this->package->getDevRequires() : $this->package->getRequires();
+            if ($withDevReqs) {
+                $links = array_merge($this->package->getRequires(), $this->package->getDevRequires());
+            } else {
+                $links = $this->package->getRequires();
+            }
 
             foreach ($links as $link) {
                 $request->install($link->getTarget(), $link->getConstraint());
             }
         }
 
-        // fix the version of all installed packages (+ platform) that are not
-        // in the current local repo to prevent rogue updates (e.g. non-dev
-        // updating when in dev)
-        foreach ($installedRepo->getPackages() as $package) {
-            if ($package->getRepository() === $localRepo) {
-                continue;
-            }
-
-            $constraint = new VersionConstraint('=', $package->getVersion());
-            $constraint->setPrettyString($package->getPrettyVersion());
-
-            if (!($package->getRepository() instanceof PlatformRepository)
-                || !($provided = $this->package->getProvides())
-                || !isset($provided[$package->getName()])
-                || !$provided[$package->getName()]->getConstraint()->matches($constraint)
-            ) {
-                $request->install($package->getName(), $constraint);
-            }
-        }
-
         // if the updateWhitelist is enabled, packages not in it are also fixed
         // to the version specified in the lock, or their currently installed version
         if ($this->update && $this->updateWhitelist) {
-            if ($this->locker->isLocked($devMode)) {
+            if ($this->locker->isLocked()) {
-                $currentPackages = $this->locker->getLockedRepository($devMode)->getPackages();
+                $currentPackages = $this->locker->getLockedRepository($withDevReqs)->getPackages();
             } else {
                 $currentPackages = $installedRepo->getPackages();
             }
@@ -397,19 +394,6 @@ class Installer
             return false;
         }
 
-        if ($devMode) {
-            // remove bogus operations that the solver creates for stuff that was force-updated in the non-dev pass
-            // TODO this should not be necessary ideally, but it seems to work around the problem quite well
-            foreach ($operations as $index => $op) {
-                if ('update' === $op->getJobType() && $op->getInitialPackage()->getUniqueName() === $op->getTargetPackage()->getUniqueName()
-                    && $op->getInitialPackage()->getSourceReference() === $op->getTargetPackage()->getSourceReference()
-                    && $op->getInitialPackage()->getDistReference() === $op->getTargetPackage()->getDistReference()
-                ) {
-                    unset($operations[$index]);
-                }
-            }
-        }
-
         // force dev packages to be updated if we update or install from a (potentially new) lock
         $operations = $this->processDevPackages($localRepo, $pool, $policy, $repositories, $lockedRepository, $installFromLock, 'force-updates', $operations);
 
@@ -472,6 +456,45 @@ class Installer
         return true;
     }
 
+    private function createPool()
+    {
+        $minimumStability = $this->package->getMinimumStability();
+        $stabilityFlags = $this->package->getStabilityFlags();
+
+        if (!$this->update && $this->locker->isLocked()) {
+            $minimumStability = $this->locker->getMinimumStability();
+            $stabilityFlags = $this->locker->getStabilityFlags();
+        }
+
+        return new Pool($minimumStability, $stabilityFlags);
+    }
+
+    private function createRequest(Pool $pool, RootPackageInterface $rootPackage, PlatformRepository $platformRepo)
+    {
+        $request = new Request($pool);
+
+        $constraint = new VersionConstraint('=', $rootPackage->getVersion());
+        $constraint->setPrettyString($rootPackage->getPrettyVersion());
+        $request->install($rootPackage->getName(), $constraint);
+
+        // fix the version of all installed packages (+ platform) that are not
+        // in the current local repo to prevent rogue updates (e.g. non-dev
+        // updating when in dev)
+        foreach ($platformRepo->getPackages() as $package) {
+            $constraint = new VersionConstraint('=', $package->getVersion());
+            $constraint->setPrettyString($package->getPrettyVersion());
+
+            if (!($provided = $rootPackage->getProvides())
+                || !isset($provided[$package->getName()])
+                || !$provided[$package->getName()]->getConstraint()->matches($constraint)
+            ) {
+                $request->install($package->getName(), $constraint);
+            }
+        }
+
+        return $request;
+    }
+
     private function processDevPackages($localRepo, $pool, $policy, $repositories, $lockedRepository, $installFromLock, $task, array $operations = null)
     {
         if ($task === 'force-updates' && null === $operations) {
@@ -732,18 +755,6 @@ class Installer
         $rm->setLocalRepository(
             new InstalledArrayRepository($packages)
         );
-
-        $packages = array_map(function ($p) {
-            return clone $p;
-        }, $rm->getLocalDevRepository()->getPackages());
-        foreach ($packages as $key => $package) {
-            if ($package instanceof AliasPackage) {
-                unset($packages[$key]);
-            }
-        }
-        $rm->setLocalDevRepository(
-            new InstalledArrayRepository($packages)
-        );
     }
 
     /**

src/Composer/Package/Locker.php

@@ -58,19 +58,15 @@ class Locker
     /**
      * Checks whether locker were been locked (lockfile found).
      *
-     * @param  bool $dev true to check if dev packages are locked
      * @return bool
      */
-    public function isLocked($dev = false)
+    public function isLocked()
     {
         if (!$this->lockFile->exists()) {
             return false;
         }
 
         $data = $this->getLockData();
-        if ($dev) {
-            return isset($data['packages-dev']);
-        }
 
         return isset($data['packages']);
     }
@@ -90,13 +86,12 @@ class Locker
     /**
      * Checks whether the lock file is in the new complete format or not
      *
-     * @param  bool $dev true to check in dev mode
      * @return bool
      */
-    public function isCompleteFormat($dev)
+    public function isCompleteFormat()
     {
         $lockData = $this->getLockData();
-        $lockedPackages = $dev ? $lockData['packages-dev'] : $lockData['packages'];
+        $lockedPackages = $lockData['packages'];
 
         if (empty($lockedPackages) || isset($lockedPackages[0]['name'])) {
             return true;
@@ -108,15 +103,22 @@ class Locker
     /**
      * Searches and returns an array of locked packages, retrieved from registered repositories.
      *
-     * @param  bool                                     $dev true to retrieve the locked dev packages
+     * @param  bool                                     $withDevReqs true to retrieve the locked dev packages
      * @return \Composer\Repository\RepositoryInterface
      */
-    public function getLockedRepository($dev = false)
+    public function getLockedRepository($withDevReqs = false)
     {
         $lockData = $this->getLockData();
         $packages = new ArrayRepository();
 
-        $lockedPackages = $dev ? $lockData['packages-dev'] : $lockData['packages'];
+        $lockedPackages = $lockData['packages'];
+        if ($withDevReqs) {
+            if (isset($lockData['packages-dev'])) {
+                $lockedPackages = array_merge($lockedPackages, $lockData['packages-dev']);
+            } else {
+                throw new \RuntimeException('The lock file does not contain require-dev information, run install without --dev or run update to install those packages.');
+            }
+        }
 
         if (empty($lockedPackages)) {
             return $packages;
@@ -131,7 +133,7 @@ class Locker
         }
 
         // legacy lock file support
-        $repo = $dev ? $this->repositoryManager->getLocalDevRepository() : $this->repositoryManager->getLocalRepository();
+        $repo = $this->repositoryManager->getLocalRepository();
         foreach ($lockedPackages as $info) {
             $resolvedVersion = !empty($info['alias-version']) ? $info['alias-version'] : $info['version'];
 

src/Composer/Repository/RepositoryManager.php

@@ -25,7 +25,6 @@ use Composer\Config;
 class RepositoryManager
 {
     private $localRepository;
-    private $localDevRepository;
     private $repositories = array();
     private $repositoryClasses = array();
     private $io;
@@ -143,26 +142,6 @@ class RepositoryManager
         return $this->localRepository;
     }
 
-    /**
-     * Sets localDev repository for the project.
-     *
-     * @param RepositoryInterface $repository repository instance
-     */
-    public function setLocalDevRepository(RepositoryInterface $repository)
-    {
-        $this->localDevRepository = $repository;
-    }
-
-    /**
-     * Returns localDev repository for the project.
-     *
-     * @return RepositoryInterface
-     */
-    public function getLocalDevRepository()
-    {
-        return $this->localDevRepository;
-    }
-
     /**
      * Returns all local repositories for the project.
      *
@@ -170,6 +149,8 @@ class RepositoryManager
      */
     public function getLocalRepositories()
     {
-        return array($this->localRepository, $this->localDevRepository);
+        trigger_error('This method is deprecated, use getLocalRepository instead since the getLocalDevRepository is now gone', E_USER_DEPRECATED);
+
+        return array($this->localRepository);
     }
 }

tests/Composer/Test/CacheTest.php

@@ -29,7 +29,7 @@ class CacheTest extends TestCase
             file_put_contents("{$this->root}/cached.file{$i}.zip", $zeros);
             $this->files[] = new \SplFileInfo("{$this->root}/cached.file{$i}.zip");
         }
-        $this->finder = $this->getMock('Symfony\Component\Finder\Finder');
+        $this->finder = $this->getMockBuilder('Symfony\Component\Finder\Finder')->disableOriginalConstructor()->getMock();
 
         $io = $this->getMock('Composer\IO\IOInterface');
         $this->cache = $this->getMock(
@@ -65,7 +65,7 @@ class CacheTest extends TestCase
 
     public function testRemoveFilesWhenCacheIsTooLarge()
     {
-        $emptyFinder = $this->getMock('Symfony\Component\Finder\Finder');
+        $emptyFinder = $this->getMockBuilder('Symfony\Component\Finder\Finder')->disableOriginalConstructor()->getMock();
         $emptyFinder
             ->expects($this->once())
             ->method('getIterator')

tests/Composer/Test/Fixtures/installer/update-all.test

@@ -30,10 +30,7 @@ Updates updateable packages
 --INSTALLED--
 [
     { "name": "a/a", "version": "1.0.0" },
-    { "name": "a/c", "version": "1.0.0" }
+    { "name": "a/c", "version": "1.0.0" },
-]
---INSTALLED-DEV--
-[
     { "name": "a/b", "version": "1.0.0" }
 ]
 --RUN--

tests/Composer/Test/InstallerTest.php

@@ -41,7 +41,6 @@ class InstallerTest extends TestCase
 
         $repositoryManager = new RepositoryManager($io, $config);
         $repositoryManager->setLocalRepository(new WritableRepositoryMock());
-        $repositoryManager->setLocalDevRepository(new WritableRepositoryMock());
 
         if (!is_array($repositories)) {
             $repositories = array($repositories);
@@ -124,7 +123,7 @@ class InstallerTest extends TestCase
     /**
      * @dataProvider getIntegrationTests
      */
-    public function testIntegration($file, $message, $condition, $composerConfig, $lock, $installed, $installedDev, $run, $expectLock, $expectOutput, $expect)
+    public function testIntegration($file, $message, $condition, $composerConfig, $lock, $installed, $run, $expectLock, $expectOutput, $expect)
     {
         if ($condition) {
             eval('$res = '.$condition.';');
@@ -151,17 +150,8 @@ class InstallerTest extends TestCase
             ->method('exists')
             ->will($this->returnValue(true));
 
-        $devJsonMock = $this->getMockBuilder('Composer\Json\JsonFile')->disableOriginalConstructor()->getMock();
-        $devJsonMock->expects($this->any())
-            ->method('read')
-            ->will($this->returnValue($installedDev));
-        $devJsonMock->expects($this->any())
-            ->method('exists')
-            ->will($this->returnValue(true));
-
         $repositoryManager = $composer->getRepositoryManager();
         $repositoryManager->setLocalRepository(new InstalledFilesystemRepositoryMock($jsonMock));
-        $repositoryManager->setLocalDevRepository(new InstalledFilesystemRepositoryMock($devJsonMock));
 
         $lockJsonMock = $this->getMockBuilder('Composer\Json\JsonFile')->disableOriginalConstructor()->getMock();
         $lockJsonMock->expects($this->any())
@@ -253,7 +243,6 @@ class InstallerTest extends TestCase
                 --COMPOSER--\s*(?P<composer>'.$content.')\s*
                 (?:--LOCK--\s*(?P<lock>'.$content.'))?\s*
                 (?:--INSTALLED--\s*(?P<installed>'.$content.'))?\s*
-                (?:--INSTALLED-DEV--\s*(?P<installedDev>'.$content.'))?\s*
                 --RUN--\s*(?P<run>.*?)\s*
                 (?:--EXPECT-LOCK--\s*(?P<expectLock>'.$content.'))?\s*
                 (?:--EXPECT-OUTPUT--\s*(?P<expectOutput>'.$content.'))?\s*
@@ -279,9 +268,6 @@ class InstallerTest extends TestCase
                     if (!empty($match['installed'])) {
                         $installed = JsonFile::parseJson($match['installed']);
                     }
-                    if (!empty($match['installedDev'])) {
-                        $installedDev = JsonFile::parseJson($match['installedDev']);
-                    }
                     $run = $match['run'];
                     if (!empty($match['expectLock'])) {
                         $expectLock = JsonFile::parseJson($match['expectLock']);
@@ -295,7 +281,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, $installedDev, $run, $expectLock, $expectOutput, $expect);
+            $tests[] = array(str_replace($fixturesDir.'/', '', $file), $message, $condition, $composer, $lock, $installed, $run, $expectLock, $expectOutput, $expect);
         }
 
         return $tests;