Browse Source

Shorten long lists of similar versions in problem output, fixes #8743

Jordi Boggiano 4 years ago
parent
commit
80a7c40c76

+ 31 - 12
src/Composer/DependencyResolver/Problem.php

@@ -66,7 +66,7 @@ class Problem
      * @param  array  $installedMap A map of all present packages
      * @return string
      */
-    public function getPrettyString(RepositorySet $repositorySet, Request $request, Pool $pool, array $installedMap = array(), array $learnedPool = array())
+    public function getPrettyString(RepositorySet $repositorySet, Request $request, Pool $pool, $isVerbose, array $installedMap = array(), array $learnedPool = array())
     {
         // TODO doesn't this entirely defeat the purpose of the problem sections? what's the point of sections?
         $reasons = call_user_func_array('array_merge', array_reverse($this->reasons));
@@ -90,13 +90,13 @@ class Problem
             }
 
             if (empty($packages)) {
-                return "\n    ".implode(self::getMissingPackageReason($repositorySet, $request, $pool, $packageName, $constraint));
+                return "\n    ".implode(self::getMissingPackageReason($repositorySet, $request, $pool, $isVerbose, $packageName, $constraint));
             }
         }
 
         $messages = array();
         foreach ($reasons as $rule) {
-            $messages[] = $rule->getPrettyString($repositorySet, $request, $pool, $installedMap, $learnedPool);
+            $messages[] = $rule->getPrettyString($repositorySet, $request, $pool, $isVerbose, $installedMap, $learnedPool);
         }
 
         return "\n    - ".implode("\n    - ", array_unique($messages));
@@ -138,7 +138,7 @@ class Problem
     /**
      * @internal
      */
-    public static function getMissingPackageReason(RepositorySet $repositorySet, Request $request, Pool $pool, $packageName, $constraint = null)
+    public static function getMissingPackageReason(RepositorySet $repositorySet, Request $request, Pool $pool, $isVerbose, $packageName, $constraint = null)
     {
         // handle php/hhvm
         if ($packageName === 'php' || $packageName === 'php-64bit' || $packageName === 'hhvm') {
@@ -210,7 +210,7 @@ class Problem
                     return $rootReqs[$packageName]->matches(new Constraint('==', $p->getVersion()));
                 });
                 if (0 === count($filtered)) {
-                    return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', ', 'found '.self::getPackageList($packages).' but '.(self::hasMultipleNames($packages) ? 'these conflict' : 'it conflicts').' with your root composer.json require ('.$rootReqs[$packageName]->getPrettyString().').');
+                    return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', ', 'found '.self::getPackageList($packages, $isVerbose).' but '.(self::hasMultipleNames($packages) ? 'these conflict' : 'it conflicts').' with your root composer.json require ('.$rootReqs[$packageName]->getPrettyString().').');
                 }
             }
 
@@ -220,7 +220,7 @@ class Problem
                     return $fixedConstraint->matches(new Constraint('==', $p->getVersion()));
                 });
                 if (0 === count($filtered)) {
-                    return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', ', 'found '.self::getPackageList($packages).' but the package is fixed to '.$fixedPackage->getPrettyVersion().' (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.');
+                    return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', ', 'found '.self::getPackageList($packages, $isVerbose).' but the package is fixed to '.$fixedPackage->getPrettyVersion().' (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.');
                 }
             }
 
@@ -229,15 +229,15 @@ class Problem
             });
 
             if (!$nonLockedPackages) {
-                return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', ', 'found '.self::getPackageList($packages).' in lock file but not in remote repositories, make sure you avoid updating this package to keep the one from lock file.');
+                return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', ', 'found '.self::getPackageList($packages, $isVerbose).' in lock file but not in remote repositories, make sure you avoid updating this package to keep the one from lock file.');
             }
 
