From 304753ff693775f5897afaea71dd1cadf9aa2988 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 16 Jan 2020 07:58:50 +0100 Subject: [PATCH 1/8] Remove callback and pass stabilities all the way instead This allows optimizing the loading of ~dev files, and cleans up a few things --- .../DependencyResolver/PoolBuilder.php | 23 +++++----- src/Composer/Installer.php | 20 ++------- .../Package/Version/StabilityFilter.php | 43 +++++++++++++++++++ src/Composer/Repository/ArrayRepository.php | 5 ++- .../Repository/ComposerRepository.php | 31 ++++++------- .../Repository/CompositeRepository.php | 4 +- .../Repository/RepositoryInterface.php | 6 +-- src/Composer/Repository/RepositorySet.php | 35 ++++++--------- 8 files changed, 94 insertions(+), 73 deletions(-) create mode 100644 src/Composer/Package/Version/StabilityFilter.php diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 1be0522a6..0f44edf3b 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -25,10 +25,11 @@ use Composer\Semver\Constraint\MultiConstraint; */ class PoolBuilder { - private $isPackageAcceptableCallable; - private $rootRequires; + private $acceptableStabilities; + private $stabilityFlags; private $rootAliases; private $rootReferences; + private $rootRequires; private $aliasMap = array(); private $nameConstraints = array(); @@ -37,17 +38,18 @@ class PoolBuilder private $packages = array(); - public function __construct($isPackageAcceptableCallable, array $rootRequires = array()) + public function __construct(array $acceptableStabilities, array $stabilityFlags, array $rootAliases, array $rootReferences, array $rootRequires = array()) { - $this->isPackageAcceptableCallable = $isPackageAcceptableCallable; + $this->acceptableStabilities = $acceptableStabilities; + $this->stabilityFlags = $stabilityFlags; + $this->rootAliases = $rootAliases; + $this->rootReferences = $rootReferences; $this->rootRequires = $rootRequires; } - public function buildPool(array $repositories, array $rootAliases, array $rootReferences, Request $request) + public function buildPool(array $repositories, Request $request) { $pool = new Pool(); - $this->rootAliases = $rootAliases; - $this->rootReferences = $rootReferences; // TODO do we really want the request here? kind of want a root requirements thingy instead $loadNames = array(); @@ -87,17 +89,14 @@ class PoolBuilder continue; } - // TODO should we really pass the callable into here? - $result = $repository->loadPackages($loadNames, $this->isPackageAcceptableCallable); + $result = $repository->loadPackages($loadNames, $this->acceptableStabilities, $this->stabilityFlags); 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) { - if (call_user_func($this->isPackageAcceptableCallable, $package->getNames(), $package->getStability())) { - $newLoadNames += $this->loadPackage($request, $package); - } + $newLoadNames += $this->loadPackage($request, $package); } } diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index 9ed7f2d8e..e9e8f34ea 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -364,19 +364,6 @@ class Installer $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? - foreach ($lockedRepository->getPackages() as $lockedPackage) { - if (!$repositorySet->isPackageAcceptable($lockedPackage->getNames(), $lockedPackage->getStability())) { - $constraint = new Constraint('=', $lockedPackage->getVersion()); - $constraint->setPrettyString('(stability not acceptable)'); - - // if we can get rid of this remove() here, we can generally get rid of remove support in the request - $request->remove($lockedPackage->getName(), $constraint); - } - } - } - $this->io->writeError('Updating dependencies'); $links = array_merge($this->package->getRequires(), $this->package->getDevRequires()); @@ -393,10 +380,11 @@ class Installer } // if the updateWhitelist is enabled, packages not in it are also fixed - // to the version specified in the lock - if ($this->updateWhitelist) { + // to the version specified in the lock, except if their stability is not + // acceptable anymore, to make sure that they get updated/downgraded to + // a working version + if ($this->updateWhitelist && $lockedRepository) { foreach ($lockedRepository->getPackages() as $lockedPackage) { - // TODO should this really be checking acceptability here? if (!$this->isUpdateable($lockedPackage) && $repositorySet->isPackageAcceptable($lockedPackage->getNames(), $lockedPackage->getStability())) { // TODO add reason for fix? $request->fixPackage($lockedPackage); diff --git a/src/Composer/Package/Version/StabilityFilter.php b/src/Composer/Package/Version/StabilityFilter.php new file mode 100644 index 000000000..ed27af080 --- /dev/null +++ b/src/Composer/Package/Version/StabilityFilter.php @@ -0,0 +1,43 @@ + + * Jordi Boggiano + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Composer\Package\Version; + +use Composer\Package\BasePackage; + +/** + * @author Jordi Boggiano + */ +class StabilityFilter +{ + /** + * Checks if any of the provided package names in the given stability match the configured acceptable stability and flags + * + * @return bool true if any package name is acceptable + */ + public static function isPackageAcceptable(array $acceptableStabilities, array $stabilityFlags, $names, $stability) + { + foreach ($names as $name) { + // allow if package matches the package-specific stability flag + if (isset($stabilityFlags[$name])) { + if (BasePackage::$stabilities[$stability] <= $stabilityFlags[$name]) { + return true; + } + } elseif (isset($acceptableStabilities[$stability])) { + // allow if package matches the global stability requirement and has no exception + return true; + } + } + + return false; + } +} diff --git a/src/Composer/Repository/ArrayRepository.php b/src/Composer/Repository/ArrayRepository.php index 42e9d50e1..6c7d0e65e 100644 --- a/src/Composer/Repository/ArrayRepository.php +++ b/src/Composer/Repository/ArrayRepository.php @@ -16,6 +16,7 @@ use Composer\Package\AliasPackage; use Composer\Package\PackageInterface; use Composer\Package\CompletePackageInterface; use Composer\Package\Version\VersionParser; +use Composer\Package\Version\StabilityFilter; use Composer\Semver\Constraint\ConstraintInterface; use Composer\Semver\Constraint\Constraint; @@ -44,7 +45,7 @@ class ArrayRepository extends BaseRepository /** * {@inheritDoc} */ - public function loadPackages(array $packageMap, $isPackageAcceptableCallable) + public function loadPackages(array $packageMap, array $acceptableStabilities, array $stabilityFlags) { $packages = $this->getPackages(); @@ -54,7 +55,7 @@ class ArrayRepository extends BaseRepository 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()) + && StabilityFilter::isPackageAcceptable($acceptableStabilities, $stabilityFlags, $package->getNames(), $package->getStability()) ) { $result[spl_object_hash($package)] = $package; if ($package instanceof AliasPackage && !isset($result[spl_object_hash($package->getAliasOf())])) { diff --git a/src/Composer/Repository/ComposerRepository.php b/src/Composer/Repository/ComposerRepository.php index 0ee04ff90..b23d5aced 100644 --- a/src/Composer/Repository/ComposerRepository.php +++ b/src/Composer/Repository/ComposerRepository.php @@ -16,6 +16,7 @@ use Composer\Package\Loader\ArrayLoader; use Composer\Package\PackageInterface; use Composer\Package\AliasPackage; use Composer\Package\Version\VersionParser; +use Composer\Package\Version\StabilityFilter; use Composer\Json\JsonFile; use Composer\Cache; use Composer\Config; @@ -292,13 +293,13 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito return $names; } - public function loadPackages(array $packageNameMap, $isPackageAcceptableCallable) + public function loadPackages(array $packageNameMap, array $acceptableStabilities, array $stabilityFlags) { // this call initializes loadRootServerFile which is needed for the rest below to work $hasProviders = $this->hasProviders(); if (!$hasProviders && !$this->hasPartialPackages() && !$this->lazyProvidersUrl) { - return parent::loadPackages($packageNameMap, $isPackageAcceptableCallable); + return parent::loadPackages($packageNameMap, $acceptableStabilities, $stabilityFlags); } $packages = array(); @@ -314,7 +315,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito continue; } - $candidates = $this->whatProvides($name, $isPackageAcceptableCallable); + $candidates = $this->whatProvides($name, $acceptableStabilities, $stabilityFlags); foreach ($candidates as $candidate) { if ($candidate->getName() !== $name) { throw new \LogicException('whatProvides should never return a package with a different name than the requested one'); @@ -350,7 +351,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito }, ARRAY_FILTER_USE_KEY); } - $result = $this->loadAsyncPackages($packageNameMap, $isPackageAcceptableCallable); + $result = $this->loadAsyncPackages($packageNameMap, $acceptableStabilities, $stabilityFlags); $packages = array_merge($packages, $result['packages']); $namesFound = array_merge($namesFound, $result['namesFound']); } @@ -444,7 +445,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito * @param callable $isPackageAcceptableCallable * @return array|mixed */ - private function whatProvides($name, $isPackageAcceptableCallable = null) + private function whatProvides($name, array $acceptableStabilities = null, array $stabilityFlags = null) { if (!$this->hasPartialPackages() || !isset($this->partialPackagesByName[$name])) { // skip platform packages, root package and composer-plugin-api @@ -533,7 +534,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito $version['version_normalized'] = $this->versionParser->normalize($version['version']); } - if ($this->isVersionAcceptable($isPackageAcceptableCallable, null, $normalizedName, $version)) { + if ($this->isVersionAcceptable($acceptableStabilities, $stabilityFlags, null, $normalizedName, $version)) { $versionsToLoad[$version['uid']] = $version; } } @@ -590,7 +591,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito /** * @param array $packageNames array of package name => ConstraintInterface|null - if a constraint is provided, only packages matching it will be loaded */ - private function loadAsyncPackages(array $packageNames, $isPackageAcceptableCallable = null) + private function loadAsyncPackages(array $packageNames, array $acceptableStabilities = null, array $stabilityFlags = null) { $this->loadRootServerFile(); @@ -603,11 +604,11 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito throw new \LogicException('loadAsyncPackages only supports v2 protocol composer repos with a metadata-url'); } - // load ~dev variants as well if present - // TODO ideally there should be a flag set from the repositoryset/poolbuilder to know which packages should have the dev packages loaded - // so we can optimize away some requests entirely + // load ~dev versions of the packages as well if needed foreach ($packageNames as $name => $constraint) { - $packageNames[$name.'~dev'] = $constraint; + if ($acceptableStabilities && $stabilityFlags && StabilityFilter::isPackageAcceptable($acceptableStabilities, $stabilityFlags, array($name), 'dev')) { + $packageNames[$name.'~dev'] = $constraint; + } } foreach ($packageNames as $name => $constraint) { @@ -629,7 +630,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito } $promises[] = $this->asyncFetchFile($url, $cacheKey, $lastModified) - ->then(function ($response) use (&$packages, &$namesFound, $contents, $realName, $constraint, $repo, $isPackageAcceptableCallable) { + ->then(function ($response) use (&$packages, &$namesFound, $contents, $realName, $constraint, $repo, $acceptableStabilities, $stabilityFlags) { if (true === $response) { $response = $contents; } @@ -651,7 +652,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito $version['version_normalized'] = $repo->versionParser->normalize($version['version']); } - if ($repo->isVersionAcceptable($isPackageAcceptableCallable, $constraint, $realName, $version)) { + if ($repo->isVersionAcceptable($acceptableStabilities, $stabilityFlags, $constraint, $realName, $version)) { $versionsToLoad[] = $version; } } @@ -681,7 +682,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito * @param string $name package name (must be lowercased already) * @private */ - public function isVersionAcceptable($isPackageAcceptableCallable, $constraint, $name, $versionData) + public function isVersionAcceptable(array $acceptableStabilities = null, array $stabilityFlags = null, $constraint = null, $name, $versionData) { $versions = array($versionData['version_normalized']); @@ -690,7 +691,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito } foreach ($versions as $version) { - if ($isPackageAcceptableCallable && !call_user_func($isPackageAcceptableCallable, $name, VersionParser::parseStability($version))) { + if ($acceptableStabilities && $stabilityFlags && !StabilityFilter::isPackageAcceptable($acceptableStabilities, $stabilityFlags, array($name), VersionParser::parseStability($version))) { continue; } diff --git a/src/Composer/Repository/CompositeRepository.php b/src/Composer/Repository/CompositeRepository.php index ddaa94694..8ead6693a 100644 --- a/src/Composer/Repository/CompositeRepository.php +++ b/src/Composer/Repository/CompositeRepository.php @@ -97,13 +97,13 @@ class CompositeRepository extends BaseRepository /** * {@inheritDoc} */ - public function loadPackages(array $packageMap, $isPackageAcceptableCallable) + public function loadPackages(array $packageMap, array $acceptableStabilities, array $stabilityFlags) { $packages = array(); $namesFound = array(); foreach ($this->repositories as $repository) { /* @var $repository RepositoryInterface */ - $result = $repository->findPackages($name, $constraint); + $result = $repository->loadPackages($packageMap, $acceptableStabilities, $stabilityFlags); $packages[] = $result['packages']; $namesFound[] = $result['namesFound']; } diff --git a/src/Composer/Repository/RepositoryInterface.php b/src/Composer/Repository/RepositoryInterface.php index f5e80c24c..8b1bb3cff 100644 --- a/src/Composer/Repository/RepositoryInterface.php +++ b/src/Composer/Repository/RepositoryInterface.php @@ -56,7 +56,6 @@ interface RepositoryInterface extends \Countable */ public function findPackages($name, $constraint = null); - // TODO this should really not be in this generic interface anymore /** * Returns list of registered packages. * @@ -68,10 +67,11 @@ interface RepositoryInterface extends \Countable * Returns list of registered packages with the supplied name * * @param ConstraintInterface[] $packageNameMap package names pointing to constraints - * @param $isPackageAcceptableCallable + * @param array $acceptableStabilities + * @param array $stabilityFlags * @return array [namesFound => string[], packages => PackageInterface[]] */ - public function loadPackages(array $packageNameMap, $isPackageAcceptableCallable); + public function loadPackages(array $packageNameMap, array $acceptableStabilities, array $stabilityFlags); /** * Searches the repository for packages containing the query diff --git a/src/Composer/Repository/RepositorySet.php b/src/Composer/Repository/RepositorySet.php index 5ffffb73d..9ca6e98e3 100644 --- a/src/Composer/Repository/RepositorySet.php +++ b/src/Composer/Repository/RepositorySet.php @@ -22,7 +22,7 @@ use Composer\Repository\PlatformRepository; use Composer\Repository\LockArrayRepository; use Composer\Repository\InstalledRepositoryInterface; use Composer\Semver\Constraint\ConstraintInterface; -use Composer\Test\DependencyResolver\PoolTest; +use Composer\Package\Version\StabilityFilter; /** * @author Nils Adermann @@ -89,23 +89,6 @@ class RepositorySet } } - public function isPackageAcceptable($name, $stability) - { - foreach ((array) $name as $n) { - // allow if package matches the global stability requirement and has no exception - if (!isset($this->stabilityFlags[$n]) && isset($this->acceptableStabilities[$stability])) { - return true; - } - - // allow if package matches the package-specific stability flag - if (isset($this->stabilityFlags[$n]) && BasePackage::$stabilities[$stability] <= $this->stabilityFlags[$n]) { - return true; - } - } - - return false; - } - /** * Find packages providing or matching a name and optionally meeting a constraint in all repositories * @@ -113,10 +96,11 @@ class RepositorySet * * @param string $name * @param ConstraintInterface|null $constraint - * @param bool $exactMatch + * @param bool $exactMatch if set to false, packages which replace/provide the given name might be returned as well even if they do not match the name exactly + * @param bool $ignoreStability if set to true, packages are returned even though their stability does not match the required stability * @return array */ - public function findPackages($name, ConstraintInterface $constraint = null, $exactMatch = true) + public function findPackages($name, ConstraintInterface $constraint = null, $exactMatch = true, $ignoreStability = false) { $packages = array(); foreach ($this->repositories as $repository) { @@ -131,7 +115,7 @@ class RepositorySet continue; } - if ($this->isPackageAcceptable($candidate->getNames(), $candidate->getStability())) { + if (!$ignoreStability && $this->isPackageAcceptable($candidate->getNames(), $candidate->getStability())) { $result[] = $candidate; } } @@ -139,6 +123,11 @@ class RepositorySet return $candidates; } + public function isPackageAcceptable($names, $stability) + { + return StabilityFilter::isPackageAcceptable($this->acceptableStabilities, $this->stabilityFlags, $names, $stability); + } + /** * Create a pool for dependency resolution from the packages in this repository set. * @@ -146,7 +135,7 @@ class RepositorySet */ public function createPool(Request $request) { - $poolBuilder = new PoolBuilder(array($this, 'isPackageAcceptable'), $this->rootRequires); + $poolBuilder = new PoolBuilder($this->acceptableStabilities, $this->stabilityFlags, $this->rootAliases, $this->rootReferences, $this->rootRequires); foreach ($this->repositories as $repo) { if ($repo instanceof InstalledRepositoryInterface) { @@ -154,7 +143,7 @@ class RepositorySet } } - return $this->pool = $poolBuilder->buildPool($this->repositories, $this->rootAliases, $this->rootReferences, $request); + return $this->pool = $poolBuilder->buildPool($this->repositories, $request); } // TODO unify this with above in some simpler version without "request"? From e162cc6f0a031c7405bced971f6978801ecc4c23 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 17 Jan 2020 10:08:21 +0100 Subject: [PATCH 2/8] Remove "remove" from request --- src/Composer/DependencyResolver/Problem.php | 4 ---- src/Composer/DependencyResolver/Request.php | 5 ----- 2 files changed, 9 deletions(-) diff --git a/src/Composer/DependencyResolver/Problem.php b/src/Composer/DependencyResolver/Problem.php index 8c73bd91d..54815995f 100644 --- a/src/Composer/DependencyResolver/Problem.php +++ b/src/Composer/DependencyResolver/Problem.php @@ -224,10 +224,6 @@ class Problem } return 'Installation request for '.$packageName.$this->constraintToText($constraint).' -> satisfiable by '.$this->getPackageList($packages).'.'; - case 'update': - return 'Update request for '.$packageName.$this->constraintToText($constraint).'.'; - case 'remove': - return 'Removal request for '.$packageName.$this->constraintToText($constraint).''; } if (isset($constraint)) { diff --git a/src/Composer/DependencyResolver/Request.php b/src/Composer/DependencyResolver/Request.php index fc4a21070..851407591 100644 --- a/src/Composer/DependencyResolver/Request.php +++ b/src/Composer/DependencyResolver/Request.php @@ -38,11 +38,6 @@ class Request $this->addJob($packageName, 'install', $constraint); } - public function remove($packageName, ConstraintInterface $constraint = null) - { - $this->addJob($packageName, 'remove', $constraint); - } - /** * Mark an existing package as being installed and having to remain installed * From b5e34ca7674a7edb3c88e0fa1c2e1d71bf0218fc Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 17 Jan 2020 10:27:12 +0100 Subject: [PATCH 3/8] Fix remove tests --- .../Test/DependencyResolver/RequestTest.php | 4 +--- .../Test/DependencyResolver/SolverTest.php | 23 ------------------- 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/tests/Composer/Test/DependencyResolver/RequestTest.php b/tests/Composer/Test/DependencyResolver/RequestTest.php index 405f3b0c8..7534f9a5c 100644 --- a/tests/Composer/Test/DependencyResolver/RequestTest.php +++ b/tests/Composer/Test/DependencyResolver/RequestTest.php @@ -18,7 +18,7 @@ use Composer\Test\TestCase; class RequestTest extends TestCase { - public function testRequestInstallAndRemove() + public function testRequestInstall() { $repo = new ArrayRepository; $foo = $this->getPackage('foo', '1'); @@ -31,12 +31,10 @@ class RequestTest extends TestCase $request = new Request(); $request->install('foo'); - $request->remove('foobar'); $this->assertEquals( array( array('cmd' => 'install', 'packageName' => 'foo', 'constraint' => null), - array('cmd' => 'remove', 'packageName' => 'foobar', 'constraint' => null), ), $request->getJobs() ); diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index 4801188e4..01a518f27 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/tests/Composer/Test/DependencyResolver/SolverTest.php @@ -196,28 +196,6 @@ class SolverTest extends TestCase $this->checkSolverResult(array()); } - public function testSolverRemoveSingle() - { - $this->repoLocked->addPackage($packageA = $this->getPackage('A', '1.0')); - $this->reposComplete(); - - $this->request->remove('A'); - - $this->checkSolverResult(array( - array('job' => 'remove', 'package' => $packageA), - )); - } - - public function testSolverRemoveUninstalled() - { - $this->repo->addPackage($this->getPackage('A', '1.0')); - $this->reposComplete(); - - $this->request->remove('A'); - - $this->checkSolverResult(array()); - } - public function testSolverUpdateDoesOnlyUpdate() { $this->repoLocked->addPackage($packageA = $this->getPackage('A', '1.0')); @@ -367,7 +345,6 @@ class SolverTest extends TestCase $this->request->install('A'); $this->request->install('C'); - $this->request->remove('D'); $this->checkSolverResult(array( array('job' => 'remove', 'package' => $packageD), From 6dc576738a3a0eb68281a3bb4117f4d25d3b5303 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 17 Jan 2020 15:15:46 +0100 Subject: [PATCH 4/8] Avoid partial updates from applying changes to packages which are not locked with an acceptable stability --- src/Composer/DependencyResolver/PoolBuilder.php | 9 ++++++++- src/Composer/DependencyResolver/RuleSetGenerator.php | 3 ++- src/Composer/Installer.php | 6 ++---- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 0f44edf3b..7eaae9423 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -16,7 +16,9 @@ use Composer\Package\AliasPackage; use Composer\Package\BasePackage; use Composer\Package\Package; use Composer\Package\PackageInterface; +use Composer\Package\Version\StabilityFilter; use Composer\Repository\PlatformRepository; +use Composer\Repository\RootPackageRepository; use Composer\Semver\Constraint\Constraint; use Composer\Semver\Constraint\MultiConstraint; @@ -57,7 +59,12 @@ class PoolBuilder $this->nameConstraints[$package->getName()] = null; $this->loadedNames[$package->getName()] = true; unset($loadNames[$package->getName()]); - $loadNames += $this->loadPackage($request, $package); + if ( + $package->getRepository() instanceof RootPackageRepository + || StabilityFilter::isPackageAcceptable($this->acceptableStabilities, $this->stabilityFlags, $package->getNames(), $package->getStability()) + ) { + $loadNames += $this->loadPackage($request, $package); + } } foreach ($request->getJobs() as $job) { diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index 442be135c..bdf187470 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -290,8 +290,9 @@ class RuleSetGenerator $unlockableMap = $request->getUnlockableMap(); foreach ($request->getFixedPackages() as $package) { + // fixed package was not added to the pool which must mean it did not pass the stability requirements if ($package->id == -1) { - throw new \RuntimeException("Fixed package ".$package->getName()." ".$package->getVersion().($package instanceof AliasPackage ? " (alias)" : "")." was not added to solver pool."); + continue; } $this->addRulesForPackage($package, $ignorePlatformReqs); diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index e9e8f34ea..9c6c5a719 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -380,12 +380,10 @@ class Installer } // if the updateWhitelist is enabled, packages not in it are also fixed - // to the version specified in the lock, except if their stability is not - // acceptable anymore, to make sure that they get updated/downgraded to - // a working version + // to the version specified in the lock if ($this->updateWhitelist && $lockedRepository) { foreach ($lockedRepository->getPackages() as $lockedPackage) { - if (!$this->isUpdateable($lockedPackage) && $repositorySet->isPackageAcceptable($lockedPackage->getNames(), $lockedPackage->getStability())) { + if (!$this->isUpdateable($lockedPackage)) { // TODO add reason for fix? $request->fixPackage($lockedPackage); } From ebe910c3a55b2767549350de114f3c63f243d5c3 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 17 Jan 2020 15:16:01 +0100 Subject: [PATCH 5/8] Tweak test to follow changes --- ...e-downgrades-non-whitelisted-unstable.test | 40 +++++++++---------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/tests/Composer/Test/Fixtures/installer/partial-update-downgrades-non-whitelisted-unstable.test b/tests/Composer/Test/Fixtures/installer/partial-update-downgrades-non-whitelisted-unstable.test index d87df634c..9735d0a2c 100644 --- a/tests/Composer/Test/Fixtures/installer/partial-update-downgrades-non-whitelisted-unstable.test +++ b/tests/Composer/Test/Fixtures/installer/partial-update-downgrades-non-whitelisted-unstable.test @@ -48,25 +48,23 @@ Partial update from lock file should apply lock file and downgrade unstable pack ] --RUN-- update c/uptodate ---EXPECT-LOCK-- -{ - "packages": [ - { "name": "a/old", "version": "1.0.0", "type": "library" }, - { "name": "b/unstable", "version": "1.0.0", "type": "library" }, - { "name": "c/uptodate", "version": "1.0.0", "type": "library" }, - { "name": "d/removed", "version": "1.0.0", "type": "library" } - ], - "packages-dev": [], - "aliases": [], - "minimum-stability": "stable", - "stability-flags": [], - "prefer-stable": false, - "prefer-lowest": false, - "platform": [], - "platform-dev": [] -} --EXPECT-- -Updating a/old (0.9.0 => 1.0.0) -Updating b/unstable (1.1.0-alpha => 1.0.0) -Updating c/uptodate (2.0.0 => 1.0.0) -Installing d/removed (1.0.0) + +--EXPECT-EXIT-CODE-- +2 + +--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 b/unstable 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. From 7cc8a4aed8f52f5971f16782ee21c20d783c6ede Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 17 Jan 2020 15:29:30 +0100 Subject: [PATCH 6/8] Avoid checking stability on platform packages too --- src/Composer/DependencyResolver/PoolBuilder.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 7eaae9423..9d004758c 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -61,6 +61,7 @@ class PoolBuilder unset($loadNames[$package->getName()]); if ( $package->getRepository() instanceof RootPackageRepository + || $package->getRepository() instanceof PlatformRepository || StabilityFilter::isPackageAcceptable($this->acceptableStabilities, $this->stabilityFlags, $package->getNames(), $package->getStability()) ) { $loadNames += $this->loadPackage($request, $package); From c6a3f48eaf0dd5824b2e22e7267bfc0e730527a2 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 17 Jan 2020 15:35:37 +0100 Subject: [PATCH 7/8] Remove some more remove request handling --- src/Composer/DependencyResolver/RuleSetGenerator.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index bdf187470..7bed3bb0b 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -325,15 +325,6 @@ class RuleSetGenerator $this->addRule(RuleSet::TYPE_JOB, $rule); } break; - case 'remove': - // remove all packages with this name including uninstalled - // ones to make sure none of them are picked as replacements - $packages = $this->pool->whatProvides($job['packageName'], $job['constraint']); - foreach ($packages as $package) { - $rule = $this->createRemoveRule($package, Rule::RULE_JOB_REMOVE, $job); - $this->addRule(RuleSet::TYPE_JOB, $rule); - } - break; } } } From 1d3119047294476244e872552671a12a41994aed Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 17 Jan 2020 15:48:31 +0100 Subject: [PATCH 8/8] Keep track of unacceptable fixed packages for later to use in error reporting and make sure the pool state is consistent --- src/Composer/DependencyResolver/Pool.php | 17 ++++++++++++----- src/Composer/DependencyResolver/PoolBuilder.php | 15 +++++++++------ .../DependencyResolver/RuleSetGenerator.php | 9 +++++++-- .../Test/DependencyResolver/PoolTest.php | 13 +++++-------- .../Test/DependencyResolver/RuleSetTest.php | 3 +-- .../Test/DependencyResolver/RuleTest.php | 3 +-- 6 files changed, 35 insertions(+), 25 deletions(-) diff --git a/src/Composer/DependencyResolver/Pool.php b/src/Composer/DependencyResolver/Pool.php index ffb70c300..69a5b2cc2 100644 --- a/src/Composer/DependencyResolver/Pool.php +++ b/src/Composer/DependencyResolver/Pool.php @@ -38,24 +38,26 @@ class Pool implements \Countable protected $packageByExactName = array(); protected $versionParser; protected $providerCache = array(); + protected $unacceptableFixedPackages; - public function __construct() + public function __construct(array $packages = array(), array $unacceptableFixedPackages = array()) { $this->versionParser = new VersionParser; + $this->setPackages($packages); + $this->unacceptableFixedPackages = $unacceptableFixedPackages; } - public function setPackages(array $packages) + private function setPackages(array $packages) { $id = 1; - foreach ($packages as $i => $package) { + foreach ($packages as $package) { $this->packages[] = $package; $package->id = $id++; - $names = $package->getNames(); $this->packageByExactName[$package->getName()][$package->id] = $package; - foreach ($names as $provided) { + foreach ($package->getNames() as $provided) { $this->packageByName[$provided][] = $package; } } @@ -227,4 +229,9 @@ class Pool implements \Countable return self::MATCH_NONE; } + + public function isUnacceptableFixedPackage(PackageInterface $package) + { + return in_array($package, $this->unacceptableFixedPackages, true); + } } diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 9d004758c..9cc6e18c1 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -35,10 +35,9 @@ class PoolBuilder private $aliasMap = array(); private $nameConstraints = array(); - private $loadedNames = array(); - private $packages = array(); + private $unacceptableFixedPackages = array(); public function __construct(array $acceptableStabilities, array $stabilityFlags, array $rootAliases, array $rootReferences, array $rootRequires = array()) { @@ -65,6 +64,8 @@ class PoolBuilder || StabilityFilter::isPackageAcceptable($this->acceptableStabilities, $this->stabilityFlags, $package->getNames(), $package->getStability()) ) { $loadNames += $this->loadPackage($request, $package); + } else { + $this->unacceptableFixedPackages[] = $package; } } @@ -137,11 +138,13 @@ class PoolBuilder } } - $pool->setPackages($this->packages); + $pool = new Pool($this->packages, $this->unacceptableFixedPackages); - unset($this->aliasMap); - unset($this->loadedNames); - unset($this->nameConstraints); + $this->aliasMap = array(); + $this->nameConstraints = array(); + $this->loadedNames = array(); + $this->packages = array(); + $this->unacceptableFixedPackages = array(); return $pool; } diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index 7bed3bb0b..01b75e712 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -290,9 +290,14 @@ class RuleSetGenerator $unlockableMap = $request->getUnlockableMap(); foreach ($request->getFixedPackages() as $package) { - // fixed package was not added to the pool which must mean it did not pass the stability requirements if ($package->id == -1) { - continue; + // fixed package was not added to the pool as it did not pass the stability requirements, this is fine + if ($this->pool->isUnacceptableFixedPackage($package)) { + continue; + } + + // otherwise, looks like a bug + throw new \LogicException("Fixed package ".$package->getName()." ".$package->getVersion().($package instanceof AliasPackage ? " (alias)" : "")." was not added to solver pool."); } $this->addRulesForPackage($package, $ignorePlatformReqs); diff --git a/tests/Composer/Test/DependencyResolver/PoolTest.php b/tests/Composer/Test/DependencyResolver/PoolTest.php index dba110691..ceeb928ba 100644 --- a/tests/Composer/Test/DependencyResolver/PoolTest.php +++ b/tests/Composer/Test/DependencyResolver/PoolTest.php @@ -21,10 +21,9 @@ class PoolTest extends TestCase { public function testPool() { - $pool = $this->createPool(); $package = $this->getPackage('foo', '1'); - $pool->setPackages(array($package)); + $pool = $this->createPool(array($package)); $this->assertEquals(array($package), $pool->whatProvides('foo')); $this->assertEquals(array($package), $pool->whatProvides('foo')); @@ -32,12 +31,11 @@ class PoolTest extends TestCase public function testWhatProvidesPackageWithConstraint() { - $pool = $this->createPool(); $firstPackage = $this->getPackage('foo', '1'); $secondPackage = $this->getPackage('foo', '2'); - $pool->setPackages(array( + $pool = $this->createPool(array( $firstPackage, $secondPackage, )); @@ -48,10 +46,9 @@ class PoolTest extends TestCase public function testPackageById() { - $pool = $this->createPool(); $package = $this->getPackage('foo', '1'); - $pool->setPackages(array($package)); + $pool = $this->createPool(array($package)); $this->assertSame($package, $pool->packageById(1)); } @@ -63,8 +60,8 @@ class PoolTest extends TestCase $this->assertEquals(array(), $pool->whatProvides('foo')); } - protected function createPool() + protected function createPool(array $packages = array()) { - return new Pool(); + return new Pool($packages); } } diff --git a/tests/Composer/Test/DependencyResolver/RuleSetTest.php b/tests/Composer/Test/DependencyResolver/RuleSetTest.php index 05780e526..290b3af2f 100644 --- a/tests/Composer/Test/DependencyResolver/RuleSetTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleSetTest.php @@ -139,8 +139,7 @@ class RuleSetTest extends TestCase public function testPrettyString() { - $pool = new Pool(); - $pool->setPackages(array( + $pool = new Pool(array( $p = $this->getPackage('foo', '2.1'), )); diff --git a/tests/Composer/Test/DependencyResolver/RuleTest.php b/tests/Composer/Test/DependencyResolver/RuleTest.php index 3a0e2e5ca..8941f430e 100644 --- a/tests/Composer/Test/DependencyResolver/RuleTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleTest.php @@ -93,8 +93,7 @@ class RuleTest extends TestCase public function testPrettyString() { - $pool = new Pool(); - $pool->setPackages(array( + $pool = new Pool(array( $p1 = $this->getPackage('foo', '2.1'), $p2 = $this->getPackage('baz', '1.1'), ));