From 7197278fe96fe65177062d9d964b2e1df5e008ec Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 26 Nov 2020 12:10:07 +0100 Subject: [PATCH] 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--