From 55b0ed8c8be74af348d9fec6702da3efd50feb46 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 10 Mar 2016 18:36:58 +0000 Subject: [PATCH] Change installs into updates if there is no lock file, simplify some code, fixes #5034 --- src/Composer/Installer.php | 232 ++++++++---------- .../functional/create-project-command.test | 2 +- .../Fixtures/installer/abandoned-listed.test | 2 +- .../installer/broken-deps-do-not-replace.test | 2 +- .../installer/github-issues-4319.test | 2 +- .../Fixtures/installer/suggest-installed.test | 2 +- .../Test/Fixtures/installer/suggest-prod.test | 2 +- .../Fixtures/installer/suggest-replaced.test | 2 +- .../installer/suggest-uninstalled.test | 2 +- 9 files changed, 116 insertions(+), 132 deletions(-) diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index 1c8d7bb41..ec2191d5f 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -171,6 +171,11 @@ class Installer gc_collect_cycles(); gc_disable(); + // Force update if there is no lock file present + if (!$this->update && !$this->locker->isLocked()) { + $this->update = true; + } + if ($this->dryRun) { $this->verbose = true; $this->runScripts = false; @@ -196,10 +201,10 @@ class Installer // create installed repo, this contains all local packages + platform packages (php & extensions) $localRepo = $this->repositoryManager->getLocalRepository(); - if (!$this->update && $this->locker->isLocked()) { - $platformOverrides = $this->locker->getPlatformOverrides(); - } else { + if ($this->update) { $platformOverrides = $this->config->get('platform') ?: array(); + } else { + $platformOverrides = $this->locker->getPlatformOverrides(); } $platformRepo = new PlatformRepository(array(), $platformOverrides); $repos = array( @@ -261,7 +266,7 @@ class Installer if (!$this->dryRun) { // write lock - if ($this->update || !$this->locker->isLocked()) { + if ($this->update) { $localRepo->reload(); // if this is not run in dev mode and the root has dev requires, the lock must @@ -362,13 +367,10 @@ class Installer $lockedRepository = null; $repositories = null; - // initialize locker to create aliased packages - $installFromLock = !$this->update && $this->locker->isLocked(); - // initialize locked repo if we are installing from lock or in a partial update // and a lock file is present as we need to force install non-whitelisted lock file // packages in that case - if ($installFromLock || (!empty($this->updateWhitelist) && $this->locker->isLocked())) { + if (!$this->update || (!empty($this->updateWhitelist) && $this->locker->isLocked())) { try { $lockedRepository = $this->locker->getLockedRepository($withDevReqs); } catch (\RuntimeException $e) { @@ -392,9 +394,9 @@ class Installer // creating repository pool $policy = $this->createPolicy(); - $pool = $this->createPool($withDevReqs, $installFromLock ? $lockedRepository : null); + $pool = $this->createPool($withDevReqs, $this->update ? null : $lockedRepository); $pool->addRepository($installedRepo, $aliases); - if (!$installFromLock) { + if ($this->update) { $repositories = $this->repositoryManager->getRepositories(); foreach ($repositories as $repository) { $pool->addRepository($repository, $aliases); @@ -410,7 +412,7 @@ class Installer // creating requirements request $request = $this->createRequest($this->package, $platformRepo); - if (!$installFromLock) { + if ($this->update) { // remove unstable packages from the localRepo if they don't match the current stability settings $removedUnstablePackages = array(); foreach ($localRepo->getPackages() as $package) { @@ -422,9 +424,7 @@ class Installer $request->remove($package->getName(), new Constraint('=', $package->getVersion())); } } - } - if ($this->update) { $this->io->writeError('Updating dependencies'.($withDevReqs ? ' (including require-dev)' : '').''); $request->updateAll(); @@ -466,7 +466,7 @@ class Installer } } } - } elseif ($installFromLock) { + } else { $this->io->writeError('Installing dependencies'.($withDevReqs ? ' (including require-dev)' : '').' from lock file'); if (!$this->locker->isFresh()) { @@ -486,22 +486,10 @@ class Installer foreach ($this->locker->getPlatformRequirements($withDevReqs) as $link) { $request->install($link->getTarget(), $link->getConstraint()); } - } else { - $this->io->writeError('Installing dependencies'.($withDevReqs ? ' (including require-dev)' : '').''); - - 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()); - } } // force dev packages to have the latest links if we update or install from a (potentially new) lock - $this->processDevPackages($localRepo, $pool, $policy, $repositories, $installedRepo, $lockedRepository, $installFromLock, $withDevReqs, 'force-links'); + $this->processDevPackages($localRepo, $pool, $policy, $repositories, $installedRepo, $lockedRepository, $withDevReqs, 'force-links'); // solve dependencies $this->eventDispatcher->dispatchInstallerEvent(InstallerEvents::PRE_DEPENDENCIES_SOLVING, $this->devMode, $policy, $pool, $installedRepo, $request); @@ -520,7 +508,7 @@ class Installer $this->io->writeError("Analyzed ".$solver->getRuleSetSize()." rules to resolve dependencies", true, IOInterface::VERBOSE); // force dev packages to be updated if we update or install from a (potentially new) lock - $operations = $this->processDevPackages($localRepo, $pool, $policy, $repositories, $installedRepo, $lockedRepository, $installFromLock, $withDevReqs, 'force-updates', $operations); + $operations = $this->processDevPackages($localRepo, $pool, $policy, $repositories, $installedRepo, $lockedRepository, $withDevReqs, 'force-updates', $operations); // execute operations if (!$operations) { @@ -536,8 +524,8 @@ class Installer $this->suggestedPackagesReporter->addSuggestionsFromPackage($operation->getPackage()); } - // not installing from lock, force dev packages' references if they're in root package refs - if (!$installFromLock) { + // updating, force dev packages' references if they're in root package refs + if ($this->update) { $package = null; if ('update' === $operation->getJobType()) { $package = $operation->getTargetPackage(); @@ -687,7 +675,15 @@ class Installer */ private function createPool($withDevReqs, RepositoryInterface $lockedRepository = null) { - if (!$this->update && $this->locker->isLocked()) { // install from lock + if ($this->update) { + $minimumStability = $this->package->getMinimumStability(); + $stabilityFlags = $this->package->getStabilityFlags(); + + $requires = $this->package->getRequires(); + if ($withDevReqs) { + $requires = array_merge($requires, $this->package->getDevRequires()); + } + } else { $minimumStability = $this->locker->getMinimumStability(); $stabilityFlags = $this->locker->getStabilityFlags(); @@ -697,14 +693,6 @@ class Installer $constraint->setPrettyString($package->getPrettyVersion()); $requires[$package->getName()] = $constraint; } - } else { - $minimumStability = $this->package->getMinimumStability(); - $stabilityFlags = $this->package->getStabilityFlags(); - - $requires = $this->package->getRequires(); - if ($withDevReqs) { - $requires = array_merge($requires, $this->package->getDevRequires()); - } } $rootConstraints = array(); @@ -730,7 +718,7 @@ class Installer { $preferStable = null; $preferLowest = null; - if (!$this->update && $this->locker->isLocked()) { + if (!$this->update) { $preferStable = $this->locker->getPreferStable(); $preferLowest = $this->locker->getPreferLowest(); } @@ -791,13 +779,12 @@ class Installer * @param array $repositories * @param RepositoryInterface $installedRepo * @param RepositoryInterface $lockedRepository - * @param bool $installFromLock * @param bool $withDevReqs * @param string $task * @param array|null $operations * @return array */ - private function processDevPackages($localRepo, $pool, $policy, $repositories, $installedRepo, $lockedRepository, $installFromLock, $withDevReqs, $task, array $operations = null) + private function processDevPackages($localRepo, $pool, $policy, $repositories, $installedRepo, $lockedRepository, $withDevReqs, $task, array $operations = null) { if ($task === 'force-updates' && null === $operations) { throw new \InvalidArgumentException('Missing operations argument'); @@ -806,7 +793,7 @@ class Installer $operations = array(); } - if (!$installFromLock && $this->updateWhitelist) { + if ($this->update && $this->updateWhitelist) { $currentPackages = $this->getCurrentPackages($withDevReqs, $installedRepo); } @@ -825,8 +812,81 @@ class Installer } } - // force update to locked version if it does not match the installed version - if ($installFromLock) { + if ($this->update) { + // skip package if the whitelist is enabled and it is not in it + if ($this->updateWhitelist && !$this->isUpdateable($package)) { + // check if non-updateable packages are out of date compared to the lock file to ensure we don't corrupt it + foreach ($currentPackages as $curPackage) { + if ($curPackage->isDev() && $curPackage->getName() === $package->getName() && $curPackage->getVersion() === $package->getVersion()) { + if ($task === 'force-links') { + $package->setRequires($curPackage->getRequires()); + $package->setConflicts($curPackage->getConflicts()); + $package->setProvides($curPackage->getProvides()); + $package->setReplaces($curPackage->getReplaces()); + } elseif ($task === 'force-updates') { + if (($curPackage->getSourceReference() && $curPackage->getSourceReference() !== $package->getSourceReference()) + || ($curPackage->getDistReference() && $curPackage->getDistReference() !== $package->getDistReference()) + ) { + $operations[] = new UpdateOperation($package, $curPackage); + } + } + + break; + } + } + + continue; + } + + // find similar packages (name/version) in all repositories + $matches = $pool->whatProvides($package->getName(), new Constraint('=', $package->getVersion())); + foreach ($matches as $index => $match) { + // skip local packages + if (!in_array($match->getRepository(), $repositories, true)) { + unset($matches[$index]); + continue; + } + + // skip providers/replacers + if ($match->getName() !== $package->getName()) { + unset($matches[$index]); + continue; + } + + $matches[$index] = $match->getId(); + } + + // select preferred package according to policy rules + if ($matches && $matches = $policy->selectPreferredPackages($pool, array(), $matches)) { + $newPackage = $pool->literalToPackage($matches[0]); + + if ($task === 'force-links' && $newPackage) { + $package->setRequires($newPackage->getRequires()); + $package->setConflicts($newPackage->getConflicts()); + $package->setProvides($newPackage->getProvides()); + $package->setReplaces($newPackage->getReplaces()); + } + + if ($task === 'force-updates' && $newPackage && ( + (($newPackage->getSourceReference() && $newPackage->getSourceReference() !== $package->getSourceReference()) + || ($newPackage->getDistReference() && $newPackage->getDistReference() !== $package->getDistReference()) + ) + )) { + $operations[] = new UpdateOperation($package, $newPackage); + } + } + + if ($task === 'force-updates') { + // force installed package to update to referenced version in root package if it does not match the installed version + $references = $this->package->getReferences(); + + if (isset($references[$package->getName()]) && $references[$package->getName()] !== $package->getSourceReference()) { + // changing the source ref to update to will be handled in the operations loop below + $operations[] = new UpdateOperation($package, clone $package); + } + } + } else { + // force update to locked version if it does not match the installed version foreach ($lockedRepository->findPackages($package->getName()) as $lockedPackage) { if ($lockedPackage->isDev() && $lockedPackage->getVersion() === $package->getVersion()) { if ($task === 'force-links') { @@ -845,82 +905,6 @@ class Installer break; } } - } else { - // force update to latest on update - if ($this->update) { - // skip package if the whitelist is enabled and it is not in it - if ($this->updateWhitelist && !$this->isUpdateable($package)) { - // check if non-updateable packages are out of date compared to the lock file to ensure we don't corrupt it - foreach ($currentPackages as $curPackage) { - if ($curPackage->isDev() && $curPackage->getName() === $package->getName() && $curPackage->getVersion() === $package->getVersion()) { - if ($task === 'force-links') { - $package->setRequires($curPackage->getRequires()); - $package->setConflicts($curPackage->getConflicts()); - $package->setProvides($curPackage->getProvides()); - $package->setReplaces($curPackage->getReplaces()); - } elseif ($task === 'force-updates') { - if (($curPackage->getSourceReference() && $curPackage->getSourceReference() !== $package->getSourceReference()) - || ($curPackage->getDistReference() && $curPackage->getDistReference() !== $package->getDistReference()) - ) { - $operations[] = new UpdateOperation($package, $curPackage); - } - } - - break; - } - } - - continue; - } - - // find similar packages (name/version) in all repositories - $matches = $pool->whatProvides($package->getName(), new Constraint('=', $package->getVersion())); - foreach ($matches as $index => $match) { - // skip local packages - if (!in_array($match->getRepository(), $repositories, true)) { - unset($matches[$index]); - continue; - } - - // skip providers/replacers - if ($match->getName() !== $package->getName()) { - unset($matches[$index]); - continue; - } - - $matches[$index] = $match->getId(); - } - - // select preferred package according to policy rules - if ($matches && $matches = $policy->selectPreferredPackages($pool, array(), $matches)) { - $newPackage = $pool->literalToPackage($matches[0]); - - if ($task === 'force-links' && $newPackage) { - $package->setRequires($newPackage->getRequires()); - $package->setConflicts($newPackage->getConflicts()); - $package->setProvides($newPackage->getProvides()); - $package->setReplaces($newPackage->getReplaces()); - } - - if ($task === 'force-updates' && $newPackage && ( - (($newPackage->getSourceReference() && $newPackage->getSourceReference() !== $package->getSourceReference()) - || ($newPackage->getDistReference() && $newPackage->getDistReference() !== $package->getDistReference()) - ) - )) { - $operations[] = new UpdateOperation($package, $newPackage); - } - } - } - - if ($task === 'force-updates') { - // force installed package to update to referenced version in root package if it does not match the installed version - $references = $this->package->getReferences(); - - if (isset($references[$package->getName()]) && $references[$package->getName()] !== $package->getSourceReference()) { - // changing the source ref to update to will be handled in the operations loop below - $operations[] = new UpdateOperation($package, clone $package); - } - } } } @@ -952,10 +936,10 @@ class Installer */ private function getRootAliases() { - if (!$this->update && $this->locker->isLocked()) { - $aliases = $this->locker->getAliases(); - } else { + if ($this->update) { $aliases = $this->package->getAliases(); + } else { + $aliases = $this->locker->getAliases(); } $normalizedAliases = array(); diff --git a/tests/Composer/Test/Fixtures/functional/create-project-command.test b/tests/Composer/Test/Fixtures/functional/create-project-command.test index 6282e6357..bdb04303c 100644 --- a/tests/Composer/Test/Fixtures/functional/create-project-command.test +++ b/tests/Composer/Test/Fixtures/functional/create-project-command.test @@ -7,7 +7,7 @@ Installing seld/jsonlint (1.0.0) Created project in %testDir% Loading composer repositories with package information -Installing dependencies (including require-dev) +Updating dependencies (including require-dev) Nothing to install or update Writing lock file Generating autoload files diff --git a/tests/Composer/Test/Fixtures/installer/abandoned-listed.test b/tests/Composer/Test/Fixtures/installer/abandoned-listed.test index 1e0b9ff2c..bc8a0cdb6 100644 --- a/tests/Composer/Test/Fixtures/installer/abandoned-listed.test +++ b/tests/Composer/Test/Fixtures/installer/abandoned-listed.test @@ -25,7 +25,7 @@ Abandoned packages are flagged install --EXPECT-OUTPUT-- Loading composer repositories with package information -Installing dependencies (including require-dev) +Updating dependencies (including require-dev) Package a/a is abandoned, you should avoid using it. No replacement was suggested. Package c/c is abandoned, you should avoid using it. Use b/b instead. Writing lock file diff --git a/tests/Composer/Test/Fixtures/installer/broken-deps-do-not-replace.test b/tests/Composer/Test/Fixtures/installer/broken-deps-do-not-replace.test index e7c6cd984..fe6b00fff 100644 --- a/tests/Composer/Test/Fixtures/installer/broken-deps-do-not-replace.test +++ b/tests/Composer/Test/Fixtures/installer/broken-deps-do-not-replace.test @@ -22,7 +22,7 @@ Broken dependencies should not lead to a replacer being installed which is not m install --EXPECT-OUTPUT-- Loading composer repositories with package information -Installing dependencies (including require-dev) +Updating dependencies (including require-dev) Your requirements could not be resolved to an installable set of packages. Problem 1 diff --git a/tests/Composer/Test/Fixtures/installer/github-issues-4319.test b/tests/Composer/Test/Fixtures/installer/github-issues-4319.test index 56536ed72..581627bf3 100644 --- a/tests/Composer/Test/Fixtures/installer/github-issues-4319.test +++ b/tests/Composer/Test/Fixtures/installer/github-issues-4319.test @@ -32,7 +32,7 @@ install --EXPECT-OUTPUT-- Loading composer repositories with package information -Installing dependencies (including require-dev) +Updating dependencies (including require-dev) Your requirements could not be resolved to an installable set of packages. Problem 1 diff --git a/tests/Composer/Test/Fixtures/installer/suggest-installed.test b/tests/Composer/Test/Fixtures/installer/suggest-installed.test index 4929f972e..df573c997 100644 --- a/tests/Composer/Test/Fixtures/installer/suggest-installed.test +++ b/tests/Composer/Test/Fixtures/installer/suggest-installed.test @@ -20,7 +20,7 @@ Suggestions are not displayed for installed packages install --EXPECT-OUTPUT-- Loading composer repositories with package information -Installing dependencies (including require-dev) +Updating dependencies (including require-dev) Writing lock file Generating autoload files diff --git a/tests/Composer/Test/Fixtures/installer/suggest-prod.test b/tests/Composer/Test/Fixtures/installer/suggest-prod.test index c89bb0c20..0fe9c8853 100644 --- a/tests/Composer/Test/Fixtures/installer/suggest-prod.test +++ b/tests/Composer/Test/Fixtures/installer/suggest-prod.test @@ -18,7 +18,7 @@ Suggestions are not displayed in non-dev mode install --no-dev --EXPECT-OUTPUT-- Loading composer repositories with package information -Installing dependencies +Updating dependencies Writing lock file Generating autoload files diff --git a/tests/Composer/Test/Fixtures/installer/suggest-replaced.test b/tests/Composer/Test/Fixtures/installer/suggest-replaced.test index 5d64d2176..38755d7fb 100644 --- a/tests/Composer/Test/Fixtures/installer/suggest-replaced.test +++ b/tests/Composer/Test/Fixtures/installer/suggest-replaced.test @@ -20,7 +20,7 @@ Suggestions are not displayed for packages if they are replaced install --EXPECT-OUTPUT-- Loading composer repositories with package information -Installing dependencies (including require-dev) +Updating dependencies (including require-dev) Writing lock file Generating autoload files diff --git a/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test b/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test index d04b6c8d5..fda020699 100644 --- a/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test +++ b/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test @@ -18,7 +18,7 @@ Suggestions are displayed install --EXPECT-OUTPUT-- Loading composer repositories with package information -Installing dependencies (including require-dev) +Updating dependencies (including require-dev) a/a suggests installing b/b (an obscure reason) Writing lock file Generating autoload files