Restore dev package extraction

New approach is to use only the solved set of packages as input and then
to resolve with only the non-dev requirements and to mark everything as
dev that is not part of the result set, rather than transitioning a
temporary local repo state by uninstalling dev packages.

src/Composer/DependencyResolver/LocalRepoTransaction.php

@@ -33,6 +33,9 @@ class LocalRepoTransaction
     /** @var RepositoryInterface */
     protected $localRepository;
 
+    /**
+     * Reassigns ids for all packages in the lockedrepository
+     */
     public function __construct(RepositoryInterface $lockedRepository, $localRepository)
     {
         $this->localRepository = $localRepository;
@@ -50,7 +53,10 @@ class LocalRepoTransaction
             return strcmp($b->getName(), $a->getName());
         };
 
+        $id = 1;
+        $this->lockedPackages = array();
         foreach ($lockedRepository->getPackages() as $package) {
+            $package->id = $id++;
             $this->lockedPackages[$package->id] = $package;
             foreach ($package->getNames() as $name) {
                 $this->lockedPackagesByName[$name][] = $package;

src/Composer/DependencyResolver/LockTransaction.php

@@ -16,7 +16,9 @@ use Composer\DependencyResolver\Operation\OperationInterface;
 use Composer\Package\AliasPackage;
 use Composer\Package\RootAliasPackage;
 use Composer\Package\RootPackageInterface;
+use Composer\Repository\ArrayRepository;
 use Composer\Repository\RepositoryInterface;
+use Composer\Test\Repository\ArrayRepositoryTest;
 
 /**
  * @author Nils Adermann <naderman@naderman.de>
@@ -40,7 +42,8 @@ class LockTransaction
     protected $unlockableMap;
 
     protected $decisions;
-    protected $transaction;
+
+    protected $resultPackages;
 
     public function __construct($policy, $pool, $presentMap, $unlockableMap, $decisions)
     {
@@ -104,31 +107,54 @@ class LockTransaction
             }
         }
 
+        $this->setResultPackages();
+
         return $operations;
     }
 
-    // TODO additionalFixedRepository needs to be looked at here as well?
-    public function getNewLockNonDevPackages()
+    // TODO make this a bit prettier instead of the two text indexes?
+    public function setResultPackages()
     {
-        $packages = array();
+        $this->resultPackages = array('non-dev' => array(), 'dev' => array());
         foreach ($this->decisions as $i => $decision) {
             $literal = $decision[Decisions::DECISION_LITERAL];
 
             if ($literal > 0) {
                 $package = $this->pool->literalToPackage($literal);
-                if (!isset($this->unlockableMap[$package->id]) && !($package instanceof AliasPackage) && !($package instanceof RootAliasPackage)) {
-                    $packages[] = $package;
+                if (!isset($this->unlockableMap[$package->id])) {
+                    $this->resultPackages['non-dev'][] = $package;
                 }
             }
         }
+    }
 
-        return $packages;
+    public function setNonDevPackages(LockTransaction $extractionResult)
+    {
+        $packages = $extractionResult->getNewLockPackages(false);
+
+        $this->resultPackages['dev'] = $this->resultPackages['non-dev'];
+        $this->resultPackages['non-dev'] = array();
+
+        foreach ($packages as $package) {
+            foreach ($this->resultPackages['dev'] as $i => $resultPackage) {
+                if ($package->getName() == $resultPackage->getName()) {
+                    $this->resultPackages['non-dev'][] = $resultPackage;
+                    unset($this->resultPackages['dev'][$i]);
+                }
+            }
+        }
     }
 
-    public function getNewLockDevPackages()
+    // TODO additionalFixedRepository needs to be looked at here as well?
+    public function getNewLockPackages($devMode)
     {
-        // TODO this is empty?
         $packages = array();
+        foreach ($this->resultPackages[$devMode ? 'dev' : 'non-dev'] as $package) {
+            if (!($package instanceof AliasPackage) && !($package instanceof RootAliasPackage)) {
+                $packages[] = $package;
+            }
+        }
+
         return $packages;
     }
 

src/Composer/Installer.php

@@ -15,6 +15,7 @@ namespace Composer;
 use Composer\Autoload\AutoloadGenerator;
 use Composer\DependencyResolver\DefaultPolicy;
 use Composer\DependencyResolver\LocalRepoTransaction;
+use Composer\DependencyResolver\LockTransaction;
 use Composer\DependencyResolver\Operation\UpdateOperation;
 use Composer\DependencyResolver\Operation\InstallOperation;
 use Composer\DependencyResolver\Operation\UninstallOperation;
@@ -40,6 +41,7 @@ use Composer\Package\Link;
 use Composer\Package\Loader\ArrayLoader;
 use Composer\Package\Dumper\ArrayDumper;
 use Composer\Package\Package;
+use Composer\Repository\ArrayRepository;
 use Composer\Repository\RepositorySet;
 use Composer\Semver\Constraint\Constraint;
 use Composer\Package\Locker;
@@ -418,6 +420,8 @@ class Installer
             $this->io->writeError('Nothing to modify in lock file');
         }
 
+        $this->extractDevPackages($lockTransaction, $platformRepo, $aliases, $policy);
+
         // write lock
         $platformReqs = $this->extractPlatformRequirements($this->package->getRequires());
         $platformDevReqs = $this->extractPlatformRequirements($this->package->getDevRequires());
@@ -485,8 +489,8 @@ class Installer
         }
 
         $updatedLock = $this->locker->setLockData(
-            $lockTransaction->getNewLockNonDevPackages(),
-            $lockTransaction->getNewLockDevPackages(),
+            $lockTransaction->getNewLockPackages(false),
+            $lockTransaction->getNewLockPackages(true),
             $platformReqs,
             $platformDevReqs,
             $aliases,
@@ -509,6 +513,124 @@ class Installer
         return 0;
     }
 
+    /**
+     * Run the solver a second time on top of the existing update result with only the current result set in the pool
+     * and see what packages would get removed if we only had the non-dev packages in the solver request
+     */
+    protected function extractDevPackages(LockTransaction $lockTransaction, $platformRepo, $aliases, $policy)
+    {
+        if (!$this->package->getDevRequires()) {
+            return array();
+        }
+
+        ;
+
+        $resultRepo = new ArrayRepository(array());
+        $loader = new ArrayLoader(null, true);
+        $dumper = new ArrayDumper();
+        foreach ($lockTransaction->getNewLockPackages(false) as $pkg) {
+            $resultRepo->addPackage($loader->load($dumper->dump($pkg)));
+        }
+
+        $repositorySet = $this->createRepositorySet($platformRepo, $aliases, null);
+        $repositorySet->addRepository($resultRepo);
+
+        $request = $this->createRequest($this->fixedRootPackage, $platformRepo, null);
+
+        $links = $this->package->getRequires();
+        foreach ($links as $link) {
+            $request->install($link->getTarget(), $link->getConstraint());
+        }
+
+        $pool = $repositorySet->createPool($request);
+
+        // solve dependencies
+        $solver = new Solver($policy, $pool, $this->io);
+        try {
+            $nonDevLockTransaction = $solver->solve($request, $this->ignorePlatformReqs);
+            $solver = null;
+        } catch (SolverProblemsException $e) {
+            // TODO change info message here
+            $this->io->writeError('<error>Your requirements could not be resolved to an installable set of packages.</error>', true, IOInterface::QUIET);
+            $this->io->writeError($e->getMessage());
+
+            return max(1, $e->getCode());
+        }
+
+        $lockTransaction->setNonDevPackages($nonDevLockTransaction);
+    }
+
+
+    // TODO add proper output and events to above function based on old version below
+    /**
+     * Extracts the dev packages out of the localRepo
+     *
+     * This works by faking the operations so we can see what the dev packages
+     * would be at the end of the operation execution. This lets us then remove
+     * the dev packages from the list of operations accordingly if we are in a
+     * --no-dev install or update.
+     *
+     * @return array
+    private function extractDevPackages(array $operations, RepositoryInterface $localRepo, PlatformRepository $platformRepo, array $aliases)
+    {
+        // fake-apply all operations to this clone of the local repo so we see the complete set of package we would end up with
+        $tempLocalRepo = clone $localRepo;
+        foreach ($operations as $operation) {
+            switch ($operation->getJobType()) {
+                case 'install':
+                case 'markAliasInstalled':
+                    if (!$tempLocalRepo->hasPackage($operation->getPackage())) {
+                        $tempLocalRepo->addPackage(clone $operation->getPackage());
+                    }
+                    break;
+                case 'uninstall':
+                case 'markAliasUninstalled':
+                    $tempLocalRepo->removePackage($operation->getPackage());
+                    break;
+                case 'update':
+                    $tempLocalRepo->removePackage($operation->getInitialPackage());
+                    if (!$tempLocalRepo->hasPackage($operation->getTargetPackage())) {
+                        $tempLocalRepo->addPackage(clone $operation->getTargetPackage());
+                    }
+                    break;
+                default:
+                    throw new \LogicException('Unknown type: '.$operation->getJobType());
+            }
+        }
+        // we have to reload the local repo to handle aliases properly
+        // but as it is not persisted on disk we use a loader/dumper
+        // to reload it in memory
+        $localRepo = new InstalledArrayRepository(array());
+        $loader = new ArrayLoader(null, true);
+        $dumper = new ArrayDumper();
+        foreach ($tempLocalRepo->getCanonicalPackages() as $pkg) {
+            $localRepo->addPackage($loader->load($dumper->dump($pkg)));
+        }
+        unset($tempLocalRepo, $loader, $dumper);
+        $policy = $this->createPolicy();
+        $pool = $this->createPool();
+        $installedRepo = $this->createInstalledRepo($localRepo, $platformRepo);
+        $pool->addRepository($installedRepo, $aliases);
+        // creating requirements request without dev requirements
+        $request = $this->createRequest($this->package, $platformRepo);
+        $request->updateAll();
+        foreach ($this->package->getRequires() as $link) {
+            $request->install($link->getTarget(), $link->getConstraint());
+        }
+        // solve deps to see which get removed
+        $this->eventDispatcher->dispatchInstallerEvent(InstallerEvents::PRE_DEPENDENCIES_SOLVING, false, $policy, $pool, $installedRepo, $request);
+        $solver = new Solver($policy, $pool, $installedRepo, $this->io);
+        $ops = $solver->solve($request, $this->ignorePlatformReqs);
+        $this->eventDispatcher->dispatchInstallerEvent(InstallerEvents::POST_DEPENDENCIES_SOLVING, false, $policy, $pool, $installedRepo, $request, $ops);
+        $devPackages = array();
+        foreach ($ops as $op) {
+            if ($op->getJobType() === 'uninstall') {
+                $devPackages[] = $op->getPackage();
+            }
+        }
+        return $devPackages;
+    }*/
+
     /**
      * @param  RepositoryInterface $localRepo
      * @param  RepositoryInterface $installedRepo
@@ -575,12 +697,6 @@ class Installer
 
             // TODO should we warn people / error if plugins in vendor folder do not match contents of lock file before update?
             //$this->eventDispatcher->dispatchInstallerEvent(InstallerEvents::POST_DEPENDENCIES_SOLVING, $this->devMode, $policy, $repositorySet, $installedRepo, $request, $lockTransaction);
-        } else {
-            // we still need to ensure all packages have an id for correct functionality
-            $id = 1;
-            foreach ($lockedRepository->getPackages() as $package) {
-                $package->id = $id++;
-            }
         }
 
         // TODO in how far do we need to do anything here to ensure dev packages being updated to latest in lock without version change are treated correctly?
@@ -672,6 +788,7 @@ class Installer
 
         // TODO what's the point of rootConstraints at all, we generate the package pool taking them into account anyway?
         // TODO maybe we can drop the lockedRepository here
+        // TODO if this gets called in doInstall, this->update is still true?!
         if ($this->update) {
             $minimumStability = $this->package->getMinimumStability();
             $stabilityFlags = $this->package->getStabilityFlags();

tests/Composer/Test/Fixtures/installer/update-no-dev-still-resolves-dev.test

@@ -62,7 +62,7 @@ update --no-dev
 --EXPECT--
 Uninstalling a/b (1.0.0)
 Updating a/a (1.0.0) to a/a (1.0.1)
-Updating dev/pkg (dev-master old) to dev/pkg (dev-master new)
 Installing a/c (1.0.0)
+Updating dev/pkg (dev-master old) to dev/pkg (dev-master new)
 Marking dev/pkg (1.1.x-dev new) as installed, alias of dev/pkg (dev-master new)
 Marking dev/pkg (1.0.x-dev old) as uninstalled, alias of dev/pkg (dev-master old)