From 189d5adab0dd504d1c4eea2f6dde7f2d9c0302b2 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 30 Jan 2020 17:11:34 +0100 Subject: [PATCH] Fix reporting of replace conflicts to not mention provides --- src/Composer/DependencyResolver/Rule.php | 92 +++++++++++++------ .../Test/DependencyResolver/SolverTest.php | 2 +- .../installer/provider-conflicts.test | 4 +- .../installer/provider-conflicts2.test | 4 +- .../installer/provider-conflicts3.test | 8 +- 5 files changed, 74 insertions(+), 36 deletions(-) diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index 8f9dbc4ca..58f6962cd 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -190,49 +190,77 @@ abstract class Rule if (count($literals) === 2 && $literals[0] < 0 && $literals[1] < 0) { $package1 = $pool->literalToPackage($literals[0]); $package2 = $pool->literalToPackage($literals[1]); - $conflictingNames = array_values(array_intersect($package1->getNames(), $package2->getNames())); - $provideClash = count($conflictingNames) > 1 ? '['.implode(', ', $conflictingNames).']' : $conflictingNames[0]; - if ($conflictingNames && isset($installedMap[$package1->id]) && !isset($installedMap[$package2->id])) { - // swap vars so the if below passes - $tmp = $package2; - $package2 = $package1; - $package1 = $tmp; + $replaces1 = $this->getReplacedNames($package1); + $replaces2 = $this->getReplacedNames($package2); + + $reason = null; + if ($conflictingNames = array_values(array_intersect($replaces1, $replaces2))) { + $reason = 'They both replace '.(count($conflictingNames) > 1 ? '['.implode(', ', $conflictingNames).']' : $conflictingNames[0]).' and can thus not coexist.'; + } elseif (in_array($package1->getName(), $replaces2, true)) { + $reason = $package2->getName().' replaces '.$package1->getName().' and can thus not coexist with it.'; + } elseif (in_array($package2->getName(), $replaces1, true)) { + $reason = $package1->getName().' replaces '.$package2->getName().' and can thus not coexist with it.'; } - if ($conflictingNames && !isset($installedMap[$package1->id]) && isset($installedMap[$package2->id])) { - return $package1->getPrettyString().' can not be installed as that would require removing '.$package2->getPrettyString().'. They both provide '.$provideClash.' and can thus not coexist.'; - } - if (!isset($installedMap[$package1->id]) && !isset($installedMap[$package2->id])) { - if ($conflictingNames) { - return 'Only one of these can be installed: '.$package1->getPrettyString().', '.$package2->getPrettyString().'. They both provide '.$provideClash.' and can thus not coexist.'; + + if ($reason) { + if (isset($installedMap[$package1->id]) && !isset($installedMap[$package2->id])) { + // swap vars so the if below passes + $tmp = $package2; + $package2 = $package1; + $package1 = $tmp; + } + if (!isset($installedMap[$package1->id]) && isset($installedMap[$package2->id])) { + return $package1->getPrettyString().' can not be installed as that would require removing '.$package2->getPrettyString().'. '.$reason; } - return 'Only one of these can be installed: '.$package1->getPrettyString().', '.$package2->getPrettyString().'.'; + if (!isset($installedMap[$package1->id]) && !isset($installedMap[$package2->id])) { + return 'Only one of these can be installed: '.$package1->getPrettyString().', '.$package2->getPrettyString().'. '.$reason; + } } + + return 'Only one of these can be installed: '.$package1->getPrettyString().', '.$package2->getPrettyString().'.'; } return $ruleText; case self::RULE_INSTALLED_PACKAGE_OBSOLETES: return $ruleText; case self::RULE_PACKAGE_SAME_NAME: - $conflictingNames = null; - $allNames = array(); + $replacedNames = null; + $packageNames = array(); foreach ($literals as $literal) { $package = $pool->literalToPackage($literal); - if ($conflictingNames === null) { - $conflictingNames = $package->getNames(); - } else { - $conflictingNames = array_values(array_intersect($conflictingNames, $package->getNames())); + $pkgReplaces = $this->getReplacedNames($package); + if ($pkgReplaces) { + if ($replacedNames === null) { + $replacedNames = $this->getReplacedNames($package); + } else { + $replacedNames = array_intersect($replacedNames, $this->getReplacedNames($package)); + } } - $allNames = array_unique(array_merge($allNames, $package->getNames())); - } - $provideClash = count($conflictingNames) > 1 ? '['.implode(', ', $conflictingNames).']' : $conflictingNames[0]; - - if ($conflictingNames && count($allNames) > 1) { - return 'Only one of these can be installed: ' . $this->formatPackagesUnique($pool, $literals) . '. They all provide '.$provideClash.' and can thus not coexist.'; + $packageNames[$package->getName()] = true; } - return 'Only one of these can be installed: ' . $this->formatPackagesUnique($pool, $literals) . '.'; + if ($replacedNames) { + $replacedNames = array_values(array_intersect(array_keys($packageNames), $replacedNames)); + } + if ($replacedNames && count($packageNames) > 1) { + $replacer = null; + foreach ($literals as $literal) { + $package = $pool->literalToPackage($literal); + if (array_intersect($replacedNames, $this->getReplacedNames($package))) { + $replacer = $package; + break; + } + } + $replacedNames = count($replacedNames) > 1 ? '['.implode(', ', $replacedNames).']' : $replacedNames[0]; + + if ($replacer) { + return 'Only one of these can be installed: ' . $this->formatPackagesUnique($pool, $literals) . '. '.$replacer->getName().' replaces '.$replacedNames.' and can thus not coexist with it.'; + } + } + + return 'You can only install one version of a package, so only one of these can be installed: ' . $this->formatPackagesUnique($pool, $literals) . '.'; case self::RULE_PACKAGE_IMPLICIT_OBSOLETES: return $ruleText; case self::RULE_LEARNED: @@ -272,4 +300,14 @@ abstract class Rule return Problem::getPackageList($packages); } + + private function getReplacedNames(PackageInterface $package) + { + $names = array(); + foreach ($package->getReplaces() as $link) { + $names[] = $link->getTarget(); + } + + return $names; + } } diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index c09ac07b1..38e16c6a0 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/tests/Composer/Test/DependencyResolver/SolverTest.php @@ -726,7 +726,7 @@ class SolverTest extends TestCase $msg .= " - C 1.0 requires d >= 1.0 -> satisfiable by D[1.0].\n"; $msg .= " - D 1.0 requires b < 1.0 -> satisfiable by B[0.9].\n"; $msg .= " - B 1.0 requires c >= 1.0 -> satisfiable by C[1.0].\n"; - $msg .= " - Only one of these can be installed: B[0.9, 1.0].\n"; + $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)); diff --git a/tests/Composer/Test/Fixtures/installer/provider-conflicts.test b/tests/Composer/Test/Fixtures/installer/provider-conflicts.test index 4e9ce54fa..1c8bd5608 100644 --- a/tests/Composer/Test/Fixtures/installer/provider-conflicts.test +++ b/tests/Composer/Test/Fixtures/installer/provider-conflicts.test @@ -1,5 +1,5 @@ --TEST-- -Test that providers provided by a dependent and root package cause a conflict +Test that names provided by a dependent and root package cause a conflict only for replace --COMPOSER-- { "version": "1.2.3", @@ -42,7 +42,7 @@ Your requirements could not be resolved to an installable set of packages. Problem 1 - __root__ is present at version 1.2.3 and cannot be modified by Composer - - provider/pkg 1.0.0 can not be installed as that would require removing __root__ 1.2.3. They both provide [root-provided/transitive-provided, root-replaced/transitive-provided, root-provided/transitive-replaced, root-replaced/transitive-replaced] and can thus not coexist. + - provider/pkg 1.0.0 can not be installed as that would require removing __root__ 1.2.3. They both replace root-replaced/transitive-replaced and can thus not coexist. - Root composer.json requires provider/pkg * -> satisfiable by provider/pkg[1.0.0]. --EXPECT-- diff --git a/tests/Composer/Test/Fixtures/installer/provider-conflicts2.test b/tests/Composer/Test/Fixtures/installer/provider-conflicts2.test index 18f8f2db6..343dab537 100644 --- a/tests/Composer/Test/Fixtures/installer/provider-conflicts2.test +++ b/tests/Composer/Test/Fixtures/installer/provider-conflicts2.test @@ -1,5 +1,5 @@ --TEST-- -Test that providers provided by two dependents cause a conflict +Test that names provided by two dependents cause a conflict --COMPOSER-- { "repositories": [ @@ -38,7 +38,7 @@ Your requirements could not be resolved to an installable set of packages. Problem 1 - Root composer.json requires provider/pkg * -> satisfiable by provider/pkg[1.0.0]. - - Only one of these can be installed: replacer/pkg 1.0.0, provider/pkg 1.0.0. They both provide third/pkg and can thus not coexist. + - Only one of these can be installed: replacer/pkg 1.0.0, provider/pkg 1.0.0. - Root composer.json requires replacer/pkg * -> satisfiable by replacer/pkg[1.0.0]. --EXPECT-- diff --git a/tests/Composer/Test/Fixtures/installer/provider-conflicts3.test b/tests/Composer/Test/Fixtures/installer/provider-conflicts3.test index 7cc4f27b0..3a865f3bf 100644 --- a/tests/Composer/Test/Fixtures/installer/provider-conflicts3.test +++ b/tests/Composer/Test/Fixtures/installer/provider-conflicts3.test @@ -39,14 +39,14 @@ Your requirements could not be resolved to an installable set of packages. Problem 1 - Conclusion: don't install regular/pkg 1.0.3, learned rules: - Root composer.json requires replacer/pkg 2.* -> satisfiable by replacer/pkg[2.0.0, 2.0.1, 2.0.2, 2.0.3]. - - Only one of these can be installed: regular/pkg[1.0.3, 1.0.2, 1.0.1, 1.0.0], replacer/pkg[2.0.3, 2.0.2, 2.0.1, 2.0.0]. They all provide regular/pkg and can thus not coexist. + - Only one of these can be installed: regular/pkg[1.0.3, 1.0.2, 1.0.1, 1.0.0], replacer/pkg[2.0.3, 2.0.2, 2.0.1, 2.0.0]. replacer/pkg replaces regular/pkg and can thus not coexist with it. - Conclusion: don't install regular/pkg 1.0.2, learned rules: - Root composer.json requires replacer/pkg 2.* -> satisfiable by replacer/pkg[2.0.0, 2.0.1, 2.0.2, 2.0.3]. - - Only one of these can be installed: regular/pkg[1.0.3, 1.0.2, 1.0.1, 1.0.0], replacer/pkg[2.0.3, 2.0.2, 2.0.1, 2.0.0]. They all provide regular/pkg and can thus not coexist. + - Only one of these can be installed: regular/pkg[1.0.3, 1.0.2, 1.0.1, 1.0.0], replacer/pkg[2.0.3, 2.0.2, 2.0.1, 2.0.0]. replacer/pkg replaces regular/pkg and can thus not coexist with it. - Conclusion: don't install regular/pkg 1.0.1, learned rules: - Root composer.json requires replacer/pkg 2.* -> satisfiable by replacer/pkg[2.0.0, 2.0.1, 2.0.2, 2.0.3]. - - Only one of these can be installed: regular/pkg[1.0.3, 1.0.2, 1.0.1, 1.0.0], replacer/pkg[2.0.3, 2.0.2, 2.0.1, 2.0.0]. They all provide regular/pkg and can thus not coexist. - - Only one of these can be installed: regular/pkg[1.0.3, 1.0.2, 1.0.1, 1.0.0], replacer/pkg[2.0.3, 2.0.2, 2.0.1, 2.0.0]. They all provide regular/pkg and can thus not coexist. + - Only one of these can be installed: regular/pkg[1.0.3, 1.0.2, 1.0.1, 1.0.0], replacer/pkg[2.0.3, 2.0.2, 2.0.1, 2.0.0]. replacer/pkg replaces regular/pkg and can thus not coexist with it. + - Only one of these can be installed: regular/pkg[1.0.3, 1.0.2, 1.0.1, 1.0.0], replacer/pkg[2.0.3, 2.0.2, 2.0.1, 2.0.0]. replacer/pkg replaces regular/pkg and can thus not coexist with it. - Root composer.json requires regular/pkg 1.* -> satisfiable by regular/pkg[1.0.0, 1.0.1, 1.0.2, 1.0.3]. - Root composer.json requires replacer/pkg 2.* -> satisfiable by replacer/pkg[2.0.0, 2.0.1, 2.0.2, 2.0.3].