From 06d11f2f382335c082dfa8bd19c6dc21792f1083 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 7 Sep 2019 02:28:40 +0200 Subject: [PATCH] Fix calculation of lock transaction updates and start updating output in tests --- .../LocalRepoTransaction.php | 5 +- .../DependencyResolver/LockTransaction.php | 2 +- src/Composer/DependencyResolver/Request.php | 2 +- src/Composer/Installer.php | 51 +++++++++---------- .../installer/broken-deps-do-not-replace.test | 2 +- .../installer/github-issues-4319.test | 2 +- .../installer/github-issues-4795-2.test | 28 ++++++++-- .../Test/Fixtures/installer/suggest-prod.test | 5 +- .../installer/suggest-uninstalled.test | 7 ++- ...dating-dev-from-lock-removes-old-deps.test | 2 +- 10 files changed, 67 insertions(+), 39 deletions(-) diff --git a/src/Composer/DependencyResolver/LocalRepoTransaction.php b/src/Composer/DependencyResolver/LocalRepoTransaction.php index 4ac747243..db224989d 100644 --- a/src/Composer/DependencyResolver/LocalRepoTransaction.php +++ b/src/Composer/DependencyResolver/LocalRepoTransaction.php @@ -12,6 +12,8 @@ namespace Composer\DependencyResolver; +use Composer\DependencyResolver\Operation\MarkAliasUninstalledOperation; +use Composer\DependencyResolver\Operation\UninstallOperation; use Composer\Package\AliasPackage; use Composer\Repository\PlatformRepository; use Composer\Repository\RepositoryInterface; @@ -86,7 +88,6 @@ class LocalRepoTransaction $operations = $this->movePluginsToFront($operations); $operations = $this->moveUninstallsToFront($operations); - // TODO skip updates which don't update? is this needed? we shouldn't schedule this update in the first place? /* if ('update' === $jobType) { @@ -175,7 +176,7 @@ class LocalRepoTransaction { $uninstOps = array(); foreach ($operations as $idx => $op) { - if ($op instanceof UninstallOperation) { + if ($op instanceof UninstallOperation || $op instanceof MarkAliasUninstalledOperation) { $uninstOps[] = $op; unset($operations[$idx]); } diff --git a/src/Composer/DependencyResolver/LockTransaction.php b/src/Composer/DependencyResolver/LockTransaction.php index e113988bd..63214be9d 100644 --- a/src/Composer/DependencyResolver/LockTransaction.php +++ b/src/Composer/DependencyResolver/LockTransaction.php @@ -78,8 +78,8 @@ class LockTransaction $operations[] = new Operation\UpdateOperation($lockMeansUpdateMap[abs($literal)], $package, $reason); // avoid updates to one package from multiple origins + $ignoreRemove[$lockMeansUpdateMap[abs($literal)]->id] = true; unset($lockMeansUpdateMap[abs($literal)]); - $ignoreRemove[$source->id] = true; } else { if ($package instanceof AliasPackage) { $operations[] = new Operation\MarkAliasInstalledOperation($package, $reason); diff --git a/src/Composer/DependencyResolver/Request.php b/src/Composer/DependencyResolver/Request.php index 166ebbe07..54adea9b5 100644 --- a/src/Composer/DependencyResolver/Request.php +++ b/src/Composer/DependencyResolver/Request.php @@ -90,7 +90,7 @@ class Request $presentMap = array(); if ($this->lockedRepository) { - foreach ($this->lockedRepository as $package) { + foreach ($this->lockedRepository->getPackages() as $package) { $presentMap[$package->id] = $package; } } diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index a59a4dc4c..791c7bcfd 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -351,7 +351,7 @@ class Installer } // TODO can we drop any locked packages that we have matching remote versions for? - $request = $this->createRequest($this->fixedRootPackage, $platformRepo); + $request = $this->createRequest($this->fixedRootPackage, $platformRepo, $lockedRepository); if ($lockedRepository) { // TODO do we really always need this? Maybe only to skip fix() in updateWhitelist case cause these packages get removed on full update automatically? @@ -422,23 +422,6 @@ class Installer $platformReqs = $this->extractPlatformRequirements($this->package->getRequires()); $platformDevReqs = $this->extractPlatformRequirements($this->package->getDevRequires()); - $updatedLock = $this->locker->setLockData( - $lockTransaction->getNewLockNonDevPackages(), - $lockTransaction->getNewLockDevPackages(), - $platformReqs, - $platformDevReqs, - $aliases, - $this->package->getMinimumStability(), - $this->package->getStabilityFlags(), - $this->preferStable || $this->package->getPreferStable(), - $this->preferLowest, - $this->config->get('platform') ?: array(), - $this->writeLock && $this->executeOperations - ); - if ($updatedLock && $this->writeLock && $this->executeOperations) { - $this->io->writeError('Writing lock file'); - } - if ($lockTransaction->getOperations()) { $installs = $updates = $uninstalls = array(); foreach ($lockTransaction->getOperations() as $operation) { @@ -501,7 +484,22 @@ class Installer } } - $this->io->write('foo'); + $updatedLock = $this->locker->setLockData( + $lockTransaction->getNewLockNonDevPackages(), + $lockTransaction->getNewLockDevPackages(), + $platformReqs, + $platformDevReqs, + $aliases, + $this->package->getMinimumStability(), + $this->package->getStabilityFlags(), + $this->preferStable || $this->package->getPreferStable(), + $this->preferLowest, + $this->config->get('platform') ?: array(), + $this->writeLock && $this->executeOperations + ); + if ($updatedLock && $this->writeLock && $this->executeOperations) { + $this->io->writeError('Writing lock file'); + } if ($doInstall) { // TODO ensure lock is used from locker as-is, since it may not have been written to disk in case of executeOperations == false @@ -530,7 +528,7 @@ class Installer $repositorySet = $this->createRepositorySet($platformRepo, $aliases, $lockedRepository); $repositorySet->addRepository($lockedRepository); - $this->io->writeError('Installing dependencies'.($this->devMode ? ' (including require-dev)' : '').' from lock file'); + $this->io->writeError('Installing dependencies from lock file'.($this->devMode ? ' (including require-dev)' : '').''); // verify that the lock file works with the current platform repository // we can skip this part if we're doing this as the second step after an update @@ -538,7 +536,7 @@ class Installer $this->io->writeError('Verifying lock file contents can be installed on current platform.'); // creating requirements request - $request = $this->createRequest($this->fixedRootPackage, $platformRepo); + $request = $this->createRequest($this->fixedRootPackage, $platformRepo, $lockedRepository); if (!$this->locker->isFresh()) { $this->io->writeError('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.', true, IOInterface::QUIET); @@ -627,7 +625,7 @@ class Installer } // output op, but alias op only in debug verbosity - if (false === strpos($operation->getJobType(), 'Alias') || $this->io->isDebug()) { + if ((!$this->executeOperations && false === strpos($operation->getJobType(), 'Alias')) || $this->io->isDebug()) { $this->io->writeError(' - ' . $operation); } @@ -736,13 +734,14 @@ class Installer } /** - * @param RootPackageInterface $rootPackage - * @param PlatformRepository $platformRepo + * @param RootPackageInterface $rootPackage + * @param PlatformRepository $platformRepo + * @param RepositoryInterface|null $lockedRepository * @return Request */ - private function createRequest(RootPackageInterface $rootPackage, PlatformRepository $platformRepo) + private function createRequest(RootPackageInterface $rootPackage, PlatformRepository $platformRepo, $lockedRepository = null) { - $request = new Request(); + $request = new Request($lockedRepository); $request->fixPackage($rootPackage, false); 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 db4ef23c0..a4bfe6a4d 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 -Updating dependencies (including require-dev) +Updating dependencies 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 ee221dab0..b387942fb 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 -Updating dependencies (including require-dev) +Updating dependencies Your requirements could not be resolved to an installable set of packages. Problem 1 diff --git a/tests/Composer/Test/Fixtures/installer/github-issues-4795-2.test b/tests/Composer/Test/Fixtures/installer/github-issues-4795-2.test index 877ac3653..878e9429a 100644 --- a/tests/Composer/Test/Fixtures/installer/github-issues-4795-2.test +++ b/tests/Composer/Test/Fixtures/installer/github-issues-4795-2.test @@ -29,15 +29,37 @@ that are also a root package, when that root package is also explicitly whitelis { "name": "a/a", "version": "1.0.0" }, { "name": "b/b", "version": "1.0.0", "require": { "a/a": "~1.0" } } ] - +--LOCK-- +{ + "packages": [ + { + "name": "a/a", "version": "1.0.0" + }, + { + "name": "b/b", "version": "1.0.0", "require": { "a/a": "~1.0" } + } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": {}, + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} --RUN-- update a/a b/b --with-dependencies --EXPECT-OUTPUT-- Loading composer repositories with package information -Updating dependencies (including require-dev) -Package operations: 0 installs, 2 updates, 0 removals +Updating dependencies +Lock file operations: 0 installs, 2 updates, 0 removals + - Updating b/b (1.0.0) to b/b (1.1.0) + - Updating a/a (1.0.0) to a/a (1.1.0) Writing lock file +Installing dependencies from lock file (including require-dev) +Package operations: 0 installs, 2 updates, 0 removals Generating autoload files --EXPECT-- diff --git a/tests/Composer/Test/Fixtures/installer/suggest-prod.test b/tests/Composer/Test/Fixtures/installer/suggest-prod.test index 40546f8d0..e28ef03e5 100644 --- a/tests/Composer/Test/Fixtures/installer/suggest-prod.test +++ b/tests/Composer/Test/Fixtures/installer/suggest-prod.test @@ -19,8 +19,11 @@ install --no-dev --EXPECT-OUTPUT-- Loading composer repositories with package information Updating dependencies -Package operations: 1 install, 0 updates, 0 removals +Lock file operations: 1 install, 0 updates, 0 removals + - Installing a/a (1.0.0) Writing lock file +Installing dependencies from lock file +Package operations: 1 install, 0 updates, 0 removals Generating autoload files --EXPECT-- diff --git a/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test b/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test index ae5ff36e3..ab22eeb6e 100644 --- a/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test +++ b/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test @@ -18,10 +18,13 @@ Suggestions are displayed install --EXPECT-OUTPUT-- Loading composer repositories with package information -Updating dependencies (including require-dev) +Updating dependencies +Lock file operations: 1 install, 0 updates, 0 removals + - Installing a/a (1.0.0) +Writing lock file +Installing dependencies from lock file (including require-dev) Package operations: 1 install, 0 updates, 0 removals a/a suggests installing b/b (an obscure reason) -Writing lock file Generating autoload files --EXPECT-- diff --git a/tests/Composer/Test/Fixtures/installer/updating-dev-from-lock-removes-old-deps.test b/tests/Composer/Test/Fixtures/installer/updating-dev-from-lock-removes-old-deps.test index 04624561d..6575530d0 100644 --- a/tests/Composer/Test/Fixtures/installer/updating-dev-from-lock-removes-old-deps.test +++ b/tests/Composer/Test/Fixtures/installer/updating-dev-from-lock-removes-old-deps.test @@ -16,7 +16,7 @@ Installing locked dev packages should remove old dependencies "require": {} } ], - "packages-dev": null, + "packages-dev": [], "aliases": [], "minimum-stability": "dev", "stability-flags": [],