-            return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', ', 'found '.self::getPackageList($packages).' but '.(self::hasMultipleNames($packages) ? 'these conflict' : 'it conflicts').' with another require.');
+            return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', ', 'found '.self::getPackageList($packages, $isVerbose).' but '.(self::hasMultipleNames($packages) ? 'these conflict' : 'it conflicts').' with another require.');
         }
 
         // check if the package is found when bypassing stability checks
         if ($packages = $repositorySet->findPackages($packageName, $constraint, RepositorySet::ALLOW_UNACCEPTABLE_STABILITIES)) {
-            return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', ', 'found '.self::getPackageList($packages).' but '.(self::hasMultipleNames($packages) ? 'these do' : 'it does').' not match your minimum-stability.');
+            return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', ', 'found '.self::getPackageList($packages, $isVerbose).' but '.(self::hasMultipleNames($packages) ? 'these do' : 'it does').' not match your minimum-stability.');
         }
 
         // check if the package is found when bypassing the constraint check
@@ -257,10 +257,10 @@ class Problem
                     }
                 }
 
-                return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', it is ', 'satisfiable by '.self::getPackageList($nextRepoPackages).' from '.$nextRepo->getRepoName().' but '.self::getPackageList($higherRepoPackages).' from '.reset($higherRepoPackages)->getRepository()->getRepoName().' has higher repository priority. The packages with higher priority do not match your constraint and are therefore not installable. See https://getcomposer.org/repoprio for details and assistance.');
+                return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', it is ', 'satisfiable by '.self::getPackageList($nextRepoPackages, $isVerbose).' from '.$nextRepo->getRepoName().' but '.self::getPackageList($higherRepoPackages, $isVerbose).' from '.reset($higherRepoPackages)->getRepository()->getRepoName().' has higher repository priority. The packages with higher priority do not match your constraint and are therefore not installable. See https://getcomposer.org/repoprio for details and assistance.');
             }
 
