From 9fbc386a7b88b37c63f93f16d3086cc5baf49683 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 21 Jul 2022 11:05:14 +0200 Subject: [PATCH] Fix package selector warnings to only show for versions that were actually skipped, #10942 --- src/Composer/Command/RequireCommand.php | 2 +- .../Package/Version/VersionSelector.php | 136 +++++++++--------- .../Package/Version/VersionSelectorTest.php | 107 +++++++------- 3 files changed, 124 insertions(+), 121 deletions(-) diff --git a/src/Composer/Command/RequireCommand.php b/src/Composer/Command/RequireCommand.php index b648166db..76f220d3b 100644 --- a/src/Composer/Command/RequireCommand.php +++ b/src/Composer/Command/RequireCommand.php @@ -240,7 +240,7 @@ EOT if (count($inconsistentRequireKeys) > 0) { foreach ($inconsistentRequireKeys as $package) { $io->warning(sprintf( - '%s is currently present in the %s key and you ran the command %s the --dev flag, which would move it to the %s key.', + '%s is currently present in the %s key and you ran the command %s the --dev flag, which will move it to the %s key.', $package, $removeKey, $input->getOption('dev') ? 'with' : 'without', diff --git a/src/Composer/Package/Version/VersionSelector.php b/src/Composer/Package/Version/VersionSelector.php index 241251779..6832695c7 100644 --- a/src/Composer/Package/Version/VersionSelector.php +++ b/src/Composer/Package/Version/VersionSelector.php @@ -86,86 +86,84 @@ class VersionSelector $constraint = $targetPackageVersion ? $this->getParser()->parseConstraints($targetPackageVersion) : null; $candidates = $this->repositorySet->findPackages(strtolower($packageName), $constraint, $repoSetFlags); - $skippedWarnings = []; - if ($this->platformConstraints && !($platformRequirementFilter instanceof IgnoreAllPlatformRequirementFilter)) { - $platformConstraints = $this->platformConstraints; - $candidates = array_filter($candidates, static function ($pkg) use ($platformConstraints, $platformRequirementFilter, &$skippedWarnings): bool { - $reqs = $pkg->getRequires(); - - foreach ($reqs as $name => $link) { - if (!$platformRequirementFilter->isIgnored($name)) { - if (isset($platformConstraints[$name])) { - foreach ($platformConstraints[$name] as $constraint) { - if ($link->getConstraint()->matches($constraint)) { - continue 2; - } - } - - $skippedWarnings[$pkg->getName()][] = ['version' => $pkg->getPrettyVersion(), 'link' => $link, 'reason' => 'is not satisfied by your platform']; - return false; - } elseif (PlatformRepository::isPlatformPackage($name)) { - // Package requires a platform package that is unknown on current platform. - // It means that current platform cannot validate this constraint and so package is not installable. - $skippedWarnings[$pkg->getName()][] = ['version' => $pkg->getPrettyVersion(), 'link' => $link, 'reason' => 'is missing from your platform']; - return false; - } - } - } - - return true; - }); - } - - if (!$candidates) { - return false; - } - - if (count($skippedWarnings) > 0 && $io !== null) { - foreach ($skippedWarnings as $name => $warnings) { - foreach ($warnings as $index => $warning) { - $link = $warning['link']; - $latest = $index === 0 ? "'s latest version" : ''; - $io->writeError( - 'Cannot use '.$name.$latest.' '.$warning['version'].' as it '.$link->getDescription().' '.$link->getTarget().' '.$link->getPrettyConstraint().' which '.$warning['reason'].'.', - true, - $index === 0 ? IOInterface::NORMAL : IOInterface::VERBOSE - ); - } - } - } - - // select highest version if we have many - $package = reset($candidates); $minPriority = BasePackage::$stabilities[$preferredStability]; - foreach ($candidates as $candidate) { - $candidatePriority = $candidate->getStabilityPriority(); - $currentPriority = $package->getStabilityPriority(); + usort($candidates, function (PackageInterface $a, PackageInterface $b) use ($minPriority) { + $aPriority = $a->getStabilityPriority(); + $bPriority = $b->getStabilityPriority(); - // candidate is less stable than our preferred stability, - // and current package is more stable than candidate, skip it - if ($minPriority < $candidatePriority && $currentPriority < $candidatePriority) { - continue; + // A is less stable than our preferred stability, + // and B is more stable than A, select B + if ($minPriority < $aPriority && $bPriority < $aPriority) { + return 1; } - // candidate is less stable than our preferred stability, - // and current package is less stable than candidate, select candidate - if ($minPriority < $candidatePriority && $candidatePriority < $currentPriority) { - $package = $candidate; - continue; + // A is less stable than our preferred stability, + // and B is less stable than A, select A + if ($minPriority < $aPriority && $aPriority < $bPriority) { + return -1; } - // candidate is more stable than our preferred stability, - // and current package is less stable than preferred stability, select candidate - if ($minPriority >= $candidatePriority && $minPriority < $currentPriority) { - $package = $candidate; - continue; + // A is more stable than our preferred stability, + // and B is less stable than preferred stability, select A + if ($minPriority >= $aPriority && $minPriority < $bPriority) { + return -1; } // select highest version of the two - if (version_compare($package->getVersion(), $candidate->getVersion(), '<')) { - $package = $candidate; + return version_compare($b->getVersion(), $a->getVersion()); + }); + + if (count($this->platformConstraints) > 0 && !($platformRequirementFilter instanceof IgnoreAllPlatformRequirementFilter)) { + /** @var array $alreadyWarnedNames */ + $alreadyWarnedNames = []; + + foreach ($candidates as $pkg) { + $reqs = $pkg->getRequires(); + foreach ($reqs as $name => $link) { + if (!PlatformRepository::isPlatformPackage($name) || $platformRequirementFilter->isIgnored($name)) { + continue; + } + if (isset($this->platformConstraints[$name])) { + foreach ($this->platformConstraints[$name] as $providedConstraint) { + if ($link->getConstraint()->matches($providedConstraint)) { + // constraint satisfied, go to next require + continue 2; + } + } + + // constraint not satisfied + $reason = 'is not satisfied by your platform'; + } else { + // Package requires a platform package that is unknown on current platform. + // It means that current platform cannot validate this constraint and so package is not installable. + $reason = 'is missing from your platform'; + } + + if ($io !== null) { + $isFirst = !isset($alreadyWarnedNames[$pkg->getName()]); + $alreadyWarnedNames[$pkg->getName()] = true; + $latest = $isFirst ? "'s latest version" : ''; + $io->writeError( + 'Cannot use '.$pkg->getPrettyName().$latest.' '.$pkg->getPrettyVersion().' as it '.$link->getDescription().' '.$link->getTarget().' '.$link->getPrettyConstraint().' which '.$reason.'.', + true, + $isFirst ? IOInterface::NORMAL : IOInterface::VERBOSE + ); + } + + // skip candidate + continue 2; + } + + $package = $pkg; + break; } + } else { + $package = count($candidates) > 0 ? $candidates[0] : null; + } + + if (!isset($package)) { + return false; } // if we end up with 9999999-dev as selected package, make sure we use the original version instead of the alias diff --git a/tests/Composer/Test/Package/Version/VersionSelectorTest.php b/tests/Composer/Test/Package/Version/VersionSelectorTest.php index 1cf08155e..93b9b07d2 100644 --- a/tests/Composer/Test/Package/Version/VersionSelectorTest.php +++ b/tests/Composer/Test/Package/Version/VersionSelectorTest.php @@ -13,6 +13,8 @@ namespace Composer\Test\Package\Version; use Composer\Filter\PlatformRequirementFilter\PlatformRequirementFilterFactory; +use Composer\IO\BufferIO; +use Composer\IO\IOInterface; use Composer\Package\Version\VersionSelector; use Composer\Package\Package; use Composer\Package\Link; @@ -20,6 +22,7 @@ use Composer\Package\AliasPackage; use Composer\Repository\PlatformRepository; use Composer\Package\Version\VersionParser; use Composer\Test\TestCase; +use Symfony\Component\Console\Output\StreamOutput; class VersionSelectorTest extends TestCase { @@ -29,11 +32,11 @@ class VersionSelectorTest extends TestCase public function testLatestVersionIsReturned(): void { - $packageName = 'foobar'; + $packageName = 'foo/bar'; - $package1 = $this->createPackage('1.2.1'); - $package2 = $this->createPackage('1.2.2'); - $package3 = $this->createPackage('1.2.0'); + $package1 = $this->getPackage('foo/bar', '1.2.1'); + $package2 = $this->getPackage('foo/bar', '1.2.2'); + $package3 = $this->getPackage('foo/bar', '1.2.0'); $packages = array($package1, $package2, $package3); $repositorySet = $this->createMockRepositorySet(); @@ -51,42 +54,56 @@ class VersionSelectorTest extends TestCase public function testLatestVersionIsReturnedThatMatchesPhpRequirements(): void { - $packageName = 'foobar'; + $packageName = 'foo/bar'; $platform = new PlatformRepository(array(), array('php' => '5.5.0')); $repositorySet = $this->createMockRepositorySet(); $versionSelector = new VersionSelector($repositorySet, $platform); $parser = new VersionParser; - $package1 = $this->createPackage('1.0.0'); + $package1 = $this->getPackage('foo/bar', '1.0.0'); $package1->setRequires(array('php' => new Link($packageName, 'php', $parser->parseConstraints('>=5.4'), Link::TYPE_REQUIRE, '>=5.4'))); - $package2 = $this->createPackage('2.0.0'); + $package2 = $this->getPackage('foo/bar', '2.0.0'); $package2->setRequires(array('php' => new Link($packageName, 'php', $parser->parseConstraints('>=5.6'), Link::TYPE_REQUIRE, '>=5.6'))); - $packages = array($package1, $package2); + $package3 = $this->getPackage('foo/bar', '2.1.0'); + $package3->setRequires(array('php' => new Link($packageName, 'php', $parser->parseConstraints('>=5.6'), Link::TYPE_REQUIRE, '>=5.6'))); + $packages = array($package1, $package2, $package3); $repositorySet->expects($this->any()) ->method('findPackages') ->with($packageName, null) ->will($this->returnValue($packages)); - $best = $versionSelector->findBestCandidate($packageName); - $this->assertSame($package1, $best, 'Latest version supporting php 5.5 should be returned (1.0.0)'); + $io = new BufferIO(); + $best = $versionSelector->findBestCandidate($packageName, null, 'stable', null, 0, $io); + $this->assertSame((string) $package1, (string) $best, 'Latest version supporting php 5.5 should be returned (1.0.0)'); + self::assertSame("Cannot use foo/bar's latest version 2.1.0 as it requires php >=5.6 which is not satisfied by your platform.".PHP_EOL, $io->getOutput()); + + $io = new BufferIO('', StreamOutput::VERBOSITY_VERBOSE); + $best = $versionSelector->findBestCandidate($packageName, null, 'stable', null, 0, $io); + $this->assertSame((string) $package1, (string) $best, 'Latest version supporting php 5.5 should be returned (1.0.0)'); + self::assertSame( + "Cannot use foo/bar's latest version 2.1.0 as it requires php >=5.6 which is not satisfied by your platform.".PHP_EOL + ."Cannot use foo/bar 2.0.0 as it requires php >=5.6 which is not satisfied by your platform.".PHP_EOL, + $io->getOutput() + ); + $best = $versionSelector->findBestCandidate($packageName, null, 'stable', PlatformRequirementFilterFactory::ignoreAll()); - $this->assertSame($package2, $best, 'Latest version should be returned when ignoring platform reqs (2.0.0)'); + $this->assertSame((string) $package3, (string) $best, 'Latest version should be returned when ignoring platform reqs (2.1.0)'); } public function testLatestVersionIsReturnedThatMatchesExtRequirements(): void { - $packageName = 'foobar'; + $packageName = 'foo/bar'; $platform = new PlatformRepository(array(), array('ext-zip' => '5.3.0')); $repositorySet = $this->createMockRepositorySet(); $versionSelector = new VersionSelector($repositorySet, $platform); $parser = new VersionParser; - $package1 = $this->createPackage('1.0.0'); + $package1 = $this->getPackage('foo/bar', '1.0.0'); $package1->setRequires(array('ext-zip' => new Link($packageName, 'ext-zip', $parser->parseConstraints('^5.2'), Link::TYPE_REQUIRE, '^5.2'))); - $package2 = $this->createPackage('2.0.0'); + $package2 = $this->getPackage('foo/bar', '2.0.0'); $package2->setRequires(array('ext-zip' => new Link($packageName, 'ext-zip', $parser->parseConstraints('^5.4'), Link::TYPE_REQUIRE, '^5.4'))); $packages = array($package1, $package2); @@ -103,15 +120,15 @@ class VersionSelectorTest extends TestCase public function testLatestVersionIsReturnedThatMatchesPlatformExt(): void { - $packageName = 'foobar'; + $packageName = 'foo/bar'; $platform = new PlatformRepository(); $repositorySet = $this->createMockRepositorySet(); $versionSelector = new VersionSelector($repositorySet, $platform); $parser = new VersionParser; - $package1 = $this->createPackage('1.0.0'); - $package2 = $this->createPackage('2.0.0'); + $package1 = $this->getPackage('foo/bar', '1.0.0'); + $package2 = $this->getPackage('foo/bar', '2.0.0'); $package2->setRequires(array('ext-barfoo' => new Link($packageName, 'ext-barfoo', $parser->parseConstraints('*'), Link::TYPE_REQUIRE, '*'))); $packages = array($package1, $package2); @@ -128,16 +145,16 @@ class VersionSelectorTest extends TestCase public function testLatestVersionIsReturnedThatMatchesComposerRequirements(): void { - $packageName = 'foobar'; + $packageName = 'foo/bar'; $platform = new PlatformRepository(array(), array('composer-runtime-api' => '1.0.0')); $repositorySet = $this->createMockRepositorySet(); $versionSelector = new VersionSelector($repositorySet, $platform); $parser = new VersionParser; - $package1 = $this->createPackage('1.0.0'); + $package1 = $this->getPackage('foo/bar', '1.0.0'); $package1->setRequires(array('composer-runtime-api' => new Link($packageName, 'composer-runtime-api', $parser->parseConstraints('^1.0'), Link::TYPE_REQUIRE, '^1.0'))); - $package2 = $this->createPackage('1.1.0'); + $package2 = $this->getPackage('foo/bar', '1.1.0'); $package2->setRequires(array('composer-runtime-api' => new Link($packageName, 'composer-runtime-api', $parser->parseConstraints('^2.0'), Link::TYPE_REQUIRE, '^2.0'))); $packages = array($package1, $package2); @@ -154,10 +171,10 @@ class VersionSelectorTest extends TestCase public function testMostStableVersionIsReturned(): void { - $packageName = 'foobar'; + $packageName = 'foo/bar'; - $package1 = $this->createPackage('1.0.0'); - $package2 = $this->createPackage('1.1.0-beta'); + $package1 = $this->getPackage('foo/bar', '1.0.0'); + $package2 = $this->getPackage('foo/bar', '1.1.0-beta'); $packages = array($package1, $package2); $repositorySet = $this->createMockRepositorySet(); @@ -174,10 +191,10 @@ class VersionSelectorTest extends TestCase public function testMostStableVersionIsReturnedRegardlessOfOrder(): void { - $packageName = 'foobar'; + $packageName = 'foo/bar'; - $package1 = $this->createPackage('2.x-dev'); - $package2 = $this->createPackage('2.0.0-beta3'); + $package1 = $this->getPackage('foo/bar', '2.x-dev'); + $package2 = $this->getPackage('foo/bar', '2.0.0-beta3'); $packages = array($package1, $package2); $repositorySet = $this->createMockRepositorySet(); @@ -199,10 +216,10 @@ class VersionSelectorTest extends TestCase public function testHighestVersionIsReturned(): void { - $packageName = 'foobar'; + $packageName = 'foo/bar'; - $package1 = $this->createPackage('1.0.0'); - $package2 = $this->createPackage('1.1.0-beta'); + $package1 = $this->getPackage('foo/bar', '1.0.0'); + $package2 = $this->getPackage('foo/bar', '1.1.0-beta'); $packages = array($package1, $package2); $repositorySet = $this->createMockRepositorySet(); @@ -219,11 +236,11 @@ class VersionSelectorTest extends TestCase public function testHighestVersionMatchingStabilityIsReturned(): void { - $packageName = 'foobar'; + $packageName = 'foo/bar'; - $package1 = $this->createPackage('1.0.0'); - $package2 = $this->createPackage('1.1.0-beta'); - $package3 = $this->createPackage('1.2.0-alpha'); + $package1 = $this->getPackage('foo/bar', '1.0.0'); + $package2 = $this->getPackage('foo/bar', '1.1.0-beta'); + $package3 = $this->getPackage('foo/bar', '1.2.0-alpha'); $packages = array($package1, $package2, $package3); $repositorySet = $this->createMockRepositorySet(); @@ -240,10 +257,10 @@ class VersionSelectorTest extends TestCase public function testMostStableUnstableVersionIsReturned(): void { - $packageName = 'foobar'; + $packageName = 'foo/bar'; - $package2 = $this->createPackage('1.1.0-beta'); - $package3 = $this->createPackage('1.2.0-alpha'); + $package2 = $this->getPackage('foo/bar', '1.1.0-beta'); + $package3 = $this->getPackage('foo/bar', '1.2.0-alpha'); $packages = array($package2, $package3); $repositorySet = $this->createMockRepositorySet(); @@ -260,10 +277,10 @@ class VersionSelectorTest extends TestCase public function testDefaultBranchAliasIsNeverReturned(): void { - $packageName = 'foobar'; + $packageName = 'foo/bar'; - $package = $this->createPackage('1.1.0-beta'); - $package2 = $this->createPackage('dev-main'); + $package = $this->getPackage('foo/bar', '1.1.0-beta'); + $package2 = $this->getPackage('foo/bar', 'dev-main'); $package2Alias = new AliasPackage($package2, VersionParser::DEFAULT_BRANCH_ALIAS, VersionParser::DEFAULT_BRANCH_ALIAS); $packages = array($package, $package2Alias); @@ -357,18 +374,6 @@ class VersionSelectorTest extends TestCase ); } - /** - * @param string $version - * - * @return Package - */ - private function createPackage(string $version): Package - { - $parser = new VersionParser(); - - return new Package('foo', $parser->normalize($version), $version); - } - /** * @return \PHPUnit\Framework\MockObject\MockObject&\Composer\Repository\RepositorySet */