From 045b5c6b6bb2eb7915527672eb9809ff6eebd956 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 13 Nov 2020 14:16:40 +0100 Subject: [PATCH 1/9] Add test case verifying conflicts on alias prevent it from getting installed --- ...lict-with-alias-prevents-installation.test | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-installation.test diff --git a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-installation.test b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-installation.test new file mode 100644 index 000000000..b4425a6eb --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-installation.test @@ -0,0 +1,27 @@ +--TEST-- +Test that conflict on a branch alias is respected +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "some/dep", "version": "1.0.0" }, + { "name": "some/dep", "version": "1.1.0" }, + { "name": "some/dep", "version": "1.2.0" }, + { "name": "some/dep", "version": "dev-main", "extra": {"branch-alias": {"dev-main": "1.3.x-dev"} } }, + { "name": "some/dep", "version": "1.2.x-dev" } + ] + } + ], + "require": { + "some/dep": "^1.0@dev" + }, + "conflict": { + "some/dep": ">=1.3" + } +} +--RUN-- +update +--EXPECT-- +Installing some/dep (1.2.x-dev) From 3764b3007ded14db47e3341030b6ac6397aa86af Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 13 Nov 2020 15:58:46 +0100 Subject: [PATCH 2/9] Explicit conflict rule generation needs to use pool->whatProvides If relying on packages added by name in the generator aliases will be skipped. --- src/Composer/DependencyResolver/RuleSetGenerator.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index 3744bdd2a..19fcc8c12 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -199,6 +199,7 @@ class RuleSetGenerator /** @var PackageInterface $package */ foreach ($this->addedMap as $package) { foreach ($package->getConflicts() as $link) { + // even if conlict ends up being with an alias, there would be a conflict with at least one actual package by this name if (!isset($this->addedPackagesByNames[$link->getTarget()])) { continue; } @@ -207,11 +208,10 @@ class RuleSetGenerator continue; } - /** @var PackageInterface $possibleConflict */ - foreach ($this->addedPackagesByNames[$link->getTarget()] as $possibleConflict) { - if ($this->pool->match($possibleConflict, $link->getTarget(), $link->getConstraint())) { - $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $possibleConflict, Rule::RULE_PACKAGE_CONFLICT, $link)); - } + $conflicts = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); + + foreach ($conflicts as $conflict) { + $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $conflict, Rule::RULE_PACKAGE_CONFLICT, $link)); } } } From 780e6fc02767708769efc5d36e1db6b23a289d72 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 13 Nov 2020 16:05:20 +0100 Subject: [PATCH 3/9] Avoid creating unnecessary conflict rules for provide/replace of aliases --- src/Composer/DependencyResolver/RuleSetGenerator.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index 19fcc8c12..e4591f451 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -211,7 +211,12 @@ class RuleSetGenerator $conflicts = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); foreach ($conflicts as $conflict) { - $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $conflict, Rule::RULE_PACKAGE_CONFLICT, $link)); + // define the conflict rule for regular packages, for alias packages it's only needed if the name + // matches the conflict exactly, otherwise the name match is by provide/replace which means the + // package which this is an alias of will conflict anyway, so no need to create additional rules + if (!$conflict instanceof AliasPackage || $conflict->getName() === $link->getTarget()) { + $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $conflict, Rule::RULE_PACKAGE_CONFLICT, $link)); + } } } } From c02d2842b0fc43da355d9176e02e309336a5836f Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 13 Nov 2020 16:55:35 +0100 Subject: [PATCH 4/9] Test conflict with an unrequired alias does not prevent install. The alias still ends up being marked as installed as the install step reads it from the branch alias in the lock file and doesn't know a conflict required it to be skipped. --- ...-prevent-installation-if-not-required.test | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-installation-if-not-required.test diff --git a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-installation-if-not-required.test b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-installation-if-not-required.test new file mode 100644 index 000000000..542a883f2 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-installation-if-not-required.test @@ -0,0 +1,32 @@ +--TEST-- +Test that conflict on a branch alias is ignored if the alias is not required for installation. + +The alias ends up being marked installed anyway because the install step doesn't know that the branch alias which is +present in the lock file was ignored during resolution. This is acceptable as deps were resolvable without the alias +and we don't want to modify branch alias meta data in the lock file. +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "some/dep", "version": "1.0.0" }, + { "name": "some/dep", "version": "1.1.0" }, + { "name": "some/dep", "version": "1.2.0" }, + { "name": "some/dep", "version": "dev-main", "extra": {"branch-alias": {"dev-main": "1.3.x-dev"} } }, + { "name": "some/dep", "version": "1.2.x-dev" } + ] + } + ], + "require": { + "some/dep": "dev-main" + }, + "conflict": { + "some/dep": ">=1.3" + } +} +--RUN-- +update +--EXPECT-- +Installing some/dep (dev-main) +Marking some/dep (1.3.x-dev) as installed, alias of some/dep (dev-main)' From d19b858e204797a06e1021980cc09b85724df59b Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 13 Nov 2020 16:58:43 +0100 Subject: [PATCH 5/9] Improve decisions debug output --- src/Composer/DependencyResolver/Decisions.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Composer/DependencyResolver/Decisions.php b/src/Composer/DependencyResolver/Decisions.php index bbf774ba9..c37dfec53 100644 --- a/src/Composer/DependencyResolver/Decisions.php +++ b/src/Composer/DependencyResolver/Decisions.php @@ -197,15 +197,20 @@ class Decisions implements \Iterator, \Countable } } - public function __toString() + public function toString(Pool $pool = null) { $decisionMap = $this->decisionMap; ksort($decisionMap); $str = '['; foreach ($decisionMap as $packageId => $level) { - $str .= $packageId.':'.$level.','; + $str .= (($pool) ? $pool->literalToPackage($packageId) : $packageId).':'.$level.','; } $str .= ']'; return $str; } + + public function __toString() + { + return $this->toString(); + } } From 1b337be2362a98c01df924c8b68ef61fc4c800f8 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 16 Nov 2020 13:27:54 +0100 Subject: [PATCH 6/9] Add expected lock files to conflict with branch alias tests, add install from lock test --- ...s-not-prevent-update-if-not-required.test} | 18 +++++++- ...es-not-prevent-update-if-not-required.test | 45 +++++++++++++++++++ ...s-not-prevent-install-if-not-required.test | 45 +++++++++++++++++++ ... conflict-with-alias-prevents-update.test} | 16 +++++++ 4 files changed, 123 insertions(+), 1 deletion(-) rename tests/Composer/Test/Fixtures/installer/{conflict-with-alias-does-not-prevent-installation-if-not-required.test => conflict-on-root-with-alias-does-not-prevent-update-if-not-required.test} (73%) create mode 100644 tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-update-if-not-required.test create mode 100644 tests/Composer/Test/Fixtures/installer/conflict-with-alias-in-lock-does-not-prevent-install-if-not-required.test rename tests/Composer/Test/Fixtures/installer/{conflict-with-alias-prevents-installation.test => conflict-with-alias-prevents-update.test} (67%) diff --git a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-installation-if-not-required.test b/tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-does-not-prevent-update-if-not-required.test similarity index 73% rename from tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-installation-if-not-required.test rename to tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-does-not-prevent-update-if-not-required.test index 542a883f2..454384206 100644 --- a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-installation-if-not-required.test +++ b/tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-does-not-prevent-update-if-not-required.test @@ -27,6 +27,22 @@ and we don't want to modify branch alias meta data in the lock file. } --RUN-- update +--EXPECT-LOCK-- +{ + "packages": [ + { "name": "some/dep", "version": "dev-main", "type": "library", "extra": {"branch-alias": {"dev-main": "1.3.x-dev"} } } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": { + "some/dep": 20 + }, + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} --EXPECT-- Installing some/dep (dev-main) -Marking some/dep (1.3.x-dev) as installed, alias of some/dep (dev-main)' +Marking some/dep (1.3.x-dev) as installed, alias of some/dep (dev-main) diff --git a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-update-if-not-required.test b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-update-if-not-required.test new file mode 100644 index 000000000..e03cd0f2a --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-update-if-not-required.test @@ -0,0 +1,45 @@ +--TEST-- +Test that conflict on a branch alias is ignored if the alias is not required for installation. +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "some/dep", "version": "1.0.0" }, + { "name": "some/dep", "version": "1.1.0" }, + { "name": "some/dep", "version": "1.2.0" }, + { "name": "some/dep", "version": "dev-main", "extra": {"branch-alias": {"dev-main": "1.3.x-dev"} } }, + { "name": "some/dep", "version": "1.2.x-dev" }, + { "name": "conflictor/foo", "version": "1.0.0", "conflict": { "some/dep": ">=1.3" } } + ] + } + ], + "require": { + "some/dep": "dev-main", + "conflictor/foo": "1.0.0" + } +} +--RUN-- +update +--EXPECT-LOCK-- +{ + "packages": [ + { "name": "conflictor/foo", "version": "1.0.0", "conflict": { "some/dep": ">=1.3" }, "type": "library" }, + { "name": "some/dep", "version": "dev-main", "extra": {"branch-alias": {"dev-main": "1.3.x-dev"} }, "type": "library" } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": { + "some/dep": 20 + }, + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} +--EXPECT-- +Installing conflictor/foo (1.0.0) +Installing some/dep (dev-main) +Marking some/dep (1.3.x-dev) as installed, alias of some/dep (dev-main) diff --git a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-in-lock-does-not-prevent-install-if-not-required.test b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-in-lock-does-not-prevent-install-if-not-required.test new file mode 100644 index 000000000..c6f91e115 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-in-lock-does-not-prevent-install-if-not-required.test @@ -0,0 +1,45 @@ +--TEST-- +Test that conflict on a branch alias is ignored if the alias is not required for installation. +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "some/dep", "version": "1.0.0" }, + { "name": "some/dep", "version": "1.1.0" }, + { "name": "some/dep", "version": "1.2.0" }, + { "name": "some/dep", "version": "dev-main", "extra": {"branch-alias": {"dev-main": "1.3.x-dev"} } }, + { "name": "some/dep", "version": "1.2.x-dev" }, + { "name": "conflictor/foo", "version": "1.0.0", "conflict": { "some/dep": ">=1.3" } } + ] + } + ], + "require": { + "some/dep": "dev-main", + "conflictor/foo": "1.0.0" + } +} +--LOCK-- +{ + "packages": [ + { "name": "conflictor/foo", "version": "1.0.0", "conflict": { "some/dep": ">=1.3" }, "type": "library" }, + { "name": "some/dep", "version": "dev-main", "extra": {"branch-alias": {"dev-main": "1.3.x-dev"} }, "type": "library" } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": { + "some/dep": 20 + }, + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} +--RUN-- +install +--EXPECT-- +Installing conflictor/foo (1.0.0) +Installing some/dep (dev-main) +Marking some/dep (1.3.x-dev) as installed, alias of some/dep (dev-main) diff --git a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-installation.test b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-update.test similarity index 67% rename from tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-installation.test rename to tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-update.test index b4425a6eb..74b61304e 100644 --- a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-installation.test +++ b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-update.test @@ -23,5 +23,21 @@ Test that conflict on a branch alias is respected } --RUN-- update +--EXPECT-LOCK-- +{ + "packages": [ + { "name": "some/dep", "version": "1.2.x-dev", "type": "library" } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": { + "some/dep": 20 + }, + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} --EXPECT-- Installing some/dep (1.2.x-dev) From 58f358d028f40cde8ad2a6a6e742194ffe1eea0a Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 26 Nov 2020 11:21:53 +0100 Subject: [PATCH 7/9] Correct test case descriptions --- ...-with-alias-does-not-prevent-update-if-not-required.test | 6 +----- ...-with-alias-does-not-prevent-update-if-not-required.test | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-does-not-prevent-update-if-not-required.test b/tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-does-not-prevent-update-if-not-required.test index 454384206..df81a3a81 100644 --- a/tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-does-not-prevent-update-if-not-required.test +++ b/tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-does-not-prevent-update-if-not-required.test @@ -1,9 +1,5 @@ --TEST-- -Test that conflict on a branch alias is ignored if the alias is not required for installation. - -The alias ends up being marked installed anyway because the install step doesn't know that the branch alias which is -present in the lock file was ignored during resolution. This is acceptable as deps were resolvable without the alias -and we don't want to modify branch alias meta data in the lock file. +Test that root package conflict on a branch alias is ignored if the alias is not required for installation. --COMPOSER-- { "repositories": [ diff --git a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-update-if-not-required.test b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-update-if-not-required.test index e03cd0f2a..a7b47f119 100644 --- a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-update-if-not-required.test +++ b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-update-if-not-required.test @@ -1,5 +1,5 @@ --TEST-- -Test that conflict on a branch alias is ignored if the alias is not required for installation. +Test that conflict of a dependency with a branch alias of another dependency is ignored if the alias is not required for installation. --COMPOSER-- { "repositories": [ From 7197278fe96fe65177062d9d964b2e1df5e008ec Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 26 Nov 2020 12:10:07 +0100 Subject: [PATCH 8/9] Always install aliases together with their original package Restores some Composer 1.x behavior like unbound constraints conflicting with default branches unless they are branch aliased. Simplifies conflicts with aliases because packages cannot be installed without their aliases, so we do not need to know which aliases are uninstalled in lock file or installed.json. --- src/Composer/DependencyResolver/Rule.php | 26 ++++++----- .../DependencyResolver/RuleSetGenerator.php | 18 ++++---- .../Test/DependencyResolver/SolverTest.php | 1 + ...es-not-prevent-update-if-not-required.test | 44 ------------------- ...alias-prevents-update-if-not-required.test | 38 ++++++++++++++++ ...-alias-in-lock-does-prevents-install.test} | 19 +++++--- ...lias-prevents-update-if-not-required.test} | 35 ++++++--------- ...tch-default-branch-with-branch-alias.test} | 6 +-- ...unded-conflict-matches-default-branch.test | 39 ++++++++++++++++ 9 files changed, 134 insertions(+), 92 deletions(-) delete mode 100644 tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-does-not-prevent-update-if-not-required.test create mode 100644 tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-prevents-update-if-not-required.test rename tests/Composer/Test/Fixtures/installer/{conflict-with-alias-in-lock-does-not-prevent-install-if-not-required.test => conflict-with-alias-in-lock-does-prevents-install.test} (60%) rename tests/Composer/Test/Fixtures/installer/{conflict-with-alias-does-not-prevent-update-if-not-required.test => conflict-with-alias-prevents-update-if-not-required.test} (52%) rename tests/Composer/Test/Fixtures/installer/{unbounded-conflict-does-not-match-dev-master.test => unbounded-conflict-does-not-match-default-branch-with-branch-alias.test} (71%) create mode 100644 tests/Composer/Test/Fixtures/installer/unbounded-conflict-matches-default-branch.test diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index 7e1311a76..4ebd08810 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -35,7 +35,7 @@ abstract class Rule const RULE_PACKAGE_SAME_NAME = 10; const RULE_LEARNED = 12; const RULE_PACKAGE_ALIAS = 13; - const RULE_PACKAGE_ROOT_ALIAS = 14; + const RULE_PACKAGE_INVERSE_ALIAS = 14; // bitfield defs const BITFIELD_TYPE = 0; @@ -313,22 +313,26 @@ abstract class Rule return 'Conclusion: '.$ruleText.$learnedString; case self::RULE_PACKAGE_ALIAS: - case self::RULE_PACKAGE_ROOT_ALIAS: - if ($this->getReason() === self::RULE_PACKAGE_ALIAS) { - $aliasPackage = $pool->literalToPackage($literals[0]); - $otherLiteral = 1; - } else { - // root alias rules work the other way around - $aliasPackage = $pool->literalToPackage($literals[1]); - $otherLiteral = 0; - } + $aliasPackage = $pool->literalToPackage($literals[0]); + // avoid returning content like "9999999-dev is an alias of dev-master" as it is useless if ($aliasPackage->getVersion() === VersionParser::DEFAULT_BRANCH_ALIAS) { return ''; } - $package = $this->deduplicateDefaultBranchAlias($pool->literalToPackage($literals[$otherLiteral])); + $package = $this->deduplicateDefaultBranchAlias($pool->literalToPackage($literals[1])); return $aliasPackage->getPrettyString() .' is an alias of '.$package->getPrettyString().' and thus requires it to be installed too.'; + case self::RULE_PACKAGE_INVERSE_ALIAS: + // inverse alias rules work the other way around than above + $aliasPackage = $pool->literalToPackage($literals[1]); + + // avoid returning content like "9999999-dev is an alias of dev-master" as it is useless + if ($aliasPackage->getVersion() === VersionParser::DEFAULT_BRANCH_ALIAS) { + return ''; + } + $package = $this->deduplicateDefaultBranchAlias($pool->literalToPackage($literals[0])); + + return $aliasPackage->getPrettyString() .' is an alias of '.$package->getPrettyString().' and must be installed with it.'; default: $ruleText = ''; foreach ($literals as $i => $literal) { diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index e4591f451..dc7de29e0 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -166,10 +166,8 @@ class RuleSetGenerator $workQueue->enqueue($package->getAliasOf()); $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRequireRule($package, array($package->getAliasOf()), Rule::RULE_PACKAGE_ALIAS, $package)); - // root aliases must be installed with their main package, so create a rule the other way around as well - if ($package->isRootPackageAlias()) { - $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRequireRule($package->getAliasOf(), array($package), Rule::RULE_PACKAGE_ROOT_ALIAS, $package->getAliasOf())); - } + // aliases must be installed with their main package, so create a rule the other way around as well + $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRequireRule($package->getAliasOf(), array($package), Rule::RULE_PACKAGE_INVERSE_ALIAS, $package->getAliasOf())); // if alias package has no self.version requires, its requirements do not // need to be added as the aliased package processing will take care of it @@ -199,7 +197,7 @@ class RuleSetGenerator /** @var PackageInterface $package */ foreach ($this->addedMap as $package) { foreach ($package->getConflicts() as $link) { - // even if conlict ends up being with an alias, there would be a conflict with at least one actual package by this name + // even if conlict ends up being with an alias, there would be at least one actual package by this name if (!isset($this->addedPackagesByNames[$link->getTarget()])) { continue; } @@ -273,9 +271,13 @@ class RuleSetGenerator protected function addRulesForRootAliases($ignorePlatformReqs) { foreach ($this->pool->getPackages() as $package) { - // ensure that rules for root alias packages get loaded even if the root alias itself isn't required - // otherwise a package could be installed without its root alias which leads to unexpected behavior - if ($package instanceof AliasPackage && $package->isRootPackageAlias()) { + // ensure that rules for root alias packages and aliases of packages which were loaded are also loaded + // even if the alias itself isn't required, otherwise a package could be installed without its alias which + // leads to unexpected behavior + if (!isset($this->addedMap[$package->id]) && + $package instanceof AliasPackage && + ($package->isRootPackageAlias() || isset($this->addedMap[$package->getAliasOf()->id])) + ) { $this->addRulesForPackage($package, $ignorePlatformReqs); } } diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index 303e295cb..c756ace1d 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/tests/Composer/Test/DependencyResolver/SolverTest.php @@ -842,6 +842,7 @@ class SolverTest extends TestCase array('job' => 'install', 'package' => $packageB), array('job' => 'markAliasInstalled', 'package' => $packageBAlias), array('job' => 'install', 'package' => $packageC), + array('job' => 'markAliasInstalled', 'package' => $packageCAlias), )); } diff --git a/tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-does-not-prevent-update-if-not-required.test b/tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-does-not-prevent-update-if-not-required.test deleted file mode 100644 index df81a3a81..000000000 --- a/tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-does-not-prevent-update-if-not-required.test +++ /dev/null @@ -1,44 +0,0 @@ ---TEST-- -Test that root package conflict on a branch alias is ignored if the alias is not required for installation. ---COMPOSER-- -{ - "repositories": [ - { - "type": "package", - "package": [ - { "name": "some/dep", "version": "1.0.0" }, - { "name": "some/dep", "version": "1.1.0" }, - { "name": "some/dep", "version": "1.2.0" }, - { "name": "some/dep", "version": "dev-main", "extra": {"branch-alias": {"dev-main": "1.3.x-dev"} } }, - { "name": "some/dep", "version": "1.2.x-dev" } - ] - } - ], - "require": { - "some/dep": "dev-main" - }, - "conflict": { - "some/dep": ">=1.3" - } -} ---RUN-- -update ---EXPECT-LOCK-- -{ - "packages": [ - { "name": "some/dep", "version": "dev-main", "type": "library", "extra": {"branch-alias": {"dev-main": "1.3.x-dev"} } } - ], - "packages-dev": [], - "aliases": [], - "minimum-stability": "stable", - "stability-flags": { - "some/dep": 20 - }, - "prefer-stable": false, - "prefer-lowest": false, - "platform": [], - "platform-dev": [] -} ---EXPECT-- -Installing some/dep (dev-main) -Marking some/dep (1.3.x-dev) as installed, alias of some/dep (dev-main) diff --git a/tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-prevents-update-if-not-required.test b/tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-prevents-update-if-not-required.test new file mode 100644 index 000000000..3a457b22c --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/conflict-on-root-with-alias-prevents-update-if-not-required.test @@ -0,0 +1,38 @@ +--TEST-- +Test that a root package conflict with a branch alias leads to an error, even if the branch alias isn't required. +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "some/dep", "version": "1.0.0" }, + { "name": "some/dep", "version": "1.1.0" }, + { "name": "some/dep", "version": "1.2.0" }, + { "name": "some/dep", "version": "dev-main", "extra": {"branch-alias": {"dev-main": "1.3.x-dev"} } }, + { "name": "some/dep", "version": "1.2.x-dev" } + ] + } + ], + "require": { + "some/dep": "dev-main" + }, + "conflict": { + "some/dep": ">=1.3" + } +} +--RUN-- +update +--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__ is present at version 1.0.0+no-version-set and cannot be modified by Composer + - some/dep 1.3.x-dev is an alias of some/dep dev-main and must be installed with it. + - __root__ 1.0.0+no-version-set conflicts with some/dep 1.3.x-dev. + - Root composer.json requires some/dep dev-main -> satisfiable by some/dep[dev-main]. +--EXPECT-- diff --git a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-in-lock-does-not-prevent-install-if-not-required.test b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-in-lock-does-prevents-install.test similarity index 60% rename from tests/Composer/Test/Fixtures/installer/conflict-with-alias-in-lock-does-not-prevent-install-if-not-required.test rename to tests/Composer/Test/Fixtures/installer/conflict-with-alias-in-lock-does-prevents-install.test index c6f91e115..c19ca396c 100644 --- a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-in-lock-does-not-prevent-install-if-not-required.test +++ b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-in-lock-does-prevents-install.test @@ -1,5 +1,5 @@ --TEST-- -Test that conflict on a branch alias is ignored if the alias is not required for installation. +Test that conflict with a branch alias in the lock file leads to an error on install from lock, even if the branch alias was removed on the remote end. --COMPOSER-- { "repositories": [ @@ -9,7 +9,7 @@ Test that conflict on a branch alias is ignored if the alias is not required for { "name": "some/dep", "version": "1.0.0" }, { "name": "some/dep", "version": "1.1.0" }, { "name": "some/dep", "version": "1.2.0" }, - { "name": "some/dep", "version": "dev-main", "extra": {"branch-alias": {"dev-main": "1.3.x-dev"} } }, + { "name": "some/dep", "version": "dev-main" }, { "name": "some/dep", "version": "1.2.x-dev" }, { "name": "conflictor/foo", "version": "1.0.0", "conflict": { "some/dep": ">=1.3" } } ] @@ -39,7 +39,16 @@ Test that conflict on a branch alias is ignored if the alias is not required for } --RUN-- install +--EXPECT-EXIT-CODE-- +2 +--EXPECT-OUTPUT-- +Installing dependencies from lock file (including require-dev) +Verifying lock file contents can be installed on current platform. +Your lock file does not contain a compatible set of packages. Please run composer update. + + Problem 1 + - conflictor/foo is locked to version 1.0.0 and an update of this package was not requested. + - conflictor/foo 1.0.0 conflicts with some/dep 1.3.x-dev. + - some/dep is locked to version 1.3.x-dev and an update of this package was not requested. + --EXPECT-- -Installing conflictor/foo (1.0.0) -Installing some/dep (dev-main) -Marking some/dep (1.3.x-dev) as installed, alias of some/dep (dev-main) diff --git a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-update-if-not-required.test b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-update-if-not-required.test similarity index 52% rename from tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-update-if-not-required.test rename to tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-update-if-not-required.test index a7b47f119..78cfd0362 100644 --- a/tests/Composer/Test/Fixtures/installer/conflict-with-alias-does-not-prevent-update-if-not-required.test +++ b/tests/Composer/Test/Fixtures/installer/conflict-with-alias-prevents-update-if-not-required.test @@ -1,5 +1,5 @@ --TEST-- -Test that conflict of a dependency with a branch alias of another dependency is ignored if the alias is not required for installation. +Test that conflict of a dependency with a branch alias of another dependency is not ignored, even if the alias is not required for installation. --COMPOSER-- { "repositories": [ @@ -22,24 +22,17 @@ Test that conflict of a dependency with a branch alias of another dependency is } --RUN-- update ---EXPECT-LOCK-- -{ - "packages": [ - { "name": "conflictor/foo", "version": "1.0.0", "conflict": { "some/dep": ">=1.3" }, "type": "library" }, - { "name": "some/dep", "version": "dev-main", "extra": {"branch-alias": {"dev-main": "1.3.x-dev"} }, "type": "library" } - ], - "packages-dev": [], - "aliases": [], - "minimum-stability": "stable", - "stability-flags": { - "some/dep": 20 - }, - "prefer-stable": false, - "prefer-lowest": false, - "platform": [], - "platform-dev": [] -} +--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 some/dep dev-main -> satisfiable by some/dep[dev-main]. + - conflictor/foo 1.0.0 conflicts with some/dep 1.3.x-dev. + - some/dep 1.3.x-dev is an alias of some/dep dev-main and must be installed with it. + - Root composer.json requires conflictor/foo 1.0.0 -> satisfiable by conflictor/foo[1.0.0]. + --EXPECT-- -Installing conflictor/foo (1.0.0) -Installing some/dep (dev-main) -Marking some/dep (1.3.x-dev) as installed, alias of some/dep (dev-main) diff --git a/tests/Composer/Test/Fixtures/installer/unbounded-conflict-does-not-match-dev-master.test b/tests/Composer/Test/Fixtures/installer/unbounded-conflict-does-not-match-default-branch-with-branch-alias.test similarity index 71% rename from tests/Composer/Test/Fixtures/installer/unbounded-conflict-does-not-match-dev-master.test rename to tests/Composer/Test/Fixtures/installer/unbounded-conflict-does-not-match-default-branch-with-branch-alias.test index 900028457..ca891bbc1 100644 --- a/tests/Composer/Test/Fixtures/installer/unbounded-conflict-does-not-match-dev-master.test +++ b/tests/Composer/Test/Fixtures/installer/unbounded-conflict-does-not-match-default-branch-with-branch-alias.test @@ -1,5 +1,5 @@ --TEST-- -Test that a conflict against >=5 does not include dev-master or other dev-x +Test that a conflict against >=5 does not include the default branch if it has a branch alias defined. --COMPOSER-- { "repositories": [ @@ -7,7 +7,7 @@ Test that a conflict against >=5 does not include dev-master or other dev-x "type": "package", "package": [ { "name": "conflicter/pkg", "version": "1.0.0", "conflict": { "victim/pkg": ">=5", "victim/pkg2": ">=5" } }, - { "name": "victim/pkg", "version": "dev-master", "default-branch": true }, + { "name": "victim/pkg", "version": "dev-master", "default-branch": true, "extra": { "branch-alias": { "dev-master": "2.x-dev" } } }, { "name": "victim/pkg2", "version": "dev-foo" } ] } @@ -27,5 +27,5 @@ update --EXPECT-- Installing conflicter/pkg (1.0.0) Installing victim/pkg (dev-master) -Marking victim/pkg (9999999-dev) as installed, alias of victim/pkg (dev-master) +Marking victim/pkg (2.x-dev) as installed, alias of victim/pkg (dev-master) Installing victim/pkg2 (dev-foo) diff --git a/tests/Composer/Test/Fixtures/installer/unbounded-conflict-matches-default-branch.test b/tests/Composer/Test/Fixtures/installer/unbounded-conflict-matches-default-branch.test new file mode 100644 index 000000000..b2daec530 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/unbounded-conflict-matches-default-branch.test @@ -0,0 +1,39 @@ +--TEST-- +Test that a conflict against >=5 includes the default branch if it has a branch alias defined. +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "conflicter/pkg", "version": "1.0.0", "conflict": { "victim/pkg": ">=5", "victim/pkg2": ">=5" } }, + { "name": "victim/pkg", "version": "dev-master", "default-branch": true }, + { "name": "victim/pkg2", "version": "dev-foo" } + ] + } + ], + "require": { + "conflicter/pkg": "1.0.0", + "victim/pkg": "*", + "victim/pkg2": "*" + }, + "minimum-stability": "dev" +} + + +--RUN-- +update + +--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 + - conflicter/pkg 1.0.0 conflicts with victim/pkg dev-master. + - Root composer.json requires conflicter/pkg 1.0.0 -> satisfiable by conflicter/pkg[1.0.0]. + - Root composer.json requires victim/pkg * -> satisfiable by victim/pkg[dev-master]. + +--EXPECT-- From e203809106ad2c2205840465005b3a0e6fb09798 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 26 Nov 2020 13:59:35 +0100 Subject: [PATCH 9/9] Fix test description --- .../installer/unbounded-conflict-matches-default-branch.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Composer/Test/Fixtures/installer/unbounded-conflict-matches-default-branch.test b/tests/Composer/Test/Fixtures/installer/unbounded-conflict-matches-default-branch.test index b2daec530..d4a3b4d85 100644 --- a/tests/Composer/Test/Fixtures/installer/unbounded-conflict-matches-default-branch.test +++ b/tests/Composer/Test/Fixtures/installer/unbounded-conflict-matches-default-branch.test @@ -1,5 +1,5 @@ --TEST-- -Test that a conflict against >=5 includes the default branch if it has a branch alias defined. +Test that a conflict against >=5 includes the default branch if it has no branch alias defined (and then uses the default 9999999-dev alias). --COMPOSER-- { "repositories": [