-            return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', ', 'found '.self::getPackageList($packages).' but '.(self::hasMultipleNames($packages) ? 'these do' : 'it does').' not match your constraint.');
+            return array("- Root composer.json requires $packageName".self::constraintToText($constraint) . ', ', 'found '.self::getPackageList($packages, $isVerbose).' but '.(self::hasMultipleNames($packages) ? 'these do' : 'it does').' not match your constraint.');
         }
 
         if (!preg_match('{^[A-Za-z0-9_./-]+$}', $packageName)) {
@@ -287,7 +287,7 @@ class Problem
     /**
      * @internal
      */
-    public static function getPackageList(array $packages)
+    public static function getPackageList(array $packages, $isVerbose)
     {
         $prepared = array();
         foreach ($packages as $package) {
@@ -299,6 +299,25 @@ class Problem
             if (isset($package['versions'][VersionParser::DEV_MASTER_ALIAS]) && isset($package['versions']['dev-master'])) {
                 unset($package['versions'][VersionParser::DEV_MASTER_ALIAS]);
             }
+            if (!$isVerbose && count($package['versions']) > 4) {
+                $filtered = array();
+                $byMajor = array();
+                foreach ($package['versions'] as $version => $pretty) {
+                    $byMajor[preg_replace('{^(\d+)\..*}', '$1', $version)][] = $pretty;
+                }
+                foreach ($byMajor as $versions) {
+                    if (count($versions) > 4) {
+                        $filtered[] = $versions[0];
+                        $filtered[] = '...';
+                        $filtered[] = $versions[count($versions) - 1];
+                    } else {
+                        $filtered = array_merge($filtered, $versions);
+                    }
+                }
+
+                $package['versions'] = $filtered;
+            }
+
             $prepared[$name] = $package['name'].'['.implode(', ', $package['versions']).']';
         }
 

+ 9 - 9
src/Composer/DependencyResolver/Rule.php

@@ -127,7 +127,7 @@ abstract class Rule
         return $this->getReason() === self::RULE_FIXED && $this->reasonData['lockable'];
     }
 
-    public function getPrettyString(RepositorySet $repositorySet, Request $request, Pool $pool, array $installedMap = array(), array $learnedPool = array())
+    public function getPrettyString(RepositorySet $repositorySet, Request $request, Pool $pool, $isVerbose, array $installedMap = array(), array $learnedPool = array())
     {
         $literals = $this->getLiterals();
 
@@ -152,7 +152,7 @@ abstract class Rule
                     return 'No package found to satisfy root composer.json require '.$packageName.($constraint ? ' '.$constraint->getPrettyString() : '');
                 }
 
-                return 'Root composer.json requires '.$packageName.($constraint ? ' '.$constraint->getPrettyString() : '').' -> satisfiable by '.$this->formatPackagesUnique($pool, $packages).'.';
+                return 'Root composer.json requires '.$packageName.($constraint ? ' '.$constraint->getPrettyString() : '').' -> satisfiable by '.$this->formatPackagesUnique($pool, $packages, $isVerbose).'.';
 
             case self::RULE_FIXED:
                 $package = $this->deduplicateMasterAlias($this->reasonData['package']);
@@ -179,11 +179,11 @@ abstract class Rule
 
                 $text = $this->reasonData->getPrettyString($sourcePackage);
                 if ($requires) {
-                    $text .= ' -> satisfiable by ' . $this->formatPackagesUnique($pool, $requires) . '.';
+                    $text .= ' -> satisfiable by ' . $this->formatPackagesUnique($pool, $requires, $isVerbose) . '.';
                 } else {
                     $targetName = $this->reasonData->getTarget();
 
-                    $reason = Problem::getMissingPackageReason($repositorySet, $request, $pool, $targetName, $this->reasonData->getConstraint());
+                    $reason = Problem::getMissingPackageReason($repositorySet, $request, $pool, $isVerbose, $targetName, $this->reasonData->getConstraint());
 
                     return $text . ' -> ' . $reason[1];
                 }
@@ -227,13 +227,13 @@ abstract class Rule
                     }
 
                     if ($installedPackages && $removablePackages) {
-                        return $this->formatPackagesUnique($pool, $removablePackages).' cannot be installed as that would require removing '.$this->formatPackagesUnique($pool, $installedPackages).'. '.$reason;
+                        return $this->formatPackagesUnique($pool, $removablePackages, $isVerbose).' cannot be installed as that would require removing '.$this->formatPackagesUnique($pool, $installedPackages, $isVerbose).'. '.$reason;
                     }
 
-                    return 'Only one of these can be installed: '.$this->formatPackagesUnique($pool, $literals).'. '.$reason;
+                    return 'Only one of these can be installed: '.$this->formatPackagesUnique($pool, $literals, $isVerbose).'. '.$reason;
                 }
 
-                return 'You can only install one version of a package, so only one of these can be installed: ' . $this->formatPackagesUnique($pool, $literals) . '.';
+                return 'You can only install one version of a package, so only one of these can be installed: ' . $this->formatPackagesUnique($pool, $literals, $isVerbose) . '.';
             case self::RULE_LEARNED:
                 if (isset($learnedPool[$this->reasonData])) {
                     $learnedString = ', learned rules:'."\n        - ";
@@ -260,7 +260,7 @@ abstract class Rule
      *
      * @return string
      */
-    protected function formatPackagesUnique($pool, array $packages)
+    protected function formatPackagesUnique($pool, array $packages, $isVerbose)
     {
         $prepared = array();
         foreach ($packages as $index => $package) {
@@ -269,7 +269,7 @@ abstract class Rule
             }
         }
 
-        return Problem::getPackageList($packages);
+        return Problem::getPackageList($packages, $isVerbose);
     }
 
     private function getReplacedNames(PackageInterface $package)

+ 2 - 2
src/Composer/DependencyResolver/RuleSet.php

@@ -157,13 +157,13 @@ class RuleSet implements \IteratorAggregate, \Countable
         return array_keys($types);
     }
 
