From 2d025dce0541dac35a2241b0a000aa67844ccf91 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 26 Nov 2020 11:27:44 +0100 Subject: [PATCH 1/3] Make sure mirror updates do not fail if there are dev requirements and new requires are present, fixes #9514 --- src/Composer/Installer.php | 60 ++++++++++------- .../update-mirrors-fails-with-new-req.test | 65 +++++++++++++++++++ 2 files changed, 100 insertions(+), 25 deletions(-) create mode 100644 tests/Composer/Test/Fixtures/installer/update-mirrors-fails-with-new-req.test diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index 2f52de0f3..4a2289bd1 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -24,6 +24,7 @@ use Composer\DependencyResolver\Pool; use Composer\DependencyResolver\Request; use Composer\DependencyResolver\Solver; use Composer\DependencyResolver\SolverProblemsException; +use Composer\DependencyResolver\PolicyInterface; use Composer\Downloader\DownloadManager; use Composer\EventDispatcher\EventDispatcher; use Composer\Installer\InstallationManager; @@ -52,6 +53,7 @@ use Composer\Repository\RootPackageRepository; use Composer\Repository\PlatformRepository; use Composer\Repository\RepositoryInterface; use Composer\Repository\RepositoryManager; +use Composer\Repository\LockArrayRepository; use Composer\Script\ScriptEvents; /** @@ -387,22 +389,7 @@ class Installer } $request = $this->createRequest($this->fixedRootPackage, $platformRepo, $lockedRepository); - - // if we're updating mirrors we want to keep exactly the same versions installed which are in the lock file, but we want current remote metadata - if ($this->updateMirrors && $lockedRepository) { - foreach ($lockedRepository->getPackages() as $lockedPackage) { - // exclude alias packages here as for root aliases, both alias and aliased are - // present in the lock repo and we only want to require the aliased version - if (!$lockedPackage instanceof AliasPackage) { - $request->requireName($lockedPackage->getName(), new Constraint('==', $lockedPackage->getVersion())); - } - } - } else { - $links = array_merge($this->package->getRequires(), $this->package->getDevRequires()); - foreach ($links as $link) { - $request->requireName($link->getTarget(), $link->getConstraint()); - } - } + $this->requirePackagesForUpdate($request, $lockedRepository, true); // pass the allow list into the request, so the pool builder can apply it if ($this->updateAllowList) { @@ -442,7 +429,7 @@ class Installer $this->io->writeError('Nothing to modify in lock file'); } - $exitCode = $this->extractDevPackages($lockTransaction, $platformRepo, $aliases, $policy); + $exitCode = $this->extractDevPackages($lockTransaction, $platformRepo, $aliases, $policy, $lockedRepository); if ($exitCode !== 0) { return $exitCode; } @@ -555,7 +542,7 @@ class Installer * 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) + protected function extractDevPackages(LockTransaction $lockTransaction, PlatformRepository $platformRepo, array $aliases, PolicyInterface $policy, LockArrayRepository $lockedRepository = null) { if (!$this->package->getDevRequires()) { return 0; @@ -572,11 +559,7 @@ class Installer $repositorySet->addRepository($resultRepo); $request = $this->createRequest($this->fixedRootPackage, $platformRepo); - - $links = $this->package->getRequires(); - foreach ($links as $link) { - $request->requireName($link->getTarget(), $link->getConstraint()); - } + $this->requirePackagesForUpdate($request, $lockedRepository, false); $pool = $repositorySet->createPoolWithAllPackages(); @@ -820,7 +803,7 @@ class Installer * @param RepositoryInterface|null $lockedRepository * @return Request */ - private function createRequest(RootPackageInterface $rootPackage, PlatformRepository $platformRepo, $lockedRepository = null) + private function createRequest(RootPackageInterface $rootPackage, PlatformRepository $platformRepo, LockArrayRepository $lockedRepository = null) { $request = new Request($lockedRepository); @@ -851,6 +834,33 @@ class Installer return $request; } + private function requirePackagesForUpdate(Request $request, LockArrayRepository $lockedRepository = null, $includeDevRequires = true) + { + // if we're updating mirrors we want to keep exactly the same versions installed which are in the lock file, but we want current remote metadata + if ($this->updateMirrors) { + $excludedPackages = array(); + if (!$includeDevRequires) { + $excludedPackages = $this->locker->getDevPackageNames(); + } + + foreach ($lockedRepository->getPackages() as $lockedPackage) { + // exclude alias packages here as for root aliases, both alias and aliased are + // present in the lock repo and we only want to require the aliased version + if (!$lockedPackage instanceof AliasPackage && !in_array($lockedPackage->getName(), $excludedPackages, true)) { + $request->requireName($lockedPackage->getName(), new Constraint('==', $lockedPackage->getVersion())); + } + } + } else { + $links = $this->package->getRequires(); + if ($includeDevRequires) { + $links = array_merge($links, $this->package->getDevRequires()); + } + foreach ($links as $link) { + $request->requireName($link->getTarget(), $link->getConstraint()); + } + } + } + /** * @param bool $forUpdate * @return array @@ -870,7 +880,7 @@ class Installer * @param array $links * @return array */ - private function extractPlatformRequirements($links) + private function extractPlatformRequirements(array $links) { $platformReqs = array(); foreach ($links as $link) { diff --git a/tests/Composer/Test/Fixtures/installer/update-mirrors-fails-with-new-req.test b/tests/Composer/Test/Fixtures/installer/update-mirrors-fails-with-new-req.test new file mode 100644 index 000000000..0b47f4b5a --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/update-mirrors-fails-with-new-req.test @@ -0,0 +1,65 @@ +--TEST-- +Update mirrors with a new root require which is not yet in lock should simply ignore it +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + {"name": "a/a", "version": "1.0.0"}, + {"name": "a/a", "version": "1.1.0"}, + {"name": "b/b", "version": "1.0.0"}, + {"name": "b/b", "version": "1.1.0"}, + {"name": "new/req", "version": "1.0.0"}, + {"name": "new/req", "version": "1.1.0"} + ] + } + ], + "require": { + "a/a": "1.*", + "new/req": "1.*" + }, + "require-dev": { + "b/b": "1.*" + } +} +--INSTALLED-- +[ + {"name": "a/a", "version": "1.0.0"}, + {"name": "b/b", "version": "1.0.0"} +] +--LOCK-- +{ + "packages": [ + {"name": "a/a", "version": "1.0.0", "type": "library"} + ], + "packages-dev": [ + {"name": "b/b", "version": "1.0.0", "type": "library"} + ], + "aliases": [], + "minimum-stability": "dev", + "stability-flags": [], + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} +--RUN-- +update mirrors +--EXPECT-LOCK-- +{ + "packages": [ + {"name": "a/a", "version": "1.0.0", "type": "library"} + ], + "packages-dev": [ + {"name": "b/b", "version": "1.0.0", "type": "library"} + ], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": [], + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} +--EXPECT-- From e857a8216cc72600fe4ae4d067467272d149f428 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 26 Nov 2020 11:28:17 +0100 Subject: [PATCH 2/3] Make sure mirror update fails if no lock file is present --- src/Composer/Installer.php | 8 +++----- .../Fixtures/installer/partial-update-without-lock.test | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index 4a2289bd1..8058cb0b3 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -367,12 +367,10 @@ class Installer // doing a full update } - if ($this->updateAllowList) { - if (!$lockedRepository) { - $this->io->writeError('Cannot update only a partial set of packages without a lock file present.', true, IOInterface::QUIET); + if (($this->updateAllowList || $this->updateMirrors) && !$lockedRepository) { + $this->io->writeError('Cannot update ' . ($this->updateMirrors ? 'lock file information' : 'only a partial set of packages') . ' without a lock file present. Run `composer update` to generate a lock file.', true, IOInterface::QUIET); - return 1; - } + return 1; } $this->io->writeError('Loading composer repositories with package information'); diff --git a/tests/Composer/Test/Fixtures/installer/partial-update-without-lock.test b/tests/Composer/Test/Fixtures/installer/partial-update-without-lock.test index 74007af7b..1738b2a84 100644 --- a/tests/Composer/Test/Fixtures/installer/partial-update-without-lock.test +++ b/tests/Composer/Test/Fixtures/installer/partial-update-without-lock.test @@ -31,7 +31,7 @@ Partial update without lock file should error --RUN-- update b/unstable --EXPECT-OUTPUT-- -Cannot update only a partial set of packages without a lock file present. +Cannot update only a partial set of packages without a lock file present. Run `composer update` to generate a lock file. --EXPECT-EXIT-CODE-- 1 --EXPECT-- From be3a520331337d07e9b1eee44d58d728a25a3a48 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 26 Nov 2020 13:07:33 +0100 Subject: [PATCH 3/3] Fix feedback --- src/Composer/Installer.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index 8058cb0b3..51b7c40b6 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -796,9 +796,6 @@ class Installer } /** - * @param RootPackageInterface $rootPackage - * @param PlatformRepository $platformRepo - * @param RepositoryInterface|null $lockedRepository * @return Request */ private function createRequest(RootPackageInterface $rootPackage, PlatformRepository $platformRepo, LockArrayRepository $lockedRepository = null) @@ -838,13 +835,13 @@ class Installer if ($this->updateMirrors) { $excludedPackages = array(); if (!$includeDevRequires) { - $excludedPackages = $this->locker->getDevPackageNames(); + $excludedPackages = array_flip($this->locker->getDevPackageNames()); } foreach ($lockedRepository->getPackages() as $lockedPackage) { // exclude alias packages here as for root aliases, both alias and aliased are // present in the lock repo and we only want to require the aliased version - if (!$lockedPackage instanceof AliasPackage && !in_array($lockedPackage->getName(), $excludedPackages, true)) { + if (!$lockedPackage instanceof AliasPackage && !isset($excludedPackages[$lockedPackage->getName()])) { $request->requireName($lockedPackage->getName(), new Constraint('==', $lockedPackage->getVersion())); } }