From 47a94b3a88a22c155cd8832fc681a9071dbfa0c5 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 15 Jan 2020 12:02:12 +0100 Subject: [PATCH] Ensure packages that exist in a higher prio repo never get loaded in lower prio repos, fixes #5076 --- .../DependencyResolver/PoolBuilder.php | 9 +++- src/Composer/Repository/BaseRepository.php | 22 ++++++---- .../Repository/ComposerRepository.php | 24 +++++++---- .../Repository/RepositoryInterface.php | 2 +- .../installer/repositories-priorities.test | 42 +++++++++++++++++++ .../installer/repositories-priorities2.test | 26 ++++++++++++ 6 files changed, 106 insertions(+), 19 deletions(-) create mode 100644 tests/Composer/Test/Fixtures/installer/repositories-priorities.test create mode 100644 tests/Composer/Test/Fixtures/installer/repositories-priorities2.test diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 7b80c41bd..c57e4fa43 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -97,9 +97,14 @@ class PoolBuilder } // TODO should we really pass the callable into here? - $packages = $repository->loadPackages($loadNames, $this->isPackageAcceptableCallable); + $result = $repository->loadPackages($loadNames, $this->isPackageAcceptableCallable); + + foreach ($result['namesFound'] as $name) { + // avoid loading the same package again from other repositories once it has been found + unset($loadNames[$name]); + } + foreach ($result['packages'] as $package) { - foreach ($packages as $package) { if (call_user_func($this->isPackageAcceptableCallable, $package->getNames(), $package->getStability())) { $newLoadNames += $this->loadPackage($request, $package, $key); } diff --git a/src/Composer/Repository/BaseRepository.php b/src/Composer/Repository/BaseRepository.php index ddf8b6e73..dfd99d7c7 100644 --- a/src/Composer/Repository/BaseRepository.php +++ b/src/Composer/Repository/BaseRepository.php @@ -31,16 +31,20 @@ abstract class BaseRepository implements RepositoryInterface $packages = $this->getPackages(); $result = array(); + $namesFound = array(); foreach ($packages as $package) { - if ( - array_key_exists($package->getName(), $packageMap) - && (!$packageMap[$package->getName()] || $packageMap[$package->getName()]->matches(new Constraint('==', $package->getVersion()))) - && call_user_func($isPackageAcceptableCallable, $package->getNames(), $package->getStability()) - ) { - $result[spl_object_hash($package)] = $package; - if ($package instanceof AliasPackage && !isset($result[spl_object_hash($package->getAliasOf())])) { - $result[spl_object_hash($package->getAliasOf())] = $package->getAliasOf(); + if (array_key_exists($package->getName(), $packageMap)) { + if ( + (!$packageMap[$package->getName()] || $packageMap[$package->getName()]->matches(new Constraint('==', $package->getVersion()))) + && call_user_func($isPackageAcceptableCallable, $package->getNames(), $package->getStability()) + ) { + $result[spl_object_hash($package)] = $package; + if ($package instanceof AliasPackage && !isset($result[spl_object_hash($package->getAliasOf())])) { + $result[spl_object_hash($package->getAliasOf())] = $package->getAliasOf(); + } } + + $namesFound[$package->getName()] = true; } } @@ -52,7 +56,7 @@ abstract class BaseRepository implements RepositoryInterface } } - return $result; + return array('namesFound' => array_keys($namesFound), 'packages' => $result); } /** diff --git a/src/Composer/Repository/ComposerRepository.php b/src/Composer/Repository/ComposerRepository.php index 975282702..0a9b7d5e4 100644 --- a/src/Composer/Repository/ComposerRepository.php +++ b/src/Composer/Repository/ComposerRepository.php @@ -143,7 +143,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito $packages = $this->loadAsyncPackages(array($name => $constraint)); - return reset($packages); + return reset($packages['packages']); } if ($hasProviders) { @@ -181,7 +181,9 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito return array(); } - return $this->loadAsyncPackages(array($name => $constraint)); + $result = $this->loadAsyncPackages(array($name => $constraint)); + + return $result['packages']; } if ($hasProviders) { @@ -239,7 +241,9 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito $packageMap[$name] = new EmptyConstraint(); } - return array_values($this->loadAsyncPackages($packageMap)); + $result = $this->loadAsyncPackages($packageMap); + + return array_values($result['packages']); } if ($this->hasPartialPackages()) { @@ -297,6 +301,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito } $packages = array(); + $namesFound = array(); if ($hasProviders || $this->hasPartialPackages()) { foreach ($packageNameMap as $name => $constraint) { @@ -313,6 +318,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito if ($candidate->getName() !== $name) { throw new \LogicException('whatProvides should never return a package with a different name than the requested one'); } + $namesFound[$name] = true; if (!$constraint || $constraint->matches(new Constraint('==', $candidate->getVersion()))) { $matches[spl_object_hash($candidate)] = $candidate; if ($candidate instanceof AliasPackage && !isset($matches[spl_object_hash($candidate->getAliasOf())])) { @@ -343,10 +349,12 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito }, ARRAY_FILTER_USE_KEY); } - $packages = array_merge($packages, $this->loadAsyncPackages($packageNameMap, $isPackageAcceptableCallable)); + $result = $this->loadAsyncPackages($packageNameMap, $isPackageAcceptableCallable); + $packages = array_merge($packages, $result['packages']); + $namesFound = array_merge($namesFound, $result['namesFound']); } - return $packages; + return array('namesFound' => array_keys($namesFound), 'packages' => $packages); } /** @@ -586,6 +594,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito $this->loadRootServerFile(); $packages = array(); + $namesFound = array(); $promises = array(); $repo = $this; @@ -619,7 +628,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito } $promises[] = $this->asyncFetchFile($url, $cacheKey, $lastModified) - ->then(function ($response) use (&$packages, $contents, $realName, $constraint, $repo, $isPackageAcceptableCallable) { + ->then(function ($response) use (&$packages, &$namesFound, $contents, $realName, $constraint, $repo, $isPackageAcceptableCallable) { if (true === $response) { $response = $contents; } @@ -657,6 +666,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito unset($expanded, $expandedVersion, $versionData); } + $namesFound[$realName] = true; $versionsToLoad = array(); foreach ($versions as $version) { if (!isset($version['version_normalized'])) { @@ -683,7 +693,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito $this->loop->wait($promises); - return $packages; + return array('namesFound' => $namesFound, 'packages' => $packages); // RepositorySet should call loadMetadata, getMetadata when all promises resolved, then metadataComplete when done so we can GC the loaded json and whatnot then as needed } diff --git a/src/Composer/Repository/RepositoryInterface.php b/src/Composer/Repository/RepositoryInterface.php index e5f2c5159..f5e80c24c 100644 --- a/src/Composer/Repository/RepositoryInterface.php +++ b/src/Composer/Repository/RepositoryInterface.php @@ -69,7 +69,7 @@ interface RepositoryInterface extends \Countable * * @param ConstraintInterface[] $packageNameMap package names pointing to constraints * @param $isPackageAcceptableCallable - * @return PackageInterface[] + * @return array [namesFound => string[], packages => PackageInterface[]] */ public function loadPackages(array $packageNameMap, $isPackageAcceptableCallable); diff --git a/tests/Composer/Test/Fixtures/installer/repositories-priorities.test b/tests/Composer/Test/Fixtures/installer/repositories-priorities.test new file mode 100644 index 000000000..42f7a1a6e --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/repositories-priorities.test @@ -0,0 +1,42 @@ +--TEST-- +Packages found in a higher priority repository take precedence even if they are not found in the requested version +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "foo/a", "version": "1.0.0" } + ] + }, + { + "type": "package", + "package": [ + { "name": "foo/a", "version": "2.0.0" } + ] + } + ], + "require": { + "foo/a": "2.*" + } +} +--RUN-- +update +--EXPECT-OUTPUT-- +Loading composer repositories with package information +Updating dependencies +Your requirements could not be resolved to an installable set of packages. + + Problem 1 + - The requested package foo/a could not be found in any version, there may be a typo in the package name. + +Potential causes: + - A typo in the package name + - The package is not available in a stable-enough version according to your minimum-stability setting + see for more details. + - It's a private package and you forgot to add a custom repository to find it + +Read for further common problems. +--EXPECT-- +--EXPECT-EXIT-CODE-- +2 diff --git a/tests/Composer/Test/Fixtures/installer/repositories-priorities2.test b/tests/Composer/Test/Fixtures/installer/repositories-priorities2.test new file mode 100644 index 000000000..598079d80 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/repositories-priorities2.test @@ -0,0 +1,26 @@ +--TEST-- +Packages found in a higher priority repository take precedence +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "foo/a", "version": "1.0.0" } + ] + }, + { + "type": "package", + "package": [ + { "name": "foo/a", "version": "1.1.0" } + ] + } + ], + "require": { + "foo/a": "1.*" + } +} +--RUN-- +update +--EXPECT-- +Installing foo/a (1.0.0)