-    public function getPrettyString(RepositorySet $repositorySet = null, Request $request = null, Pool $pool = null)
+    public function getPrettyString(RepositorySet $repositorySet = null, Request $request = null, Pool $pool = null, $isVerbose = false)
     {
         $string = "\n";
         foreach ($this->rules as $type => $rules) {
             $string .= str_pad(self::$types[$type], 8, ' ') . ": ";
             foreach ($rules as $rule) {
-                $string .= ($repositorySet && $request && $pool ? $rule->getPrettyString($repositorySet, $request, $pool) : $rule)."\n";
+                $string .= ($repositorySet && $request && $pool ? $rule->getPrettyString($repositorySet, $request, $pool, $isVerbose) : $rule)."\n";
             }
             $string .= "\n\n";
         }

+ 2 - 2
src/Composer/DependencyResolver/SolverProblemsException.php

@@ -31,7 +31,7 @@ class SolverProblemsException extends \RuntimeException
         parent::__construct('Failed resolving dependencies with '.count($problems).' problems, call getPrettyString to get formatted details', 2);
     }
 
-    public function getPrettyString(RepositorySet $repositorySet, Request $request, Pool $pool, $isDevExtraction = false)
+    public function getPrettyString(RepositorySet $repositorySet, Request $request, Pool $pool, $isVerbose, $isDevExtraction = false)
     {
         $installedMap = $request->getPresentMap(true);
         $hasExtensionProblems = false;
@@ -39,7 +39,7 @@ class SolverProblemsException extends \RuntimeException
 
         $problems = array();
         foreach ($this->problems as $problem) {
-            $problems[] = $problem->getPrettyString($repositorySet, $request, $pool, $installedMap, $this->learnedPool)."\n";
+            $problems[] = $problem->getPrettyString($repositorySet, $request, $pool, $isVerbose, $installedMap, $this->learnedPool)."\n";
 
             if (!$hasExtensionProblems && $this->hasExtensionProblems($problem->getReasons())) {
                 $hasExtensionProblems = true;

+ 3 - 3
src/Composer/Installer.php

@@ -402,7 +402,7 @@ class Installer
             $solver = null;
         } catch (SolverProblemsException $e) {
             $this->io->writeError('<error>Your requirements could not be resolved to an installable set of packages.</error>', true, IOInterface::QUIET);
-            $this->io->writeError($e->getPrettyString($repositorySet, $request, $pool));
+            $this->io->writeError($e->getPrettyString($repositorySet, $request, $pool, $this->io->isVerbose()));
             if (!$this->devMode) {
                 $this->io->writeError('<warning>Running update with --no-dev does not mean require-dev is ignored, it just means the packages will not be installed. If dev requirements are blocking the update you have to resolve those problems.</warning>', true, IOInterface::QUIET);
             }
@@ -563,7 +563,7 @@ class Installer
             $this->io->writeError('<error>Unable to find a compatible set of packages based on your non-dev requirements alone.</error>', true, IOInterface::QUIET);
             $this->io->writeError('Your requirements can be resolved successfully when require-dev packages are present.');
             $this->io->writeError('You may need to move packages from require-dev or some of their dependencies to require.');
-            $this->io->writeError($e->getPrettyString($repositorySet, $request, $pool, true));
+            $this->io->writeError($e->getPrettyString($repositorySet, $request, $pool, $this->io->isVerbose(), true));
 
             return max(1, $e->getCode());
         }
@@ -627,7 +627,7 @@ class Installer
                 }
             } catch (SolverProblemsException $e) {
                 $this->io->writeError('<error>Your lock file does not contain a compatible set of packages. Please run composer update.</error>', true, IOInterface::QUIET);
-                $this->io->writeError($e->getPrettyString($repositorySet, $request, $pool));
+                $this->io->writeError($e->getPrettyString($repositorySet, $request, $pool, $this->io->isVerbose()));
 
                 return max(1, $e->getCode());
             }

+ 1 - 1
tests/Composer/Test/DependencyResolver/RuleTest.php

@@ -104,6 +104,6 @@ class RuleTest extends TestCase
 
         $rule = new GenericRule(array($p1->getId(), -$p2->getId()), Rule::RULE_PACKAGE_REQUIRES, new Link('baz', 'foo'));
 
-        $this->assertEquals('baz 1.1 relates to foo -> satisfiable by foo[2.1].', $rule->getPrettyString($repositorySetMock, $requestMock, $pool));
+        $this->assertEquals('baz 1.1 relates to foo -> satisfiable by foo[2.1].', $rule->getPrettyString($repositorySetMock, $requestMock, $pool, false));
     }
 }

+ 4 - 4
tests/Composer/Test/DependencyResolver/SolverTest.php

@@ -83,7 +83,7 @@ class SolverTest extends TestCase
             $problems = $e->getProblems();
             $this->assertCount(1, $problems);
             $this->assertEquals(2, $e->getCode());
-            $this->assertEquals("\n    - Root composer.json requires b, it could not be found in any version, there may be a typo in the package name.", $problems[0]->getPrettyString($this->repoSet, $this->request, $this->pool));
+            $this->assertEquals("\n    - Root composer.json requires b, it could not be found in any version, there may be a typo in the package name.", $problems[0]->getPrettyString($this->repoSet, $this->request, $this->pool, false));
         }
     }
 
@@ -654,7 +654,7 @@ class SolverTest extends TestCase
             $msg .= "    - Root composer.json requires a -> satisfiable by A[1.0].\n";
             $msg .= "    - A 1.0 conflicts with B 1.0.\n";
             $msg .= "    - Root composer.json requires b -> satisfiable by B[1.0].\n";
-            $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool));
+            $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool, false));
         }
     }
 
@@ -684,7 +684,7 @@ class SolverTest extends TestCase
             $msg .= "  Problem 1\n";
             $msg .= "    - Root composer.json requires a -> satisfiable by A[1.0].\n";
             $msg .= "    - A 1.0 requires b >= 2.0 -> found B[1.0] but it does not match your constraint.\n";
-            $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool));
+            $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool, false));
         }
     }
 
@@ -729,7 +729,7 @@ class SolverTest extends TestCase
             $msg .= "    - You can only install one version of a package, so only one of these can be installed: B[0.9, 1.0].\n";
             $msg .= "    - A 1.0 requires b >= 1.0 -> satisfiable by B[1.0].\n";
             $msg .= "    - Root composer.json requires a -> satisfiable by A[1.0].\n";
-            $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool));
+            $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool, false));
         }
     }
 

+ 116 - 0
tests/Composer/Test/Fixtures/installer/problems-reduce-versions.test

@@ -0,0 +1,116 @@
+--TEST--
+Test the error output minifies version lists
+--COMPOSER--
+{
+    "repositories": [
+        {
+            "type": "package",
+            "package": [
+                {"name": "a/a", "version": "1.0.0", "require": {"b/b": "1.0.0"}},
+                {"name": "b/b", "version": "1.0.0"},
+                {"name": "b/b", "version": "1.0.1"},
+                {"name": "b/b", "version": "1.0.2"},
+                {"name": "b/b", "version": "1.0.3"},
+                {"name": "b/b", "version": "1.0.4"},
+                {"name": "b/b", "version": "1.0.5"},
+                {"name": "b/b", "version": "1.0.6"},
+                {"name": "b/b", "version": "1.0.7"},
+                {"name": "b/b", "version": "1.0.8"},
+                {"name": "b/b", "version": "1.0.9"},
+                {"name": "b/b", "version": "1.1.0"},
+                {"name": "b/b", "version": "1.1.1"},
+                {"name": "b/b", "version": "1.1.2"},
+                {"name": "b/b", "version": "1.1.3"},
+                {"name": "b/b", "version": "v1.1.4"},
+                {"name": "b/b", "version": "1.1.5"},
+                {"name": "b/b", "version": "v1.1.6"},
+                {"name": "b/b", "version": "1.1.7-alpha"},
+                {"name": "b/b", "version": "1.1.8"},
+                {"name": "b/b", "version": "1.1.9"},
+                {"name": "b/b", "version": "1.2.0"},
+                {"name": "b/b", "version": "1.2.1"},
+                {"name": "b/b", "version": "1.2.2"},
+                {"name": "b/b", "version": "1.2.3"},
+                {"name": "b/b", "version": "1.2.4"},
+                {"name": "b/b", "version": "1.2.5"},
+                {"name": "b/b", "version": "1.2.6"},
+                {"name": "b/b", "version": "1.2.7"},
+                {"name": "b/b", "version": "1.2.8"},
+                {"name": "b/b", "version": "1.2.9"},
+                {"name": "b/b", "version": "2.0.0"},
+                {"name": "b/b", "version": "2.0.1"},
+                {"name": "b/b", "version": "2.0.2"},
+                {"name": "b/b", "version": "2.0.3"},
+                {"name": "b/b", "version": "2.0.4"},
+                {"name": "b/b", "version": "2.0.5"},
+                {"name": "b/b", "version": "2.0.6"},
+                {"name": "b/b", "version": "2.0.7"},
+                {"name": "b/b", "version": "2.0.8"},
+                {"name": "b/b", "version": "2.0.9"},
+                {"name": "b/b", "version": "2.1.0"},
+                {"name": "b/b", "version": "2.1.1"},
+                {"name": "b/b", "version": "2.1.2"},
+                {"name": "b/b", "version": "2.1.3"},
+                {"name": "b/b", "version": "2.1.4"},
+                {"name": "b/b", "version": "2.1.5"},
+                {"name": "b/b", "version": "2.1.6"},
+                {"name": "b/b", "version": "2.1.7"},
+                {"name": "b/b", "version": "2.1.8"},
+                {"name": "b/b", "version": "2.1.9"},
+                {"name": "b/b", "version": "2.2.0"},
+                {"name": "b/b", "version": "2.2.1"},
+                {"name": "b/b", "version": "2.2.2"},
+                {"name": "b/b", "version": "2.2.3"},
+                {"name": "b/b", "version": "2.2.4"},
+                {"name": "b/b", "version": "2.2.5"},
+                {"name": "b/b", "version": "2.2.6"},
+                {"name": "b/b", "version": "2.2.7"},
+                {"name": "b/b", "version": "2.2.8"},
+                {"name": "b/b", "version": "2.2.9"},
+                {"name": "b/b", "version": "2.3.0-RC"},
+                {"name": "b/b", "version": "3.0.0"},
+                {"name": "b/b", "version": "3.0.1"},
+                {"name": "b/b", "version": "3.0.2"},
+                {"name": "b/b", "version": "3.0.3"},
+                {"name": "b/b", "version": "4.0.0"}
+            ]
+        }
+    ],
+    "require": {
+        "a/a": "*",
+        "b/b": "^1.1 || ^2.0 || ^3.0"
+    },
+    "minimum-stability": "dev"
+}
+
+--LOCK--
+{
+    "packages": [
+        {"name": "b/b", "version": "1.0.0"}
+    ],
+    "packages-dev": [],
+    "aliases": [],
+    "minimum-stability": "dev",
+    "stability-flags": [],
+    "prefer-stable": false,
+    "prefer-lowest": false,
+    "platform": [],
+    "platform-dev": []
+}
+
+--RUN--
+update a/a
+
+--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
+    - Root composer.json requires b/b ^1.1 || ^2.0 || ^3.0, found b/b[1.1.0, ..., 1.2.9, 2.0.0, ..., 2.3.0-RC, 3.0.0, 3.0.1, 3.0.2, 3.0.3] but the package is fixed to 1.0.0 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.
+
+--EXPECT--
+