From af7feade8f3ae3db1865d00130f1205f54f25340 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Thu, 23 Apr 2020 13:28:13 +0200 Subject: [PATCH 01/54] POC --- .../DependencyResolver/PoolBuilder.php | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 29903f493..a57383d3c 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -124,12 +124,12 @@ class PoolBuilder $loadNames = array(); foreach ($request->getFixedPackages() as $package) { $this->nameConstraints[$package->getName()] = null; - $this->loadedNames[$package->getName()] = true; + $this->loadedNames[$package->getName()] = new Constraint('==', $package->getVersion()); // replace means conflict, so if a fixed package replaces a name, no need to load that one, packages would conflict anyways foreach ($package->getReplaces() as $link) { $this->nameConstraints[$package->getName()] = null; - $this->loadedNames[$link->getTarget()] = true; + $this->loadedNames[$link->getTarget()] = $link->getConstraint(); } // TODO in how far can we do the above for conflicts? It's more tricky cause conflicts can be limited to @@ -164,8 +164,8 @@ class PoolBuilder } while (!empty($loadNames)) { - foreach ($loadNames as $name => $void) { - $this->loadedNames[$name] = true; + foreach ($loadNames as $name => $constraint) { + $this->loadedNames[$name] = $constraint; } $newLoadNames = array(); @@ -290,8 +290,10 @@ class PoolBuilder $loadNames = array(); foreach ($package->getRequires() as $link) { $require = $link->getTarget(); + $linkConstraint = $link->getConstraint(); + if (!isset($this->loadedNames[$require])) { - $loadNames[$require] = null; + $loadNames[$require] = $linkConstraint; // if this is a partial update with transitive dependencies we need to unfix the package we now know is a // dependency of another package which we are trying to update, and then attempt to load it again } elseif ($propagateUpdate && $request->getUpdateAllowTransitiveDependencies() && isset($this->skippedLoad[$require])) { @@ -302,9 +304,17 @@ class PoolBuilder $this->updateAllowWarned[$require] = true; $this->io->writeError('Dependency "'.$require.'" is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies to include root dependencies.'); } + } else { + if (null === $this->loadedNames[$require] || null === $linkConstraint) { + unset($this->loadedNames[$require]); + } + + if (!$this->loadedNames[$require]->matches($linkConstraint)) { + $loadNames[$require] = MultiConstraint::create(array($this->loadedNames[$require], $linkConstraint), false); + unset($this->loadedNames[$require]); + } } - $linkConstraint = $link->getConstraint(); if ($linkConstraint && !($linkConstraint instanceof MatchAllConstraint)) { if (!\array_key_exists($require, $this->nameConstraints)) { $this->nameConstraints[$require] = array($linkConstraint); From 912aecb6661b068cf187fb6fd0359798838d86ad Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Fri, 24 Apr 2020 17:42:03 +0200 Subject: [PATCH 02/54] Removed name constraints --- .../DependencyResolver/PoolBuilder.php | 34 +++---------------- 1 file changed, 5 insertions(+), 29 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index a57383d3c..727f086d8 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -62,11 +62,12 @@ class PoolBuilder * @psalm-var array */ private $aliasMap = array(); + /** - * @psalm-var array + * @psalm-var array */ - private $nameConstraints = array(); private $loadedNames = array(); + /** * @psalm-var Package[] */ @@ -123,12 +124,10 @@ class PoolBuilder $loadNames = array(); foreach ($request->getFixedPackages() as $package) { - $this->nameConstraints[$package->getName()] = null; $this->loadedNames[$package->getName()] = new Constraint('==', $package->getVersion()); // replace means conflict, so if a fixed package replaces a name, no need to load that one, packages would conflict anyways foreach ($package->getReplaces() as $link) { - $this->nameConstraints[$package->getName()] = null; $this->loadedNames[$link->getTarget()] = $link->getConstraint(); } @@ -153,7 +152,6 @@ class PoolBuilder } $loadNames[$packageName] = $constraint; - $this->nameConstraints[$packageName] = $constraint && !($constraint instanceof MatchAllConstraint) ? array($constraint) : null; } // clean up loadNames for anything we manually marked loaded above @@ -189,19 +187,11 @@ class PoolBuilder $loadNames = $newLoadNames; } - // filter packages according to all the require statements collected for each package - $nameConstraints = array(); - foreach ($this->nameConstraints as $name => $constraints) { - if (\is_array($constraints)) { - $nameConstraints[$name] = MultiConstraint::create(array_values(array_unique($constraints)), false); - } - } foreach ($this->packages as $i => $package) { // we check all alias related packages at once, so no need to check individual aliases // isset also checks non-null value - if (!$package instanceof AliasPackage && isset($nameConstraints[$package->getName()])) { - $constraint = $nameConstraints[$package->getName()]; - + if (!$package instanceof AliasPackage) { + $constraint = new Constraint('==', $package->getVersion()); $aliasedPackages = array($i => $package); if (isset($this->aliasMap[spl_object_hash($package)])) { $aliasedPackages += $this->aliasMap[spl_object_hash($package)]; @@ -241,7 +231,6 @@ class PoolBuilder $pool = new Pool($this->packages, $this->unacceptableFixedPackages); $this->aliasMap = array(); - $this->nameConstraints = array(); $this->loadedNames = array(); $this->packages = array(); $this->unacceptableFixedPackages = array(); @@ -314,17 +303,6 @@ class PoolBuilder unset($this->loadedNames[$require]); } } - - if ($linkConstraint && !($linkConstraint instanceof MatchAllConstraint)) { - if (!\array_key_exists($require, $this->nameConstraints)) { - $this->nameConstraints[$require] = array($linkConstraint); - } elseif (\is_array($this->nameConstraints[$require])) { - $this->nameConstraints[$require][] = $linkConstraint; - } - // else it is null and should stay null - } else { - $this->nameConstraints[$require] = null; - } } // if we're doing a partial update with deps we also need to unfix packages which are being replaced in case they @@ -336,8 +314,6 @@ class PoolBuilder if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$replace])) { $this->unfixPackage($request, $replace); $loadNames[$replace] = null; - // TODO should we try to merge constraints here? - $this->nameConstraints[$replace] = null; } elseif (!$request->getUpdateAllowTransitiveRootDependencies() && $this->isRootRequire($request, $replace) && !isset($this->updateAllowWarned[$replace])) { $this->updateAllowWarned[$replace] = true; $this->io->writeError('Dependency "'.$replace.'" is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies to include root dependencies.'); From 10555ecff358e854596e21c21635bec877f4936d Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Fri, 24 Apr 2020 17:42:24 +0200 Subject: [PATCH 03/54] Make sure there's always a constraint --- src/Composer/DependencyResolver/PoolBuilder.php | 6 +----- src/Composer/DependencyResolver/Request.php | 3 ++- src/Composer/Package/Link.php | 5 +++-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 727f086d8..575e89ce6 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -294,10 +294,6 @@ class PoolBuilder $this->io->writeError('Dependency "'.$require.'" is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies to include root dependencies.'); } } else { - if (null === $this->loadedNames[$require] || null === $linkConstraint) { - unset($this->loadedNames[$require]); - } - if (!$this->loadedNames[$require]->matches($linkConstraint)) { $loadNames[$require] = MultiConstraint::create(array($this->loadedNames[$require], $linkConstraint), false); unset($this->loadedNames[$require]); @@ -313,7 +309,7 @@ class PoolBuilder if (isset($this->loadedNames[$replace]) && isset($this->skippedLoad[$replace])) { if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$replace])) { $this->unfixPackage($request, $replace); - $loadNames[$replace] = null; + $loadNames[$replace] = $link->getConstraint(); } elseif (!$request->getUpdateAllowTransitiveRootDependencies() && $this->isRootRequire($request, $replace) && !isset($this->updateAllowWarned[$replace])) { $this->updateAllowWarned[$replace] = true; $this->io->writeError('Dependency "'.$replace.'" is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies to include root dependencies.'); diff --git a/src/Composer/DependencyResolver/Request.php b/src/Composer/DependencyResolver/Request.php index 5782c3ff1..959ba884b 100644 --- a/src/Composer/DependencyResolver/Request.php +++ b/src/Composer/DependencyResolver/Request.php @@ -17,6 +17,7 @@ use Composer\Package\PackageInterface; use Composer\Package\RootAliasPackage; use Composer\Repository\LockArrayRepository; use Composer\Semver\Constraint\ConstraintInterface; +use Composer\Semver\Constraint\EmptyConstraint; /** * @author Nils Adermann @@ -55,7 +56,7 @@ class Request public function requireName($packageName, ConstraintInterface $constraint = null) { $packageName = strtolower($packageName); - $this->requires[$packageName] = $constraint; + $this->requires[$packageName] = $constraint ? $constraint : new EmptyConstraint(); } /** diff --git a/src/Composer/Package/Link.php b/src/Composer/Package/Link.php index 5a6c683cc..03ea3edff 100644 --- a/src/Composer/Package/Link.php +++ b/src/Composer/Package/Link.php @@ -13,6 +13,7 @@ namespace Composer\Package; use Composer\Semver\Constraint\ConstraintInterface; +use Composer\Semver\Constraint\EmptyConstraint; /** * Represents a link between two packages, represented by their names @@ -59,7 +60,7 @@ class Link { $this->source = strtolower($source); $this->target = strtolower($target); - $this->constraint = $constraint; + $this->constraint = $constraint ? $constraint : new EmptyConstraint(); $this->description = $description; $this->prettyConstraint = $prettyConstraint; } @@ -89,7 +90,7 @@ class Link } /** - * @return ConstraintInterface|null + * @return ConstraintInterface */ public function getConstraint() { From 1c8865a5d19cae03bf04a4a08a3a238dce60525f Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Tue, 28 Apr 2020 16:21:48 +0200 Subject: [PATCH 04/54] Comment --- src/Composer/DependencyResolver/PoolBuilder.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 575e89ce6..260775cd1 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -294,6 +294,9 @@ class PoolBuilder $this->io->writeError('Dependency "'.$require.'" is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies to include root dependencies.'); } } else { + // Check if packages we already loaded match the constraint and if they don't + // extend the constraint and mark that package as not being loaded yet + // so we get the required package versions if (!$this->loadedNames[$require]->matches($linkConstraint)) { $loadNames[$require] = MultiConstraint::create(array($this->loadedNames[$require], $linkConstraint), false); unset($this->loadedNames[$require]); From 208a73564586f80c7c9882018dbaa8907ec3aa70 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Wed, 29 Apr 2020 22:54:33 +0200 Subject: [PATCH 05/54] PoolBuilder Failing test: fixed package is not only pkg loaded for that name --- .../fixed-packages-do-not-load-from-repos.test | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-do-not-load-from-repos.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-do-not-load-from-repos.test index f1dbd69fe..0abc4a67e 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-do-not-load-from-repos.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-do-not-load-from-repos.test @@ -3,21 +3,30 @@ Fixed packages do not get loaded from the repos --REQUEST-- { - "some/pkg": "*" + "some/pkg": "*", + "root/req": "*" } --FIXED-- [ - {"name": "some/pkg", "version": "1.0.3", "id": 1} + {"name": "some/pkg", "version": "1.0.3", "id": 1}, + {"name": "dep/dep", "version": "2.1.5", "id": 2} ] --PACKAGES-- [ {"name": "some/pkg", "version": "1.0.0"}, - {"name": "some/pkg", "version": "1.1.0"} + {"name": "some/pkg", "version": "1.1.0"}, + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "root/req", "version": "2.0.0", "require": {"dep/dep": "3.*"}}, + {"name": "dep/dep", "version": "2.3.4"}, + {"name": "dep/dep", "version": "3.0.1"} ] --EXPECT-- [ - 1 + 1, + 2, + "root/req-1.0.0.0", + "root/req-2.0.0.0" ] From bb4cabc0b9245e1b7aa5e34660d9848fd0332927 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Wed, 29 Apr 2020 22:56:29 +0200 Subject: [PATCH 06/54] PoolBuilder: Avoid loading any alternatives to fixed packages --- src/Composer/DependencyResolver/PoolBuilder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 260775cd1..13f9e76ed 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -124,7 +124,7 @@ class PoolBuilder $loadNames = array(); foreach ($request->getFixedPackages() as $package) { - $this->loadedNames[$package->getName()] = new Constraint('==', $package->getVersion()); + $this->loadedNames[$package->getName()] = new EmptyConstraint(); // replace means conflict, so if a fixed package replaces a name, no need to load that one, packages would conflict anyways foreach ($package->getReplaces() as $link) { From 85ea29d2418e162e30a0577b331a97c58b81783a Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Wed, 29 Apr 2020 23:01:57 +0200 Subject: [PATCH 07/54] PoolBuilder failing test: do not load alternatives for pkg replaced by fixed pkg --- ...kages-replaced-do-not-load-from-repos.test | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-replaced-do-not-load-from-repos.test diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-replaced-do-not-load-from-repos.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-replaced-do-not-load-from-repos.test new file mode 100644 index 000000000..8006fc8db --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-replaced-do-not-load-from-repos.test @@ -0,0 +1,29 @@ +--TEST-- +Packages replaced by fixed packages do not get loaded from the repos + +--REQUEST-- +{ + "some/pkg": "*", + "root/req": "*" +} + +--FIXED-- +[ + {"name": "some/pkg", "version": "1.0.3", "replace": {"dep/dep": "2.1.0"}, "id": 1} +] + +--PACKAGES-- +[ + {"name": "some/pkg", "version": "1.0.0"}, + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "root/req", "version": "2.0.0", "require": {"dep/dep": "3.*"}}, + {"name": "dep/dep", "version": "2.3.4"}, + {"name": "dep/dep", "version": "3.0.1"} +] + +--EXPECT-- +[ + 1, + "root/req-1.0.0.0", + "root/req-2.0.0.0" +] From 5dd4b456b915aeda296fb659a43c38421627ebec Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Wed, 29 Apr 2020 23:02:37 +0200 Subject: [PATCH 08/54] PoolBuilder: Do not load any alternatives to pkgs replaced by fixed pkg --- src/Composer/DependencyResolver/PoolBuilder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 13f9e76ed..082c0d339 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -128,7 +128,7 @@ class PoolBuilder // replace means conflict, so if a fixed package replaces a name, no need to load that one, packages would conflict anyways foreach ($package->getReplaces() as $link) { - $this->loadedNames[$link->getTarget()] = $link->getConstraint(); + $this->loadedNames[$link->getTarget()] = new EmptyConstraint(); } // TODO in how far can we do the above for conflicts? It's more tricky cause conflicts can be limited to From 4f13875f3fceb99e61192cd1b70e8203097bbafc Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 30 Apr 2020 00:04:55 +0200 Subject: [PATCH 09/54] PoolBuilderTest: add new fixtures for partial updates with locked packages --- ...fixed-packages-do-not-load-from-repos.test | 6 ++- ...kages-replaced-do-not-load-from-repos.test | 6 ++- ...-update-transitive-deps-no-root-unfix.test | 42 ++++++++++++++++++ .../partial-update-transitive-deps-unfix.test | 43 +++++++++++++++++++ .../Fixtures/poolbuilder/partial-update.test | 37 ++++++++++++++++ ...minimum-stability-and-filter-packages.test | 6 ++- .../DependencyResolver/PoolBuilderTest.php | 20 ++++++++- 7 files changed, 152 insertions(+), 8 deletions(-) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-no-root-unfix.test create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update.test diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-do-not-load-from-repos.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-do-not-load-from-repos.test index 0abc4a67e..976587e5b 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-do-not-load-from-repos.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-do-not-load-from-repos.test @@ -3,8 +3,10 @@ Fixed packages do not get loaded from the repos --REQUEST-- { - "some/pkg": "*", - "root/req": "*" + "require": { + "some/pkg": "*", + "root/req": "*" + } } --FIXED-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-replaced-do-not-load-from-repos.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-replaced-do-not-load-from-repos.test index 8006fc8db..bc403f83d 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-replaced-do-not-load-from-repos.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-replaced-do-not-load-from-repos.test @@ -3,8 +3,10 @@ Packages replaced by fixed packages do not get loaded from the repos --REQUEST-- { - "some/pkg": "*", - "root/req": "*" + "require": { + "some/pkg": "*", + "root/req": "*" + } } --FIXED-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-no-root-unfix.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-no-root-unfix.test new file mode 100644 index 000000000..484d71dfe --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-no-root-unfix.test @@ -0,0 +1,42 @@ +--TEST-- +Partially updating one root requirement with transitive deps without root requirements keeps the other root requirement fixed. + +--REQUEST-- +{ + "require": { + "root/update": "*", + "root/fix": "*" + }, + "locked": [ + {"name": "root/update", "version": "1.0.1", "require": {"dep/dep": "1.*"}, "id": 1}, + {"name": "dep/dep", "version": "1.0.2", "require": {"root/fix": "1.*"}, "id": 2}, + {"name": "root/fix", "version": "1.0.3", "id": 3} + ], + "allowList": [ + "root/update" + ], + "allowTransitiveDepsNoRootRequire": true +} + +--FIXED-- +[ +] + +--PACKAGES-- +[ + {"name": "root/update", "version": "1.0.4", "require": {"dep/dep": "1.*"}}, + {"name": "root/update", "version": "1.0.5", "require": {"dep/dep": "2.*"}}, + {"name": "root/fix", "version": "1.0.6"}, + {"name": "root/fix", "version": "2.0.7"}, + {"name": "dep/dep", "version": "1.0.8", "require": {"root/fix": "1.*"}}, + {"name": "dep/dep", "version": "2.0.9", "require": {"root/fix": "2.*"}} +] + +--EXPECT-- +[ + 3, + "root/update-1.0.4.0", + "root/update-1.0.5.0", + "dep/dep-1.0.8.0", + "dep/dep-2.0.9.0" +] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test new file mode 100644 index 000000000..7e0cf2d2d --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test @@ -0,0 +1,43 @@ +--TEST-- +Partially updating one root requirement with transitive deps fully updates another one too. + +--REQUEST-- +{ + "require": { + "root/update": "*", + "root/dep": "*" + }, + "locked": [ + {"name": "root/update", "version": "1.0.1", "require": {"dep/dep": "1.*"}, "id": 1}, + {"name": "dep/dep", "version": "1.0.2", "require": {"root/dep": "1.*"}, "id": 2}, + {"name": "root/dep", "version": "1.0.3", "id": 3} + ], + "allowList": [ + "root/update" + ], + "allowTransitiveDeps": true +} + +--FIXED-- +[ +] + +--PACKAGES-- +[ + {"name": "root/update", "version": "1.0.4", "require": {"dep/dep": "1.*"}}, + {"name": "root/update", "version": "1.0.5", "require": {"dep/dep": "2.*"}}, + {"name": "root/dep", "version": "1.0.6"}, + {"name": "root/dep", "version": "2.0.7"}, + {"name": "dep/dep", "version": "1.0.8", "require": {"root/dep": "1.*"}}, + {"name": "dep/dep", "version": "2.0.9", "require": {"root/dep": "2.*"}} +] + +--EXPECT-- +[ + "root/update-1.0.4.0", + "root/update-1.0.5.0", + "dep/dep-1.0.8.0", + "dep/dep-2.0.9.0", + "root/dep-1.0.6.0", + "root/dep-2.0.7.0" +] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update.test new file mode 100644 index 000000000..af6c5c689 --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update.test @@ -0,0 +1,37 @@ +--TEST-- +Partially updating a root requirement without deps, still selects a new dependency if the update results in a replacement missing for another locked package. + +--REQUEST-- +{ + "require": { + "some/pkg": "*", + "root/req": "*" + }, + "locked": [ + {"name": "some/pkg", "version": "1.0.3", "replace": {"dep/dep": "2.1.0"}, "id": 1}, + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}, "id": 2} + ], + "allowList": [ + "some/pkg" + ] +} + +--FIXED-- +[ +] + +--PACKAGES-- +[ + {"name": "some/pkg", "version": "1.0.4"}, + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "root/req", "version": "2.0.0", "require": {"dep/dep": "3.*"}}, + {"name": "dep/dep", "version": "2.3.4"}, + {"name": "dep/dep", "version": "3.0.1"} +] + +--EXPECT-- +[ + 2, + "some/pkg-1.0.4.0", + "dep/dep-2.3.4.0" +] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/stability-flags-take-over-minimum-stability-and-filter-packages.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/stability-flags-take-over-minimum-stability-and-filter-packages.test index 5a6ca8f2f..b273753d2 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/stability-flags-take-over-minimum-stability-and-filter-packages.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/stability-flags-take-over-minimum-stability-and-filter-packages.test @@ -18,8 +18,10 @@ Stability flags apply --REQUEST-- { - "flagged/pkg": "*", - "default/pkg": "*" + "require": { + "flagged/pkg": "*", + "default/pkg": "*" + } } --PACKAGES-- diff --git a/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php b/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php index 647ee2c2d..f02b99fc9 100644 --- a/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php +++ b/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php @@ -73,14 +73,30 @@ class PoolBuilderTest extends TestCase $repositorySet = new RepositorySet($minimumStability, $stabilityFlags, $rootAliases); $repositorySet->addRepository($repo = new ArrayRepository()); + $repositorySet->addRepository($lockedRepo = new LockArrayRepository()); foreach ($packages as $package) { $repo->addPackage($loadPackage($package)); } - $request = new Request(); - foreach ($requestData as $package => $constraint) { + if (isset($requestData['locked'])) { + foreach ($requestData['locked'] as $package) { + $lockedRepo->addPackage($loadPackage($package)); + } + } + $request = new Request($lockedRepo); + foreach ($requestData['require'] as $package => $constraint) { $request->requireName($package, $parser->parseConstraints($constraint)); } + if (isset($requestData['allowList'])) { + $transitiveDeps = Request::UPDATE_ONLY_LISTED; + if (isset($requestData['allowTransitiveDepsNoRootRequire']) && $requestData['allowTransitiveDepsNoRootRequire']) { + $transitiveDeps = Request::UPDATE_LISTED_WITH_TRANSITIVE_DEPS_NO_ROOT_REQUIRE; + } + if (isset($requestData['allowTransitiveDeps']) && $requestData['allowTransitiveDeps']) { + $transitiveDeps = Request::UPDATE_LISTED_WITH_TRANSITIVE_DEPS; + } + $request->setUpdateAllowList(array_flip($requestData['allowList']), $transitiveDeps); + } foreach ($fixed as $fixedPackage) { $request->fixPackage($loadPackage($fixedPackage)); From 43888cae8c17020aa08f383e8332da0435d111f6 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 30 Apr 2020 00:15:49 +0200 Subject: [PATCH 10/54] PoolBuilder: failing test for partial update with multiple deps --- ...-update-transitive-deps-no-root-unfix.test | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-no-root-unfix.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-no-root-unfix.test index 484d71dfe..360519edb 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-no-root-unfix.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-no-root-unfix.test @@ -8,9 +8,10 @@ Partially updating one root requirement with transitive deps without root requir "root/fix": "*" }, "locked": [ - {"name": "root/update", "version": "1.0.1", "require": {"dep/dep": "1.*"}, "id": 1}, + {"name": "root/update", "version": "1.0.1", "require": {"dep/dep": "1.*", "dep2/dep2": "1.*"}, "id": 1}, {"name": "dep/dep", "version": "1.0.2", "require": {"root/fix": "1.*"}, "id": 2}, - {"name": "root/fix", "version": "1.0.3", "id": 3} + {"name": "dep2/dep2", "version": "1.0.2", "require": {"dep/dep": "1.*"}, "id": 3}, + {"name": "root/fix", "version": "1.0.3", "id": 4} ], "allowList": [ "root/update" @@ -24,19 +25,23 @@ Partially updating one root requirement with transitive deps without root requir --PACKAGES-- [ - {"name": "root/update", "version": "1.0.4", "require": {"dep/dep": "1.*"}}, - {"name": "root/update", "version": "1.0.5", "require": {"dep/dep": "2.*"}}, + {"name": "root/update", "version": "1.0.4", "require": {"dep/dep": "1.*", "dep2/dep2": "1.*"}}, + {"name": "root/update", "version": "1.0.5", "require": {"dep/dep": "2.*", "dep2/dep2": "2.*"}}, {"name": "root/fix", "version": "1.0.6"}, {"name": "root/fix", "version": "2.0.7"}, {"name": "dep/dep", "version": "1.0.8", "require": {"root/fix": "1.*"}}, - {"name": "dep/dep", "version": "2.0.9", "require": {"root/fix": "2.*"}} + {"name": "dep/dep", "version": "2.0.9", "require": {"root/fix": "2.*"}}, + {"name": "dep2/dep2", "version": "1.0.8", "require": {"dep/dep": "1.*"}}, + {"name": "dep2/dep2", "version": "2.0.9", "require": {"dep/dep": "2.*"}} ] --EXPECT-- [ - 3, + 4, "root/update-1.0.4.0", "root/update-1.0.5.0", "dep/dep-1.0.8.0", - "dep/dep-2.0.9.0" + "dep/dep-2.0.9.0", + "dep2/dep2-1.0.8.0", + "dep2/dep2-2.0.9.0" ] From d78c37edd22a0c31816334aedee36ea510e7a60d Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Thu, 30 Apr 2020 15:56:06 +0200 Subject: [PATCH 11/54] Cleanup and added some more improvements, tests still failing --- .../DependencyResolver/PoolBuilder.php | 128 +++++++++++------- 1 file changed, 77 insertions(+), 51 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 082c0d339..ea49f7eca 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -63,6 +63,11 @@ class PoolBuilder */ private $aliasMap = array(); + /** + * @psalm-var array + */ + private $packagesToLoad = array(); + /** * @psalm-var array */ @@ -122,7 +127,6 @@ class PoolBuilder } } - $loadNames = array(); foreach ($request->getFixedPackages() as $package) { $this->loadedNames[$package->getName()] = new EmptyConstraint(); @@ -139,52 +143,25 @@ class PoolBuilder || $package->getRepository() instanceof PlatformRepository || StabilityFilter::isPackageAcceptable($this->acceptableStabilities, $this->stabilityFlags, $package->getNames(), $package->getStability()) ) { - $loadNames += $this->loadPackage($request, $package, false); + $this->loadPackage($request, $package, false); } else { $this->unacceptableFixedPackages[] = $package; } } foreach ($request->getRequires() as $packageName => $constraint) { - // fixed packages have already been added, so if a root require needs one of them, no need to do anything - if (isset($this->loadedNames[$packageName])) { - continue; - } - - $loadNames[$packageName] = $constraint; + $this->markPackageNameForLoading($packageName, $constraint); } - // clean up loadNames for anything we manually marked loaded above - foreach ($loadNames as $name => $void) { + // clean up packagesToLoad for anything we manually marked loaded above + foreach ($this->packagesToLoad as $name => $constraint) { if (isset($this->loadedNames[$name])) { - unset($loadNames[$name]); + unset($this->packagesToLoad[$name]); } } - while (!empty($loadNames)) { - foreach ($loadNames as $name => $constraint) { - $this->loadedNames[$name] = $constraint; - } - - $newLoadNames = array(); - foreach ($repositories as $repository) { - // these repos have their packages fixed if they need to be loaded so we - // never need to load anything else from them - if ($repository instanceof PlatformRepository || $repository === $request->getLockedRepository()) { - continue; - } - $result = $repository->loadPackages($loadNames, $this->acceptableStabilities, $this->stabilityFlags); - - foreach ($result['namesFound'] as $name) { - // avoid loading the same package again from other repositories once it has been found - unset($loadNames[$name]); - } - foreach ($result['packages'] as $package) { - $newLoadNames += $this->loadPackage($request, $package); - } - } - - $loadNames = $newLoadNames; + while (!empty($this->packagesToLoad)) { + $this->loadPackagesMarkedForLoading($request, $repositories); } foreach ($this->packages as $i => $package) { @@ -231,6 +208,7 @@ class PoolBuilder $pool = new Pool($this->packages, $this->unacceptableFixedPackages); $this->aliasMap = array(); + $this->packagesToLoad = array(); $this->loadedNames = array(); $this->packages = array(); $this->unacceptableFixedPackages = array(); @@ -238,6 +216,65 @@ class PoolBuilder return $pool; } + private function markPackageNameForLoading($name, ConstraintInterface $constraint) + { + if (!isset($this->loadedNames[$name])) { + $this->packagesToLoad[$name] = $constraint; + return; + } + + // No need to load this package with this constraint because it was + // already loaded in one that matches + if ($this->loadedNames[$name]->matches($constraint)) { + return; + } + + // Maybe it was already marked before but not loaded yet. In that case + // we have to extend the constraint (we don't check if the match because + // MultiConstraint::create() will optimize anyway + if (isset($this->packagesToLoad[$name])) { + $constraint = MultiConstraint::create(array($this->packagesToLoad[$name], $constraint), false); + } + + // We have already loaded that package but not in the constraint that's + // required. We extend the constraint and mark that package as not being loaded + // yet so we get the required package versions + $this->packagesToLoad[$name] = MultiConstraint::create(array($this->loadedNames[$name], $constraint), false); + unset($this->loadedNames[$name]); + } + + private function loadPackagesMarkedForLoading(Request $request, $repositories) + { + foreach ($this->packagesToLoad as $name => $constraint) { + $this->loadedNames[$name] = $constraint; + } + + foreach ($repositories as $repository) { + if (empty($this->packagesToLoad)) { + break; + } + + // these repos have their packages fixed if they need to be loaded so we + // never need to load anything else from them + if ($repository instanceof PlatformRepository || $repository === $request->getLockedRepository()) { + continue; + } + $result = $repository->loadPackages($this->packagesToLoad, $this->acceptableStabilities, $this->stabilityFlags); + + foreach ($result['namesFound'] as $name) { + // avoid loading the same package again from other repositories once it has been found + unset($this->packagesToLoad[$name]); + } + foreach ($result['packages'] as $package) { + $this->loadPackage($request, $package); + } + } + + // Make sure we empty the packagesToLoad here as it would result + // in an endless loop with non-existent packages for example + $this->packagesToLoad = array(); + } + private function loadPackage(Request $request, PackageInterface $package, $propagateUpdate = true) { end($this->packages); @@ -276,32 +313,23 @@ class PoolBuilder $this->aliasMap[spl_object_hash($aliasPackage->getAliasOf())][$index+1] = $aliasPackage; } - $loadNames = array(); foreach ($package->getRequires() as $link) { $require = $link->getTarget(); $linkConstraint = $link->getConstraint(); - if (!isset($this->loadedNames[$require])) { - $loadNames[$require] = $linkConstraint; // if this is a partial update with transitive dependencies we need to unfix the package we now know is a // dependency of another package which we are trying to update, and then attempt to load it again - } elseif ($propagateUpdate && $request->getUpdateAllowTransitiveDependencies() && isset($this->skippedLoad[$require])) { + if ($propagateUpdate && $request->getUpdateAllowTransitiveDependencies() && isset($this->skippedLoad[$require])) { if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$require])) { $this->unfixPackage($request, $require); - $loadNames[$require] = null; + $this->markPackageNameForLoading($require, $linkConstraint); } elseif (!$request->getUpdateAllowTransitiveRootDependencies() && $this->isRootRequire($request, $require) && !isset($this->updateAllowWarned[$require])) { $this->updateAllowWarned[$require] = true; $this->io->writeError('Dependency "'.$require.'" is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies to include root dependencies.'); } - } else { - // Check if packages we already loaded match the constraint and if they don't - // extend the constraint and mark that package as not being loaded yet - // so we get the required package versions - if (!$this->loadedNames[$require]->matches($linkConstraint)) { - $loadNames[$require] = MultiConstraint::create(array($this->loadedNames[$require], $linkConstraint), false); - unset($this->loadedNames[$require]); - } } + + $this->markPackageNameForLoading($require, $linkConstraint); } // if we're doing a partial update with deps we also need to unfix packages which are being replaced in case they @@ -312,7 +340,7 @@ class PoolBuilder if (isset($this->loadedNames[$replace]) && isset($this->skippedLoad[$replace])) { if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$replace])) { $this->unfixPackage($request, $replace); - $loadNames[$replace] = $link->getConstraint(); + $this->markPackageNameForLoading($replace, $link->getConstraint()); } elseif (!$request->getUpdateAllowTransitiveRootDependencies() && $this->isRootRequire($request, $replace) && !isset($this->updateAllowWarned[$replace])) { $this->updateAllowWarned[$replace] = true; $this->io->writeError('Dependency "'.$replace.'" is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies to include root dependencies.'); @@ -320,8 +348,6 @@ class PoolBuilder } } } - - return $loadNames; } /** From b87fc5bbfd51263ac0da2a7b3e11e9483543e719 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Thu, 30 Apr 2020 17:19:21 +0200 Subject: [PATCH 12/54] Added more poolbuilder test --- .../DependencyResolver/PoolBuilder.php | 3 +- ...-not-loaded-if-not-required-recursive.test | 29 +++++++++++++++++++ ...are-not-loaded-if-not-required-simple.test | 26 +++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-recursive.test create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-simple.test diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index ea49f7eca..5766f25c1 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -272,7 +272,8 @@ class PoolBuilder // Make sure we empty the packagesToLoad here as it would result // in an endless loop with non-existent packages for example - $this->packagesToLoad = array(); + // TODO: fixme, this should only happen if it's not a new package + // $this->packagesToLoad = array(); } private function loadPackage(Request $request, PackageInterface $package, $propagateUpdate = true) diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-recursive.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-recursive.test new file mode 100644 index 000000000..efae34752 --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-recursive.test @@ -0,0 +1,29 @@ +--TEST-- +Test irrelevant package versions are not loaded + +--REQUEST-- +{ + "require": { + "root/req": "*" + } +} + +--FIXED-- +[ +] + +--PACKAGES-- +[ + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "dep/dep", "version": "2.3.4", "require": {"dep/dep2": "2.*"}}, + {"name": "dep/dep", "version": "3.0.1"}, + {"name": "dep/dep2", "version": "2.3.4"}, + {"name": "dep/dep2", "version": "3.0.1"} +] + +--EXPECT-- +[ + "root/req-1.0.0.0", + "dep/dep-2.3.4.0", + "dep/dep2-2.3.4.0" +] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-simple.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-simple.test new file mode 100644 index 000000000..b17fb3228 --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-simple.test @@ -0,0 +1,26 @@ +--TEST-- +Test irrelevant package versions are not loaded + +--REQUEST-- +{ + "require": { + "root/req": "*" + } +} + +--FIXED-- +[ +] + +--PACKAGES-- +[ + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "dep/dep", "version": "2.3.4"}, + {"name": "dep/dep", "version": "3.0.1"} +] + +--EXPECT-- +[ + "root/req-1.0.0.0", + "dep/dep-2.3.4.0" +] From 28f82032cd8d3b4ee6769317c5a52a6d9fe515aa Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Thu, 30 Apr 2020 17:22:15 +0200 Subject: [PATCH 13/54] And here's the key test :) --- ...-not-loaded-if-not-required-expansion.test | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test new file mode 100644 index 000000000..be4c7f871 --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test @@ -0,0 +1,34 @@ +--TEST-- +Test irrelevant package versions are not loaded + +--REQUEST-- +{ + "require": { + "root/req": "*" + } +} + +--FIXED-- +[ +] + +--PACKAGES-- +[ + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "dep/dep", "version": "2.3.4", "require": {"dep/dep2": "2.*"}}, + {"name": "dep/dep", "version": "2.3.5"}, + {"name": "dep/dep", "version": "3.0.0"}, + {"name": "dep/dep", "version": "4.0.0"}, + {"name": "dep/dep2", "version": "2.3.4", "require": {"dep/dep": "3.*"}} +] + +--EXPECT-- +[ + "root/req-1.0.0.0", + "dep/dep-2.3.4.0", + "dep/dep-2.3.5.0", + "dep/dep2-2.3.4.0", + "dep/dep-2.3.4.0", + "dep/dep-2.3.5.0", + "dep/dep-3.0.0.0" +] From bca88bdd4b41e0f18345a23e995c3aca445ed7c6 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Thu, 30 Apr 2020 17:32:25 +0200 Subject: [PATCH 14/54] Fixed PoolBuilder running endlessly when packages do not exist --- .../DependencyResolver/PoolBuilder.php | 10 +++++--- .../packages-that-do-not-exist.test | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/packages-that-do-not-exist.test diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 5766f25c1..5bcccce66 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -249,6 +249,8 @@ class PoolBuilder $this->loadedNames[$name] = $constraint; } + $preLoad = $this->packagesToLoad; + foreach ($repositories as $repository) { if (empty($this->packagesToLoad)) { break; @@ -271,9 +273,11 @@ class PoolBuilder } // Make sure we empty the packagesToLoad here as it would result - // in an endless loop with non-existent packages for example - // TODO: fixme, this should only happen if it's not a new package - // $this->packagesToLoad = array(); + // in an endless loop if the array remains unchanged. + // This could happen with non-existent packages for example. + if ($preLoad == $this->packagesToLoad) { + $this->packagesToLoad = array(); + } } private function loadPackage(Request $request, PackageInterface $package, $propagateUpdate = true) diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/packages-that-do-not-exist.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/packages-that-do-not-exist.test new file mode 100644 index 000000000..b2c57f966 --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/packages-that-do-not-exist.test @@ -0,0 +1,25 @@ +--TEST-- +Test irrelevant package versions are not loaded + +--REQUEST-- +{ + "require": { + "root/req": "*" + } +} + +--FIXED-- +[ +] + +--PACKAGES-- +[ + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "dep/dep", "version": "2.3.4", "require": {"dep/dep3": "2.*"}} +] + +--EXPECT-- +[ + "root/req-1.0.0.0", + "dep/dep-2.3.4.0" +] From 8e2dd62d10b85a3a2de198b5dd100a73e604c4b5 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Thu, 30 Apr 2020 17:40:06 +0200 Subject: [PATCH 15/54] Fixed tests related to constraint changes --- tests/Composer/Test/DependencyResolver/RequestTest.php | 3 ++- tests/Composer/Test/DependencyResolver/RuleTest.php | 2 +- tests/Composer/Test/DependencyResolver/SolverTest.php | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/Composer/Test/DependencyResolver/RequestTest.php b/tests/Composer/Test/DependencyResolver/RequestTest.php index e2a3ce9e6..77955597a 100644 --- a/tests/Composer/Test/DependencyResolver/RequestTest.php +++ b/tests/Composer/Test/DependencyResolver/RequestTest.php @@ -14,6 +14,7 @@ namespace Composer\Test\DependencyResolver; use Composer\DependencyResolver\Request; use Composer\Repository\ArrayRepository; +use Composer\Semver\Constraint\EmptyConstraint; use Composer\Test\TestCase; class RequestTest extends TestCase @@ -34,7 +35,7 @@ class RequestTest extends TestCase $this->assertEquals( array( - 'foo' => null, + 'foo' => new EmptyConstraint(), ), $request->getRequires() ); diff --git a/tests/Composer/Test/DependencyResolver/RuleTest.php b/tests/Composer/Test/DependencyResolver/RuleTest.php index 2e1a9921d..fd5567520 100644 --- a/tests/Composer/Test/DependencyResolver/RuleTest.php +++ b/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, false)); + $this->assertEquals('baz 1.1 relates to foo [] -> satisfiable by foo[2.1].', $rule->getPrettyString($repositorySetMock, $requestMock, $pool, false)); } } diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index 6fa7f1f84..48a30331f 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/tests/Composer/Test/DependencyResolver/SolverTest.php @@ -652,9 +652,9 @@ class SolverTest extends TestCase $msg = "\n"; $msg .= " Problem 1\n"; - $msg .= " - Root composer.json requires a -> satisfiable by A[1.0].\n"; + $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"; + $msg .= " - Root composer.json requires b [] -> satisfiable by B[1.0].\n"; $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool, false)); } } @@ -683,7 +683,7 @@ class SolverTest extends TestCase $msg = "\n"; $msg .= " Problem 1\n"; - $msg .= " - Root composer.json requires a -> satisfiable by A[1.0].\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 the constraint.\n"; $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool, false)); } From 29611a4d27a3b952f0d65e1c5f23a8f043a2a673 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Thu, 30 Apr 2020 18:26:20 +0200 Subject: [PATCH 16/54] Fixed test descriptions --- ...ckage-versions-are-not-loaded-if-not-required-expansion.test | 2 +- ...ckage-versions-are-not-loaded-if-not-required-recursive.test | 2 +- .../Fixtures/poolbuilder/packages-that-do-not-exist.test | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test index be4c7f871..cd487fca3 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test @@ -1,5 +1,5 @@ --TEST-- -Test irrelevant package versions are not loaded +Tests if version constraint is expanded. If not, dep/dep 3.0.0 would not be loaded here. --REQUEST-- { diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-recursive.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-recursive.test index efae34752..65b762df1 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-recursive.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-recursive.test @@ -1,5 +1,5 @@ --TEST-- -Test irrelevant package versions are not loaded +Test irrelevant package versions are not loaded recursively --REQUEST-- { diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/packages-that-do-not-exist.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/packages-that-do-not-exist.test index b2c57f966..490964fdf 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/packages-that-do-not-exist.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/packages-that-do-not-exist.test @@ -1,5 +1,5 @@ --TEST-- -Test irrelevant package versions are not loaded +Test package is not found --REQUEST-- { From 71fde800485f64409f6b7284f3150589248aad4b Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Fri, 1 May 2020 19:56:55 +0200 Subject: [PATCH 17/54] Fixed test --- ...ckage-versions-are-not-loaded-if-not-required-expansion.test | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test index cd487fca3..198df20a4 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test @@ -28,7 +28,5 @@ Tests if version constraint is expanded. If not, dep/dep 3.0.0 would not be load "dep/dep-2.3.4.0", "dep/dep-2.3.5.0", "dep/dep2-2.3.4.0", - "dep/dep-2.3.4.0", - "dep/dep-2.3.5.0", "dep/dep-3.0.0.0" ] From 8b4e8346751db37d474a72de61cb5a3c5a6a2937 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Fri, 1 May 2020 19:57:18 +0200 Subject: [PATCH 18/54] Added another poolbuilder test to show issue with constraint matching --- ...t-expansion-works-with-exact-versions.test | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test new file mode 100644 index 000000000..7a3046ae0 --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test @@ -0,0 +1,29 @@ +--TEST-- +Tests if version constraint is expanded. If not, dep/dep 3.0.0 would not be loaded here. + +--REQUEST-- +{ + "require": { + "root/req": "*" + } +} + +--FIXED-- +[ +] + +--PACKAGES-- +[ + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.3.4"}}, + {"name": "dep/dep", "version": "2.3.4", "require": {"dep/dep2": "2.*"}}, + {"name": "dep/dep", "version": "2.3.5"}, + {"name": "dep/dep2", "version": "2.3.4", "require": {"dep/dep": "2.*"}} +] + +--EXPECT-- +[ + "root/req-1.0.0.0", + "dep/dep-2.3.4.0", + "dep/dep2-2.3.4.0", + "dep/dep-2.3.5.0" +] From b935a76bc6742a2255944ad30221bf69eb40d962 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Fri, 1 May 2020 19:58:27 +0200 Subject: [PATCH 19/54] Removed already covered test --- ...are-not-loaded-if-not-required-simple.test | 26 ------------------- 1 file changed, 26 deletions(-) delete mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-simple.test diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-simple.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-simple.test deleted file mode 100644 index b17fb3228..000000000 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-simple.test +++ /dev/null @@ -1,26 +0,0 @@ ---TEST-- -Test irrelevant package versions are not loaded - ---REQUEST-- -{ - "require": { - "root/req": "*" - } -} - ---FIXED-- -[ -] - ---PACKAGES-- -[ - {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, - {"name": "dep/dep", "version": "2.3.4"}, - {"name": "dep/dep", "version": "3.0.1"} -] - ---EXPECT-- -[ - "root/req-1.0.0.0", - "dep/dep-2.3.4.0" -] From 2427bef238f3cfae721fe5a582a520715faecb76 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Fri, 1 May 2020 20:00:41 +0200 Subject: [PATCH 20/54] Typo --- src/Composer/DependencyResolver/PoolBuilder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 5bcccce66..634256cb1 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -230,7 +230,7 @@ class PoolBuilder } // Maybe it was already marked before but not loaded yet. In that case - // we have to extend the constraint (we don't check if the match because + // we have to extend the constraint (we don't check if they match because // MultiConstraint::create() will optimize anyway if (isset($this->packagesToLoad[$name])) { $constraint = MultiConstraint::create(array($this->packagesToLoad[$name], $constraint), false); From 63906171f08619fcc1f4d112389936d90be3f13b Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Fri, 1 May 2020 20:14:04 +0200 Subject: [PATCH 21/54] Cleanup EmptyConstraint output --- .../Test/DependencyResolver/RuleTest.php | 4 ++-- .../Test/DependencyResolver/SolverTest.php | 18 ++++++++++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/tests/Composer/Test/DependencyResolver/RuleTest.php b/tests/Composer/Test/DependencyResolver/RuleTest.php index fd5567520..cd3b74887 100644 --- a/tests/Composer/Test/DependencyResolver/RuleTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleTest.php @@ -102,8 +102,8 @@ class RuleTest extends TestCase $repositorySetMock = $this->getMockBuilder('Composer\Repository\RepositorySet')->disableOriginalConstructor()->getMock(); $requestMock = $this->getMockBuilder('Composer\DependencyResolver\Request')->disableOriginalConstructor()->getMock(); - $rule = new GenericRule(array($p1->getId(), -$p2->getId()), Rule::RULE_PACKAGE_REQUIRES, new Link('baz', 'foo')); + $rule = new GenericRule(array($p1->getId(), -$p2->getId()), Rule::RULE_PACKAGE_REQUIRES, new Link('baz', 'foo', 'relates to', '*')); - $this->assertEquals('baz 1.1 relates to foo [] -> satisfiable by foo[2.1].', $rule->getPrettyString($repositorySetMock, $requestMock, $pool, false)); + $this->assertEquals('baz 1.1 relates to foo * -> satisfiable by foo[2.1].', $rule->getPrettyString($repositorySetMock, $requestMock, $pool, false)); } } diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index 48a30331f..56d2d7e5b 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/tests/Composer/Test/DependencyResolver/SolverTest.php @@ -639,8 +639,11 @@ class SolverTest extends TestCase $this->reposComplete(); - $this->request->requireName('A'); - $this->request->requireName('B'); + $emptyConstraint = new EmptyConstraint(); + $emptyConstraint->setPrettyString('*'); + + $this->request->requireName('A', $emptyConstraint); + $this->request->requireName('B', $emptyConstraint); $this->createSolver(); try { @@ -652,9 +655,9 @@ class SolverTest extends TestCase $msg = "\n"; $msg .= " Problem 1\n"; - $msg .= " - Root composer.json requires a [] -> satisfiable by A[1.0].\n"; + $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"; + $msg .= " - Root composer.json requires b * -> satisfiable by B[1.0].\n"; $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool, false)); } } @@ -712,7 +715,10 @@ class SolverTest extends TestCase $this->reposComplete(); - $this->request->requireName('A'); + $emptyConstraint = new EmptyConstraint(); + $emptyConstraint->setPrettyString('*'); + + $this->request->requireName('A', $emptyConstraint); $this->createSolver(); try { @@ -729,7 +735,7 @@ class SolverTest extends TestCase $msg .= " - B 1.0 requires c >= 1.0 -> satisfiable by C[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"; + $msg .= " - Root composer.json requires a * -> satisfiable by A[1.0].\n"; $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool, false)); } } From bde9502473221a735ed6b235aab201a948ee13c3 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Fri, 1 May 2020 20:32:17 +0200 Subject: [PATCH 22/54] Made the constraint argument in Link mandatory --- src/Composer/Package/Link.php | 4 +- .../Test/Autoload/AutoloadGeneratorTest.php | 87 ++++++++++--------- .../Test/DependencyResolver/RuleTest.php | 3 +- .../Test/DependencyResolver/SolverTest.php | 4 +- .../Test/Package/RootAliasPackageTest.php | 11 +-- .../Repository/InstalledRepositoryTest.php | 5 +- 6 files changed, 59 insertions(+), 55 deletions(-) diff --git a/src/Composer/Package/Link.php b/src/Composer/Package/Link.php index 03ea3edff..16254f68e 100644 --- a/src/Composer/Package/Link.php +++ b/src/Composer/Package/Link.php @@ -56,11 +56,11 @@ class Link * @param string $description Used to create a descriptive string representation * @param string|null $prettyConstraint */ - public function __construct($source, $target, ConstraintInterface $constraint = null, $description = 'relates to', $prettyConstraint = null) + public function __construct($source, $target, ConstraintInterface $constraint, $description = 'relates to', $prettyConstraint = null) { $this->source = strtolower($source); $this->target = strtolower($target); - $this->constraint = $constraint ? $constraint : new EmptyConstraint(); + $this->constraint = $constraint; $this->description = $description; $this->prettyConstraint = $prettyConstraint; } diff --git a/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php b/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php index c178e93d8..896221c13 100644 --- a/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php +++ b/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php @@ -16,6 +16,7 @@ use Composer\Autoload\AutoloadGenerator; use Composer\Package\Link; use Composer\Package\Version\VersionParser; use Composer\Semver\Constraint\Constraint; +use Composer\Semver\Constraint\EmptyConstraint; use Composer\Util\Filesystem; use Composer\Package\AliasPackage; use Composer\Package\Package; @@ -366,8 +367,8 @@ class AutoloadGeneratorTest extends TestCase { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a'), - new Link('a', 'b/b'), + new Link('a', 'a/a', new EmptyConstraint()), + new Link('a', 'b/b', new EmptyConstraint()), )); $packages = array(); @@ -395,7 +396,7 @@ class AutoloadGeneratorTest extends TestCase { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a'), + new Link('a', 'a/a', new EmptyConstraint()), )); $packages = array(); @@ -403,11 +404,11 @@ class AutoloadGeneratorTest extends TestCase $packages[] = $b = new Package('b/b', '1.0', '1.0'); $a->setAutoload(array('psr-0' => array('A' => 'src/', 'A\\B' => 'lib/'))); $a->setRequires(array( - new Link('a/a', 'b/b'), + new Link('a/a', 'b/b', new EmptyConstraint()), )); $b->setAutoload(array('psr-0' => array('B\\Sub\\Name' => 'src/'))); $b->setRequires(array( - new Link('b/b', 'a/a'), + new Link('b/b', 'a/a', new EmptyConstraint()), )); $this->repository->expects($this->once()) @@ -427,13 +428,13 @@ class AutoloadGeneratorTest extends TestCase public function testNonDevAutoloadShouldIncludeReplacedPackages() { $package = new Package('a', '1.0', '1.0'); - $package->setRequires(array(new Link('a', 'a/a'))); + $package->setRequires(array(new Link('a', 'a/a', new EmptyConstraint()))); $packages = array(); $packages[] = $a = new Package('a/a', '1.0', '1.0'); $packages[] = $b = new Package('b/b', '1.0', '1.0'); - $a->setRequires(array(new Link('a/a', 'b/c'))); + $a->setRequires(array(new Link('a/a', 'b/c', new EmptyConstraint()))); $b->setAutoload(array('psr-4' => array('B\\' => 'src/'))); $b->setReplaces( @@ -462,7 +463,7 @@ class AutoloadGeneratorTest extends TestCase { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a'), + new Link('a', 'a/a', new EmptyConstraint()), )); $packages = array(); @@ -470,11 +471,11 @@ class AutoloadGeneratorTest extends TestCase $packages[] = $b = new Package('b/b', '1.0', '1.0'); $a->setAutoload(array('psr-0' => array('A' => 'src/', 'A\\B' => 'lib/'))); $a->setRequires(array( - new Link('a/a', 'c/c'), + new Link('a/a', 'c/c', new EmptyConstraint()), )); $b->setAutoload(array('psr-0' => array('B\\Sub\\Name' => 'src/'))); $b->setReplaces(array( - new Link('b/b', 'c/c'), + new Link('b/b', 'c/c', new EmptyConstraint()), )); $this->repository->expects($this->once()) @@ -495,7 +496,7 @@ class AutoloadGeneratorTest extends TestCase { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a') + new Link('a', 'a/a', new EmptyConstraint()) )); $packages = array(); @@ -506,18 +507,18 @@ class AutoloadGeneratorTest extends TestCase $packages[] = $e = new Package('e/e', '1.0', '1.0'); $a->setAutoload(array('classmap' => array('src/A.php'))); $a->setRequires(array( - new Link('a/a', 'b/b') + new Link('a/a', 'b/b', new EmptyConstraint()) )); $b->setAutoload(array('classmap' => array('src/B.php'))); $b->setRequires(array( - new Link('b/b', 'e/e') + new Link('b/b', 'e/e', new EmptyConstraint()) )); $c->setAutoload(array('classmap' => array('src/C.php'))); $c->setReplaces(array( - new Link('c/c', 'b/b') + new Link('c/c', 'b/b', new EmptyConstraint()) )); $c->setRequires(array( - new Link('c/c', 'd/d') + new Link('c/c', 'd/d', new EmptyConstraint()) )); $d->setAutoload(array('classmap' => array('src/D.php'))); $e->setAutoload(array('classmap' => array('src/E.php'))); @@ -547,7 +548,7 @@ class AutoloadGeneratorTest extends TestCase { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a'), + new Link('a', 'a/a', new EmptyConstraint()), )); $package->setAutoload(array( @@ -652,8 +653,8 @@ EOF; { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a'), - new Link('a', 'b/b'), + new Link('a', 'a/a', new EmptyConstraint()), + new Link('a', 'b/b', new EmptyConstraint()), )); $packages = array(); @@ -692,8 +693,8 @@ EOF; { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a'), - new Link('a', 'b/b'), + new Link('a', 'a/a', new EmptyConstraint()), + new Link('a', 'b/b', new EmptyConstraint()), )); $packages = array(); @@ -732,9 +733,9 @@ EOF; { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a'), - new Link('a', 'b/b'), - new Link('a', 'c/c'), + new Link('a', 'a/a', new EmptyConstraint()), + new Link('a', 'b/b', new EmptyConstraint()), + new Link('a', 'c/c', new EmptyConstraint()), )); $packages = array(); @@ -777,9 +778,9 @@ EOF; { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a'), - new Link('a', 'b/b'), - new Link('a', 'c/c'), + new Link('a', 'a/a', new EmptyConstraint()), + new Link('a', 'b/b', new EmptyConstraint()), + new Link('a', 'c/c', new EmptyConstraint()), )); $packages = array(); @@ -827,9 +828,9 @@ EOF; $package = new Package('a', '1.0', '1.0'); $package->setAutoload(array('files' => array('root.php'))); $package->setRequires(array( - new Link('a', 'a/a'), - new Link('a', 'b/b'), - new Link('a', 'c/c'), + new Link('a', 'a/a', new EmptyConstraint()), + new Link('a', 'b/b', new EmptyConstraint()), + new Link('a', 'c/c', new EmptyConstraint()), )); $packages = array(); @@ -878,9 +879,9 @@ EOF; $notAutoloadPackage = new Package('a', '1.0', '1.0'); $requires = array( - new Link('a', 'a/a'), - new Link('a', 'b/b'), - new Link('a', 'c/c'), + new Link('a', 'a/a', new EmptyConstraint()), + new Link('a', 'b/b', new EmptyConstraint()), + new Link('a', 'c/c', new EmptyConstraint()), ); $autoloadPackage->setRequires($requires); $notAutoloadPackage->setRequires($requires); @@ -949,10 +950,10 @@ EOF; $package = new Package('a', '1.0', '1.0'); $package->setAutoload(array('files' => array('root2.php'))); $package->setRequires(array( - new Link('a', 'z/foo'), - new Link('a', 'b/bar'), - new Link('a', 'd/d'), - new Link('a', 'e/e'), + new Link('a', 'z/foo', new EmptyConstraint()), + new Link('a', 'b/bar', new EmptyConstraint()), + new Link('a', 'd/d', new EmptyConstraint()), + new Link('a', 'e/e', new EmptyConstraint()), )); $packages = array(); @@ -963,18 +964,18 @@ EOF; $packages[] = $e = new Package('e/e', '1.0', '1.0'); $z->setAutoload(array('files' => array('testA.php'))); - $z->setRequires(array(new Link('z/foo', 'c/lorem'))); + $z->setRequires(array(new Link('z/foo', 'c/lorem', new EmptyConstraint()))); $b->setAutoload(array('files' => array('testB.php'))); - $b->setRequires(array(new Link('b/bar', 'c/lorem'), new Link('b/bar', 'd/d'))); + $b->setRequires(array(new Link('b/bar', 'c/lorem', new EmptyConstraint()), new Link('b/bar', 'd/d', new EmptyConstraint()))); $c->setAutoload(array('files' => array('testC.php'))); $d->setAutoload(array('files' => array('testD.php'))); - $d->setRequires(array(new Link('d/d', 'c/lorem'))); + $d->setRequires(array(new Link('d/d', 'c/lorem', new EmptyConstraint()))); $e->setAutoload(array('files' => array('testE.php'))); - $e->setRequires(array(new Link('e/e', 'c/lorem'))); + $e->setRequires(array(new Link('e/e', 'c/lorem', new EmptyConstraint()))); $this->repository->expects($this->once()) ->method('getCanonicalPackages') @@ -1022,8 +1023,8 @@ EOF; 'classmap' => array($this->workingDir.'/src'), )); $mainPackage->setRequires(array( - new Link('z', 'a/a'), - new Link('z', 'b/b'), + new Link('z', 'a/a', new EmptyConstraint()), + new Link('z', 'b/b', new EmptyConstraint()), )); $packages = array(); @@ -1285,7 +1286,7 @@ EOF; 'files' => array('test.php'), )); $package->setRequires(array( - new Link('a', 'b/b'), + new Link('a', 'b/b', new EmptyConstraint()), )); $vendorPackage = new Package('b/b', '1.0', '1.0'); diff --git a/tests/Composer/Test/DependencyResolver/RuleTest.php b/tests/Composer/Test/DependencyResolver/RuleTest.php index cd3b74887..0f2face8f 100644 --- a/tests/Composer/Test/DependencyResolver/RuleTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleTest.php @@ -19,6 +19,7 @@ use Composer\DependencyResolver\Pool; use Composer\Package\BasePackage; use Composer\Package\Link; use Composer\Repository\ArrayRepository; +use Composer\Semver\Constraint\EmptyConstraint; use Composer\Test\TestCase; class RuleTest extends TestCase @@ -102,7 +103,7 @@ class RuleTest extends TestCase $repositorySetMock = $this->getMockBuilder('Composer\Repository\RepositorySet')->disableOriginalConstructor()->getMock(); $requestMock = $this->getMockBuilder('Composer\DependencyResolver\Request')->disableOriginalConstructor()->getMock(); - $rule = new GenericRule(array($p1->getId(), -$p2->getId()), Rule::RULE_PACKAGE_REQUIRES, new Link('baz', 'foo', 'relates to', '*')); + $rule = new GenericRule(array($p1->getId(), -$p2->getId()), Rule::RULE_PACKAGE_REQUIRES, new Link('baz', 'foo', new EmptyConstraint(), 'relates to', '*')); $this->assertEquals('baz 1.1 relates to foo * -> satisfiable by foo[2.1].', $rule->getPrettyString($repositorySetMock, $requestMock, $pool, false)); } diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index 56d2d7e5b..a8f91910d 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/tests/Composer/Test/DependencyResolver/SolverTest.php @@ -235,8 +235,8 @@ class SolverTest extends TestCase $this->repo->addPackage($newPackageA = $this->getPackage('A', '1.1')); $this->repo->addPackage($newPackageB = $this->getPackage('B', '1.1')); - $packageA->setRequires(array('b' => new Link('A', 'B', null, 'requires'))); - $newPackageA->setRequires(array('b' => new Link('A', 'B', null, 'requires'))); + $packageA->setRequires(array('b' => new Link('A', 'B', new EmptyConstraint(), 'requires'))); + $newPackageA->setRequires(array('b' => new Link('A', 'B', new EmptyConstraint(), 'requires'))); $this->reposComplete(); diff --git a/tests/Composer/Test/Package/RootAliasPackageTest.php b/tests/Composer/Test/Package/RootAliasPackageTest.php index a5fe9172d..5cfcd7179 100644 --- a/tests/Composer/Test/Package/RootAliasPackageTest.php +++ b/tests/Composer/Test/Package/RootAliasPackageTest.php @@ -14,6 +14,7 @@ namespace Composer\Test\Package; use Composer\Package\Link; use Composer\Package\RootAliasPackage; +use Composer\Semver\Constraint\EmptyConstraint; use Composer\Test\TestCase; use Prophecy\Argument; @@ -26,7 +27,7 @@ class RootAliasPackageTest extends TestCase $alias = new RootAliasPackage($root->reveal(), '1.0', '1.0.0.0'); $this->assertEmpty($alias->getRequires()); - $links = array(new Link('a', 'b', null, 'foo', 'self.version')); + $links = array(new Link('a', 'b', new EmptyConstraint(), 'foo', 'self.version')); $alias->setRequires($links); $this->assertNotEmpty($alias->getRequires()); } @@ -38,7 +39,7 @@ class RootAliasPackageTest extends TestCase $alias = new RootAliasPackage($root->reveal(), '1.0', '1.0.0.0'); $this->assertEmpty($alias->getDevRequires()); - $links = array(new Link('a', 'b', null, 'foo', 'self.version')); + $links = array(new Link('a', 'b', new EmptyConstraint(), 'foo', 'self.version')); $alias->setDevRequires($links); $this->assertNotEmpty($alias->getDevRequires()); } @@ -50,7 +51,7 @@ class RootAliasPackageTest extends TestCase $alias = new RootAliasPackage($root->reveal(), '1.0', '1.0.0.0'); $this->assertEmpty($alias->getConflicts()); - $links = array(new Link('a', 'b', null, 'foo', 'self.version')); + $links = array(new Link('a', 'b', new EmptyConstraint(), 'foo', 'self.version')); $alias->setConflicts($links); $this->assertNotEmpty($alias->getConflicts()); } @@ -62,7 +63,7 @@ class RootAliasPackageTest extends TestCase $alias = new RootAliasPackage($root->reveal(), '1.0', '1.0.0.0'); $this->assertEmpty($alias->getProvides()); - $links = array(new Link('a', 'b', null, 'foo', 'self.version')); + $links = array(new Link('a', 'b', new EmptyConstraint(), 'foo', 'self.version')); $alias->setProvides($links); $this->assertNotEmpty($alias->getProvides()); } @@ -74,7 +75,7 @@ class RootAliasPackageTest extends TestCase $alias = new RootAliasPackage($root->reveal(), '1.0', '1.0.0.0'); $this->assertEmpty($alias->getReplaces()); - $links = array(new Link('a', 'b', null, 'foo', 'self.version')); + $links = array(new Link('a', 'b', new EmptyConstraint(), 'foo', 'self.version')); $alias->setReplaces($links); $this->assertNotEmpty($alias->getReplaces()); } diff --git a/tests/Composer/Test/Repository/InstalledRepositoryTest.php b/tests/Composer/Test/Repository/InstalledRepositoryTest.php index a37adb058..4c63f0670 100644 --- a/tests/Composer/Test/Repository/InstalledRepositoryTest.php +++ b/tests/Composer/Test/Repository/InstalledRepositoryTest.php @@ -16,6 +16,7 @@ use Composer\Repository\InstalledRepository; use Composer\Repository\ArrayRepository; use Composer\Repository\InstalledArrayRepository; use Composer\Package\Link; +use Composer\Semver\Constraint\EmptyConstraint; use Composer\Test\TestCase; class InstalledRepositoryTest extends TestCase @@ -30,8 +31,8 @@ class InstalledRepositoryTest extends TestCase $arrayRepoTwo->addPackage($bar = $this->getPackage('bar', '1')); $arrayRepoTwo->addPackage($bar2 = $this->getPackage('bar', '2')); - $foo->setReplaces(array(new Link('foo', 'provided'))); - $bar2->setProvides(array(new Link('bar', 'provided'))); + $foo->setReplaces(array(new Link('foo', 'provided', new EmptyConstraint()))); + $bar2->setProvides(array(new Link('bar', 'provided', new EmptyConstraint()))); $repo = new InstalledRepository(array($arrayRepoOne, $arrayRepoTwo)); From 26877285add6c2d716b1d7cfdc8dcefab112d79f Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Tue, 5 May 2020 20:27:07 +0200 Subject: [PATCH 23/54] Filter duplicate packages --- src/Composer/DependencyResolver/PoolBuilder.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 634256cb1..f71824c85 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -205,6 +205,16 @@ class PoolBuilder $this->unacceptableFixedPackages = $prePoolCreateEvent->getUnacceptableFixedPackages(); } + // Filter duplicate packages + $presentPackages = array(); + foreach ($this->packages as $i => $package) { + if (isset($presentPackages[$package->getUniqueName()])) { + unset($this->packages[$i]); + } else { + $presentPackages[$package->getUniqueName()] = true; + } + } + $pool = new Pool($this->packages, $this->unacceptableFixedPackages); $this->aliasMap = array(); From 00500f83c0633ca5bc8aa4c72817d9129c6aea57 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Tue, 5 May 2020 20:28:40 +0200 Subject: [PATCH 24/54] Using the current subset branch of @Seldaek to fix subset tests --- composer.json | 8 +++- composer.lock | 43 +++++-------------- .../DependencyResolver/PoolBuilder.php | 6 +-- 3 files changed, 21 insertions(+), 36 deletions(-) diff --git a/composer.json b/composer.json index 981f96afe..f16a3dbb2 100644 --- a/composer.json +++ b/composer.json @@ -86,5 +86,11 @@ "support": { "issues": "https://github.com/composer/composer/issues", "irc": "irc://irc.freenode.org/composer" - } + }, + "repositories": [ + { + "type": "vcs", + "url": "git@github.com:Seldaek/semver.git" + } + ] } diff --git a/composer.lock b/composer.lock index aa50c62df..1f7e0e7f0 100644 --- a/composer.lock +++ b/composer.lock @@ -109,7 +109,16 @@ "Composer\\Semver\\": "src" } }, - "notification-url": "https://packagist.org/downloads/", + "autoload-dev": { + "psr-4": { + "Composer\\Semver\\": "tests" + } + }, + "scripts": { + "test": [ + "phpunit" + ] + }, "license": [ "MIT" ], @@ -441,17 +450,7 @@ "license": [ "MIT" ], - "authors": [ - { - "name": "Jan Sorgalla", - "email": "jsorgalla@gmail.com" - } - ], "description": "A lightweight implementation of CommonJS Promises/A for PHP", - "support": { - "issues": "https://github.com/reactphp/promise/issues", - "source": "https://github.com/reactphp/promise/tree/1.0" - }, "time": "2016-03-07T13:46:50+00:00" }, { @@ -501,10 +500,6 @@ "parser", "validator" ], - "support": { - "issues": "https://github.com/Seldaek/jsonlint/issues", - "source": "https://github.com/Seldaek/jsonlint/tree/master" - }, "funding": [ { "url": "https://github.com/Seldaek", @@ -1051,10 +1046,6 @@ "constructor", "instantiate" ], - "support": { - "issues": "https://github.com/doctrine/instantiator/issues", - "source": "https://github.com/doctrine/instantiator/tree/master" - }, "time": "2015-06-14T21:17:01+00:00" }, { @@ -1104,10 +1095,6 @@ "email": "mike.vanriel@naenius.com" } ], - "support": { - "issues": "https://github.com/phpDocumentor/ReflectionDocBlock/issues", - "source": "https://github.com/phpDocumentor/ReflectionDocBlock/tree/release/2.x" - }, "time": "2016-01-25T08:17:30+00:00" }, { @@ -1239,10 +1226,6 @@ "compare", "equality" ], - "support": { - "issues": "https://github.com/sebastianbergmann/comparator/issues", - "source": "https://github.com/sebastianbergmann/comparator/tree/1.2" - }, "time": "2017-01-29T09:50:25+00:00" }, { @@ -1295,10 +1278,6 @@ "keywords": [ "diff" ], - "support": { - "issues": "https://github.com/sebastianbergmann/diff/issues", - "source": "https://github.com/sebastianbergmann/diff/tree/1.4" - }, "time": "2017-05-22T07:24:03+00:00" }, { @@ -1510,5 +1489,5 @@ "platform-overrides": { "php": "5.3.9" }, - "plugin-api-version": "2.0.0" + "plugin-api-version": "1.1.0" } diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index f71824c85..3d453d49e 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -233,9 +233,9 @@ class PoolBuilder return; } - // No need to load this package with this constraint because it was - // already loaded in one that matches - if ($this->loadedNames[$name]->matches($constraint)) { + // No need to load this package with this constraint because it is + // a subset of the constraint with which we have already loaded packages + if ($constraint->isSubsetOf($this->loadedNames[$name])) { return; } From b00291cf21c09dfb72e23eb9ee60195e94b5c39a Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Tue, 5 May 2020 20:37:48 +0200 Subject: [PATCH 25/54] CS --- src/Composer/DependencyResolver/PoolBuilder.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 3d453d49e..43e51bb3c 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -57,22 +57,18 @@ class PoolBuilder * @var IOInterface */ private $io; - /** * @psalm-var array */ private $aliasMap = array(); - /** * @psalm-var array */ private $packagesToLoad = array(); - /** * @psalm-var array */ private $loadedNames = array(); - /** * @psalm-var Package[] */ From 779b56ab2b1f9aaeb6906033bedc9bdc2535f7cc Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Tue, 5 May 2020 20:40:01 +0200 Subject: [PATCH 26/54] Fixed psalm annotations --- src/Composer/DependencyResolver/PoolBuilder.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 43e51bb3c..965f5e8ff 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -62,11 +62,11 @@ class PoolBuilder */ private $aliasMap = array(); /** - * @psalm-var array + * @psalm-var array */ private $packagesToLoad = array(); /** - * @psalm-var array + * @psalm-var array */ private $loadedNames = array(); /** From 0955d38374f5cdca41f398b73d55c9198cc18d07 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Wed, 6 May 2020 17:14:40 +0200 Subject: [PATCH 27/54] Simplify package loading --- src/Composer/DependencyResolver/PoolBuilder.php | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 965f5e8ff..97c177208 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -255,10 +255,11 @@ class PoolBuilder $this->loadedNames[$name] = $constraint; } - $preLoad = $this->packagesToLoad; + $packageBatch = $this->packagesToLoad; + $this->packagesToLoad = array(); foreach ($repositories as $repository) { - if (empty($this->packagesToLoad)) { + if (empty($packageBatch)) { break; } @@ -267,23 +268,16 @@ class PoolBuilder if ($repository instanceof PlatformRepository || $repository === $request->getLockedRepository()) { continue; } - $result = $repository->loadPackages($this->packagesToLoad, $this->acceptableStabilities, $this->stabilityFlags); + $result = $repository->loadPackages($packageBatch, $this->acceptableStabilities, $this->stabilityFlags); foreach ($result['namesFound'] as $name) { // avoid loading the same package again from other repositories once it has been found - unset($this->packagesToLoad[$name]); + unset($packageBatch[$name]); } foreach ($result['packages'] as $package) { $this->loadPackage($request, $package); } } - - // Make sure we empty the packagesToLoad here as it would result - // in an endless loop if the array remains unchanged. - // This could happen with non-existent packages for example. - if ($preLoad == $this->packagesToLoad) { - $this->packagesToLoad = array(); - } } private function loadPackage(Request $request, PackageInterface $package, $propagateUpdate = true) From c7f10bdd90a05937632a4b38e09881030a01d4be Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Wed, 6 May 2020 17:30:44 +0200 Subject: [PATCH 28/54] Fixed RuleTest --- tests/Composer/Test/DependencyResolver/RuleTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/Composer/Test/DependencyResolver/RuleTest.php b/tests/Composer/Test/DependencyResolver/RuleTest.php index 0f2face8f..2608b4946 100644 --- a/tests/Composer/Test/DependencyResolver/RuleTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleTest.php @@ -103,7 +103,10 @@ class RuleTest extends TestCase $repositorySetMock = $this->getMockBuilder('Composer\Repository\RepositorySet')->disableOriginalConstructor()->getMock(); $requestMock = $this->getMockBuilder('Composer\DependencyResolver\Request')->disableOriginalConstructor()->getMock(); - $rule = new GenericRule(array($p1->getId(), -$p2->getId()), Rule::RULE_PACKAGE_REQUIRES, new Link('baz', 'foo', new EmptyConstraint(), 'relates to', '*')); + $emptyConstraint = new EmptyConstraint(); + $emptyConstraint->setPrettyString('*'); + + $rule = new GenericRule(array($p1->getId(), -$p2->getId()), Rule::RULE_PACKAGE_REQUIRES, new Link('baz', 'foo', $emptyConstraint)); $this->assertEquals('baz 1.1 relates to foo * -> satisfiable by foo[2.1].', $rule->getPrettyString($repositorySetMock, $requestMock, $pool, false)); } From 225a6a0a82871cacaf8cd02223ce35442c074df0 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Wed, 6 May 2020 18:41:11 +0200 Subject: [PATCH 29/54] Improved variable naming --- .../DependencyResolver/PoolBuilder.php | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 97c177208..689964d05 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -68,7 +68,7 @@ class PoolBuilder /** * @psalm-var array */ - private $loadedNames = array(); + private $loadedPackages = array(); /** * @psalm-var Package[] */ @@ -124,11 +124,11 @@ class PoolBuilder } foreach ($request->getFixedPackages() as $package) { - $this->loadedNames[$package->getName()] = new EmptyConstraint(); + $this->loadedPackages[$package->getName()] = new EmptyConstraint(); // replace means conflict, so if a fixed package replaces a name, no need to load that one, packages would conflict anyways foreach ($package->getReplaces() as $link) { - $this->loadedNames[$link->getTarget()] = new EmptyConstraint(); + $this->loadedPackages[$link->getTarget()] = new EmptyConstraint(); } // TODO in how far can we do the above for conflicts? It's more tricky cause conflicts can be limited to @@ -151,7 +151,7 @@ class PoolBuilder // clean up packagesToLoad for anything we manually marked loaded above foreach ($this->packagesToLoad as $name => $constraint) { - if (isset($this->loadedNames[$name])) { + if (isset($this->loadedPackages[$name])) { unset($this->packagesToLoad[$name]); } } @@ -215,7 +215,7 @@ class PoolBuilder $this->aliasMap = array(); $this->packagesToLoad = array(); - $this->loadedNames = array(); + $this->loadedPackages = array(); $this->packages = array(); $this->unacceptableFixedPackages = array(); @@ -224,14 +224,14 @@ class PoolBuilder private function markPackageNameForLoading($name, ConstraintInterface $constraint) { - if (!isset($this->loadedNames[$name])) { + if (!isset($this->loadedPackages[$name])) { $this->packagesToLoad[$name] = $constraint; return; } // No need to load this package with this constraint because it is // a subset of the constraint with which we have already loaded packages - if ($constraint->isSubsetOf($this->loadedNames[$name])) { + if ($constraint->isSubsetOf($this->loadedPackages[$name])) { return; } @@ -245,14 +245,14 @@ class PoolBuilder // We have already loaded that package but not in the constraint that's // required. We extend the constraint and mark that package as not being loaded // yet so we get the required package versions - $this->packagesToLoad[$name] = MultiConstraint::create(array($this->loadedNames[$name], $constraint), false); - unset($this->loadedNames[$name]); + $this->packagesToLoad[$name] = MultiConstraint::create(array($this->loadedPackages[$name], $constraint), false); + unset($this->loadedPackages[$name]); } private function loadPackagesMarkedForLoading(Request $request, $repositories) { foreach ($this->packagesToLoad as $name => $constraint) { - $this->loadedNames[$name] = $constraint; + $this->loadedPackages[$name] = $constraint; } $packageBatch = $this->packagesToLoad; @@ -342,7 +342,7 @@ class PoolBuilder if ($propagateUpdate && $request->getUpdateAllowTransitiveDependencies()) { foreach ($package->getReplaces() as $link) { $replace = $link->getTarget(); - if (isset($this->loadedNames[$replace]) && isset($this->skippedLoad[$replace])) { + if (isset($this->loadedPackages[$replace]) && isset($this->skippedLoad[$replace])) { if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$replace])) { $this->unfixPackage($request, $replace); $this->markPackageNameForLoading($replace, $link->getConstraint()); @@ -439,7 +439,7 @@ class PoolBuilder } unset($this->skippedLoad[$name]); - unset($this->loadedNames[$name]); + unset($this->loadedPackages[$name]); } private function getRootAliasesPerPackage(array $aliases) From 7be24dccd9c1fdb8422b76661ca73a133d5aa286 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Wed, 6 May 2020 22:22:52 +0200 Subject: [PATCH 30/54] Fixed some of the partial update tests --- .../DependencyResolver/PoolBuilder.php | 45 +++++++++++-------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 689964d05..b079628ea 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -124,11 +124,11 @@ class PoolBuilder } foreach ($request->getFixedPackages() as $package) { - $this->loadedPackages[$package->getName()] = new EmptyConstraint(); + $this->loadedPackages[$package->getName()] = new Constraint('==', $package->getVersion()); // replace means conflict, so if a fixed package replaces a name, no need to load that one, packages would conflict anyways foreach ($package->getReplaces() as $link) { - $this->loadedPackages[$link->getTarget()] = new EmptyConstraint(); + $this->loadedPackages[$link->getTarget()] = $link->getConstraint(); } // TODO in how far can we do the above for conflicts? It's more tricky cause conflicts can be limited to @@ -146,6 +146,11 @@ class PoolBuilder } foreach ($request->getRequires() as $packageName => $constraint) { + // fixed packages have already been added, so if a root require needs one of them, no need to do anything + if (isset($this->loadedPackages[$packageName])) { + continue; + } + $this->markPackageNameForLoading($packageName, $constraint); } @@ -224,6 +229,13 @@ class PoolBuilder private function markPackageNameForLoading($name, ConstraintInterface $constraint) { + // Maybe it was already marked before but not loaded yet. In that case + // we have to extend the constraint (we don't check if they match because + // MultiConstraint::create() will optimize anyway + if (isset($this->packagesToLoad[$name]) && !$constraint->isSubsetOf($this->packagesToLoad[$name])) { + $constraint = MultiConstraint::create(array($this->packagesToLoad[$name], $constraint), false); + } + if (!isset($this->loadedPackages[$name])) { $this->packagesToLoad[$name] = $constraint; return; @@ -235,13 +247,6 @@ class PoolBuilder return; } - // Maybe it was already marked before but not loaded yet. In that case - // we have to extend the constraint (we don't check if they match because - // MultiConstraint::create() will optimize anyway - if (isset($this->packagesToLoad[$name])) { - $constraint = MultiConstraint::create(array($this->packagesToLoad[$name], $constraint), false); - } - // We have already loaded that package but not in the constraint that's // required. We extend the constraint and mark that package as not being loaded // yet so we get the required package versions @@ -322,19 +327,21 @@ class PoolBuilder $require = $link->getTarget(); $linkConstraint = $link->getConstraint(); - // if this is a partial update with transitive dependencies we need to unfix the package we now know is a - // dependency of another package which we are trying to update, and then attempt to load it again - if ($propagateUpdate && $request->getUpdateAllowTransitiveDependencies() && isset($this->skippedLoad[$require])) { - if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$require])) { - $this->unfixPackage($request, $require); + if ($propagateUpdate) { + // if this is a partial update with transitive dependencies we need to unfix the package we now know is a + // dependency of another package which we are trying to update, and then attempt to load it again + if ($request->getUpdateAllowTransitiveDependencies() && isset($this->skippedLoad[$require])) { + if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$require])) { + $this->unfixPackage($request, $require); + $this->markPackageNameForLoading($require, $linkConstraint); + } elseif (!$request->getUpdateAllowTransitiveRootDependencies() && $this->isRootRequire($request, $require) && !isset($this->updateAllowWarned[$require])) { + $this->updateAllowWarned[$require] = true; + $this->io->writeError('Dependency "'.$require.'" is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies to include root dependencies.'); + } + } else { $this->markPackageNameForLoading($require, $linkConstraint); - } elseif (!$request->getUpdateAllowTransitiveRootDependencies() && $this->isRootRequire($request, $require) && !isset($this->updateAllowWarned[$require])) { - $this->updateAllowWarned[$require] = true; - $this->io->writeError('Dependency "'.$require.'" is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies to include root dependencies.'); } } - - $this->markPackageNameForLoading($require, $linkConstraint); } // if we're doing a partial update with deps we also need to unfix packages which are being replaced in case they From 5a835db24d4cbaae7680405ae6905e6bdb721aae Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Wed, 6 May 2020 23:31:58 +0200 Subject: [PATCH 31/54] Fixed another partial update test --- src/Composer/DependencyResolver/PoolBuilder.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index b079628ea..eb1403f7d 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -341,6 +341,12 @@ class PoolBuilder } else { $this->markPackageNameForLoading($require, $linkConstraint); } + } else { + // We also need to load the requirements of a fixed package + // unless it was skipped + if (!isset($this->skippedLoad[$require])) { + $this->markPackageNameForLoading($require, $linkConstraint); + } } } From 5097ed64809dd38e098826f88b573f2ef566cc75 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Thu, 7 May 2020 19:45:52 +0200 Subject: [PATCH 32/54] Fixed another test --- src/Composer/DependencyResolver/PoolBuilder.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index eb1403f7d..6230b6092 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -129,6 +129,7 @@ class PoolBuilder // replace means conflict, so if a fixed package replaces a name, no need to load that one, packages would conflict anyways foreach ($package->getReplaces() as $link) { $this->loadedPackages[$link->getTarget()] = $link->getConstraint(); + $this->skippedLoad[$link->getTarget()] = $package->getName(); } // TODO in how far can we do the above for conflicts? It's more tricky cause conflicts can be limited to From 014e9d5dd19fbd4792334e90463820f88654c9d8 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Thu, 7 May 2020 22:25:42 +0200 Subject: [PATCH 33/54] Fixed last remaining pool builder test --- src/Composer/DependencyResolver/PoolBuilder.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 6230b6092..9de2d8e6a 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -124,12 +124,13 @@ class PoolBuilder } foreach ($request->getFixedPackages() as $package) { - $this->loadedPackages[$package->getName()] = new Constraint('==', $package->getVersion()); + // using EmptyConstraint here because fixed packages do not need to retrigger + // loading any packages + $this->loadedPackages[$package->getName()] = new EmptyConstraint(); // replace means conflict, so if a fixed package replaces a name, no need to load that one, packages would conflict anyways foreach ($package->getReplaces() as $link) { - $this->loadedPackages[$link->getTarget()] = $link->getConstraint(); - $this->skippedLoad[$link->getTarget()] = $package->getName(); + $this->loadedPackages[$link->getTarget()] = new EmptyConstraint(); } // TODO in how far can we do the above for conflicts? It's more tricky cause conflicts can be limited to From 6ef47baca1a78293f1b5b73bd18330f2e66f0a74 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Thu, 7 May 2020 22:25:53 +0200 Subject: [PATCH 34/54] Added a todo for package dupes --- src/Composer/DependencyResolver/PoolBuilder.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 9de2d8e6a..724d0a3bf 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -209,6 +209,7 @@ class PoolBuilder } // Filter duplicate packages + // TODO: can we optimize this so that we don't even end up having dupes here? $presentPackages = array(); foreach ($this->packages as $i => $package) { if (isset($presentPackages[$package->getUniqueName()])) { From b6c436598bc984247a140a7adef5c6d8cf7c0b91 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Thu, 7 May 2020 22:38:24 +0200 Subject: [PATCH 35/54] Updated to latest semver intervals --- src/Composer/DependencyResolver/PoolBuilder.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 724d0a3bf..f680dd710 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -27,6 +27,7 @@ use Composer\Semver\Constraint\MultiConstraint; use Composer\EventDispatcher\EventDispatcher; use Composer\Plugin\PrePoolCreateEvent; use Composer\Plugin\PluginEvents; +use Composer\Semver\Intervals; /** * @author Nils Adermann @@ -235,7 +236,7 @@ class PoolBuilder // Maybe it was already marked before but not loaded yet. In that case // we have to extend the constraint (we don't check if they match because // MultiConstraint::create() will optimize anyway - if (isset($this->packagesToLoad[$name]) && !$constraint->isSubsetOf($this->packagesToLoad[$name])) { + if (isset($this->packagesToLoad[$name]) && !Intervals::isSubsetOf($constraint, $this->packagesToLoad[$name])) { $constraint = MultiConstraint::create(array($this->packagesToLoad[$name], $constraint), false); } @@ -246,7 +247,7 @@ class PoolBuilder // No need to load this package with this constraint because it is // a subset of the constraint with which we have already loaded packages - if ($constraint->isSubsetOf($this->loadedPackages[$name])) { + if (Intervals::isSubsetOf($constraint, $this->loadedPackages[$name])) { return; } From cb19347031f319f0ba1105cbe21e2bf74c2aefa6 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 16:17:30 +0200 Subject: [PATCH 36/54] Update to latest master --- composer.json | 8 +--- composer.lock | 43 ++++++++++++++----- .../DependencyResolver/PoolBuilder.php | 6 +-- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/composer.json b/composer.json index f16a3dbb2..981f96afe 100644 --- a/composer.json +++ b/composer.json @@ -86,11 +86,5 @@ "support": { "issues": "https://github.com/composer/composer/issues", "irc": "irc://irc.freenode.org/composer" - }, - "repositories": [ - { - "type": "vcs", - "url": "git@github.com:Seldaek/semver.git" - } - ] + } } diff --git a/composer.lock b/composer.lock index 1f7e0e7f0..aa50c62df 100644 --- a/composer.lock +++ b/composer.lock @@ -109,16 +109,7 @@ "Composer\\Semver\\": "src" } }, - "autoload-dev": { - "psr-4": { - "Composer\\Semver\\": "tests" - } - }, - "scripts": { - "test": [ - "phpunit" - ] - }, + "notification-url": "https://packagist.org/downloads/", "license": [ "MIT" ], @@ -450,7 +441,17 @@ "license": [ "MIT" ], + "authors": [ + { + "name": "Jan Sorgalla", + "email": "jsorgalla@gmail.com" + } + ], "description": "A lightweight implementation of CommonJS Promises/A for PHP", + "support": { + "issues": "https://github.com/reactphp/promise/issues", + "source": "https://github.com/reactphp/promise/tree/1.0" + }, "time": "2016-03-07T13:46:50+00:00" }, { @@ -500,6 +501,10 @@ "parser", "validator" ], + "support": { + "issues": "https://github.com/Seldaek/jsonlint/issues", + "source": "https://github.com/Seldaek/jsonlint/tree/master" + }, "funding": [ { "url": "https://github.com/Seldaek", @@ -1046,6 +1051,10 @@ "constructor", "instantiate" ], + "support": { + "issues": "https://github.com/doctrine/instantiator/issues", + "source": "https://github.com/doctrine/instantiator/tree/master" + }, "time": "2015-06-14T21:17:01+00:00" }, { @@ -1095,6 +1104,10 @@ "email": "mike.vanriel@naenius.com" } ], + "support": { + "issues": "https://github.com/phpDocumentor/ReflectionDocBlock/issues", + "source": "https://github.com/phpDocumentor/ReflectionDocBlock/tree/release/2.x" + }, "time": "2016-01-25T08:17:30+00:00" }, { @@ -1226,6 +1239,10 @@ "compare", "equality" ], + "support": { + "issues": "https://github.com/sebastianbergmann/comparator/issues", + "source": "https://github.com/sebastianbergmann/comparator/tree/1.2" + }, "time": "2017-01-29T09:50:25+00:00" }, { @@ -1278,6 +1295,10 @@ "keywords": [ "diff" ], + "support": { + "issues": "https://github.com/sebastianbergmann/diff/issues", + "source": "https://github.com/sebastianbergmann/diff/tree/1.4" + }, "time": "2017-05-22T07:24:03+00:00" }, { @@ -1489,5 +1510,5 @@ "platform-overrides": { "php": "5.3.9" }, - "plugin-api-version": "1.1.0" + "plugin-api-version": "2.0.0" } diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index f680dd710..8b602b1aa 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -125,13 +125,13 @@ class PoolBuilder } foreach ($request->getFixedPackages() as $package) { - // using EmptyConstraint here because fixed packages do not need to retrigger + // using MatchAllConstraint here because fixed packages do not need to retrigger // loading any packages - $this->loadedPackages[$package->getName()] = new EmptyConstraint(); + $this->loadedPackages[$package->getName()] = new MatchAllConstraint(); // replace means conflict, so if a fixed package replaces a name, no need to load that one, packages would conflict anyways foreach ($package->getReplaces() as $link) { - $this->loadedPackages[$link->getTarget()] = new EmptyConstraint(); + $this->loadedPackages[$link->getTarget()] = new MatchAllConstraint(); } // TODO in how far can we do the above for conflicts? It's more tricky cause conflicts can be limited to From 67a88880ec136a3e02f10235a79ba6b04667a7de Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 16:41:37 +0200 Subject: [PATCH 37/54] Get rid of EmptyConstraint --- src/Composer/DependencyResolver/Request.php | 4 +- src/Composer/Package/Link.php | 1 - .../Test/Autoload/AutoloadGeneratorTest.php | 88 +++++++++---------- .../Test/DependencyResolver/RequestTest.php | 4 +- .../Test/DependencyResolver/RuleTest.php | 4 +- .../Test/DependencyResolver/SolverTest.php | 10 +-- .../Test/Package/RootAliasPackageTest.php | 12 +-- .../Repository/InstalledRepositoryTest.php | 6 +- .../Composer/Test/Util/PackageSorterTest.php | 3 +- 9 files changed, 66 insertions(+), 66 deletions(-) diff --git a/src/Composer/DependencyResolver/Request.php b/src/Composer/DependencyResolver/Request.php index 959ba884b..332ead628 100644 --- a/src/Composer/DependencyResolver/Request.php +++ b/src/Composer/DependencyResolver/Request.php @@ -17,7 +17,7 @@ use Composer\Package\PackageInterface; use Composer\Package\RootAliasPackage; use Composer\Repository\LockArrayRepository; use Composer\Semver\Constraint\ConstraintInterface; -use Composer\Semver\Constraint\EmptyConstraint; +use Composer\Semver\Constraint\MatchAllConstraint; /** * @author Nils Adermann @@ -56,7 +56,7 @@ class Request public function requireName($packageName, ConstraintInterface $constraint = null) { $packageName = strtolower($packageName); - $this->requires[$packageName] = $constraint ? $constraint : new EmptyConstraint(); + $this->requires[$packageName] = $constraint ? $constraint : new MatchAllConstraint(); } /** diff --git a/src/Composer/Package/Link.php b/src/Composer/Package/Link.php index 16254f68e..81ca86e8b 100644 --- a/src/Composer/Package/Link.php +++ b/src/Composer/Package/Link.php @@ -13,7 +13,6 @@ namespace Composer\Package; use Composer\Semver\Constraint\ConstraintInterface; -use Composer\Semver\Constraint\EmptyConstraint; /** * Represents a link between two packages, represented by their names diff --git a/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php b/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php index 896221c13..35dcd37da 100644 --- a/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php +++ b/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php @@ -16,7 +16,7 @@ use Composer\Autoload\AutoloadGenerator; use Composer\Package\Link; use Composer\Package\Version\VersionParser; use Composer\Semver\Constraint\Constraint; -use Composer\Semver\Constraint\EmptyConstraint; +use Composer\Semver\Constraint\MatchAllConstraint; use Composer\Util\Filesystem; use Composer\Package\AliasPackage; use Composer\Package\Package; @@ -367,8 +367,8 @@ class AutoloadGeneratorTest extends TestCase { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a', new EmptyConstraint()), - new Link('a', 'b/b', new EmptyConstraint()), + new Link('a', 'a/a', new MatchAllConstraint()), + new Link('a', 'b/b', new MatchAllConstraint()), )); $packages = array(); @@ -396,7 +396,7 @@ class AutoloadGeneratorTest extends TestCase { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a', new EmptyConstraint()), + new Link('a', 'a/a', new MatchAllConstraint()), )); $packages = array(); @@ -404,11 +404,11 @@ class AutoloadGeneratorTest extends TestCase $packages[] = $b = new Package('b/b', '1.0', '1.0'); $a->setAutoload(array('psr-0' => array('A' => 'src/', 'A\\B' => 'lib/'))); $a->setRequires(array( - new Link('a/a', 'b/b', new EmptyConstraint()), + new Link('a/a', 'b/b', new MatchAllConstraint()), )); $b->setAutoload(array('psr-0' => array('B\\Sub\\Name' => 'src/'))); $b->setRequires(array( - new Link('b/b', 'a/a', new EmptyConstraint()), + new Link('b/b', 'a/a', new MatchAllConstraint()), )); $this->repository->expects($this->once()) @@ -428,13 +428,13 @@ class AutoloadGeneratorTest extends TestCase public function testNonDevAutoloadShouldIncludeReplacedPackages() { $package = new Package('a', '1.0', '1.0'); - $package->setRequires(array(new Link('a', 'a/a', new EmptyConstraint()))); + $package->setRequires(array(new Link('a', 'a/a', new MatchAllConstraint()))); $packages = array(); $packages[] = $a = new Package('a/a', '1.0', '1.0'); $packages[] = $b = new Package('b/b', '1.0', '1.0'); - $a->setRequires(array(new Link('a/a', 'b/c', new EmptyConstraint()))); + $a->setRequires(array(new Link('a/a', 'b/c', new MatchAllConstraint()))); $b->setAutoload(array('psr-4' => array('B\\' => 'src/'))); $b->setReplaces( @@ -463,7 +463,7 @@ class AutoloadGeneratorTest extends TestCase { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a', new EmptyConstraint()), + new Link('a', 'a/a', new MatchAllConstraint()), )); $packages = array(); @@ -471,11 +471,11 @@ class AutoloadGeneratorTest extends TestCase $packages[] = $b = new Package('b/b', '1.0', '1.0'); $a->setAutoload(array('psr-0' => array('A' => 'src/', 'A\\B' => 'lib/'))); $a->setRequires(array( - new Link('a/a', 'c/c', new EmptyConstraint()), + new Link('a/a', 'c/c', new MatchAllConstraint()), )); $b->setAutoload(array('psr-0' => array('B\\Sub\\Name' => 'src/'))); $b->setReplaces(array( - new Link('b/b', 'c/c', new EmptyConstraint()), + new Link('b/b', 'c/c', new MatchAllConstraint()), )); $this->repository->expects($this->once()) @@ -496,7 +496,7 @@ class AutoloadGeneratorTest extends TestCase { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a', new EmptyConstraint()) + new Link('a', 'a/a', new MatchAllConstraint()) )); $packages = array(); @@ -507,18 +507,18 @@ class AutoloadGeneratorTest extends TestCase $packages[] = $e = new Package('e/e', '1.0', '1.0'); $a->setAutoload(array('classmap' => array('src/A.php'))); $a->setRequires(array( - new Link('a/a', 'b/b', new EmptyConstraint()) + new Link('a/a', 'b/b', new MatchAllConstraint()) )); $b->setAutoload(array('classmap' => array('src/B.php'))); $b->setRequires(array( - new Link('b/b', 'e/e', new EmptyConstraint()) + new Link('b/b', 'e/e', new MatchAllConstraint()) )); $c->setAutoload(array('classmap' => array('src/C.php'))); $c->setReplaces(array( - new Link('c/c', 'b/b', new EmptyConstraint()) + new Link('c/c', 'b/b', new MatchAllConstraint()) )); $c->setRequires(array( - new Link('c/c', 'd/d', new EmptyConstraint()) + new Link('c/c', 'd/d', new MatchAllConstraint()) )); $d->setAutoload(array('classmap' => array('src/D.php'))); $e->setAutoload(array('classmap' => array('src/E.php'))); @@ -548,7 +548,7 @@ class AutoloadGeneratorTest extends TestCase { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a', new EmptyConstraint()), + new Link('a', 'a/a', new MatchAllConstraint()), )); $package->setAutoload(array( @@ -653,8 +653,8 @@ EOF; { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a', new EmptyConstraint()), - new Link('a', 'b/b', new EmptyConstraint()), + new Link('a', 'a/a', new MatchAllConstraint()), + new Link('a', 'b/b', new MatchAllConstraint()), )); $packages = array(); @@ -693,8 +693,8 @@ EOF; { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a', new EmptyConstraint()), - new Link('a', 'b/b', new EmptyConstraint()), + new Link('a', 'a/a', new MatchAllConstraint()), + new Link('a', 'b/b', new MatchAllConstraint()), )); $packages = array(); @@ -733,9 +733,9 @@ EOF; { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a', new EmptyConstraint()), - new Link('a', 'b/b', new EmptyConstraint()), - new Link('a', 'c/c', new EmptyConstraint()), + new Link('a', 'a/a', new MatchAllConstraint()), + new Link('a', 'b/b', new MatchAllConstraint()), + new Link('a', 'c/c', new MatchAllConstraint()), )); $packages = array(); @@ -778,9 +778,9 @@ EOF; { $package = new Package('a', '1.0', '1.0'); $package->setRequires(array( - new Link('a', 'a/a', new EmptyConstraint()), - new Link('a', 'b/b', new EmptyConstraint()), - new Link('a', 'c/c', new EmptyConstraint()), + new Link('a', 'a/a', new MatchAllConstraint()), + new Link('a', 'b/b', new MatchAllConstraint()), + new Link('a', 'c/c', new MatchAllConstraint()), )); $packages = array(); @@ -828,9 +828,9 @@ EOF; $package = new Package('a', '1.0', '1.0'); $package->setAutoload(array('files' => array('root.php'))); $package->setRequires(array( - new Link('a', 'a/a', new EmptyConstraint()), - new Link('a', 'b/b', new EmptyConstraint()), - new Link('a', 'c/c', new EmptyConstraint()), + new Link('a', 'a/a', new MatchAllConstraint()), + new Link('a', 'b/b', new MatchAllConstraint()), + new Link('a', 'c/c', new MatchAllConstraint()), )); $packages = array(); @@ -879,9 +879,9 @@ EOF; $notAutoloadPackage = new Package('a', '1.0', '1.0'); $requires = array( - new Link('a', 'a/a', new EmptyConstraint()), - new Link('a', 'b/b', new EmptyConstraint()), - new Link('a', 'c/c', new EmptyConstraint()), + new Link('a', 'a/a', new MatchAllConstraint()), + new Link('a', 'b/b', new MatchAllConstraint()), + new Link('a', 'c/c', new MatchAllConstraint()), ); $autoloadPackage->setRequires($requires); $notAutoloadPackage->setRequires($requires); @@ -950,10 +950,10 @@ EOF; $package = new Package('a', '1.0', '1.0'); $package->setAutoload(array('files' => array('root2.php'))); $package->setRequires(array( - new Link('a', 'z/foo', new EmptyConstraint()), - new Link('a', 'b/bar', new EmptyConstraint()), - new Link('a', 'd/d', new EmptyConstraint()), - new Link('a', 'e/e', new EmptyConstraint()), + new Link('a', 'z/foo', new MatchAllConstraint()), + new Link('a', 'b/bar', new MatchAllConstraint()), + new Link('a', 'd/d', new MatchAllConstraint()), + new Link('a', 'e/e', new MatchAllConstraint()), )); $packages = array(); @@ -964,18 +964,18 @@ EOF; $packages[] = $e = new Package('e/e', '1.0', '1.0'); $z->setAutoload(array('files' => array('testA.php'))); - $z->setRequires(array(new Link('z/foo', 'c/lorem', new EmptyConstraint()))); + $z->setRequires(array(new Link('z/foo', 'c/lorem', new MatchAllConstraint()))); $b->setAutoload(array('files' => array('testB.php'))); - $b->setRequires(array(new Link('b/bar', 'c/lorem', new EmptyConstraint()), new Link('b/bar', 'd/d', new EmptyConstraint()))); + $b->setRequires(array(new Link('b/bar', 'c/lorem', new MatchAllConstraint()), new Link('b/bar', 'd/d', new MatchAllConstraint()))); $c->setAutoload(array('files' => array('testC.php'))); $d->setAutoload(array('files' => array('testD.php'))); - $d->setRequires(array(new Link('d/d', 'c/lorem', new EmptyConstraint()))); + $d->setRequires(array(new Link('d/d', 'c/lorem', new MatchAllConstraint()))); $e->setAutoload(array('files' => array('testE.php'))); - $e->setRequires(array(new Link('e/e', 'c/lorem', new EmptyConstraint()))); + $e->setRequires(array(new Link('e/e', 'c/lorem', new MatchAllConstraint()))); $this->repository->expects($this->once()) ->method('getCanonicalPackages') @@ -1023,8 +1023,8 @@ EOF; 'classmap' => array($this->workingDir.'/src'), )); $mainPackage->setRequires(array( - new Link('z', 'a/a', new EmptyConstraint()), - new Link('z', 'b/b', new EmptyConstraint()), + new Link('z', 'a/a', new MatchAllConstraint()), + new Link('z', 'b/b', new MatchAllConstraint()), )); $packages = array(); @@ -1286,7 +1286,7 @@ EOF; 'files' => array('test.php'), )); $package->setRequires(array( - new Link('a', 'b/b', new EmptyConstraint()), + new Link('a', 'b/b', new MatchAllConstraint()), )); $vendorPackage = new Package('b/b', '1.0', '1.0'); diff --git a/tests/Composer/Test/DependencyResolver/RequestTest.php b/tests/Composer/Test/DependencyResolver/RequestTest.php index 77955597a..a93bc404b 100644 --- a/tests/Composer/Test/DependencyResolver/RequestTest.php +++ b/tests/Composer/Test/DependencyResolver/RequestTest.php @@ -14,7 +14,7 @@ namespace Composer\Test\DependencyResolver; use Composer\DependencyResolver\Request; use Composer\Repository\ArrayRepository; -use Composer\Semver\Constraint\EmptyConstraint; +use Composer\Semver\Constraint\MatchAllConstraint; use Composer\Test\TestCase; class RequestTest extends TestCase @@ -35,7 +35,7 @@ class RequestTest extends TestCase $this->assertEquals( array( - 'foo' => new EmptyConstraint(), + 'foo' => new MatchAllConstraint(), ), $request->getRequires() ); diff --git a/tests/Composer/Test/DependencyResolver/RuleTest.php b/tests/Composer/Test/DependencyResolver/RuleTest.php index 2608b4946..ebf15fcac 100644 --- a/tests/Composer/Test/DependencyResolver/RuleTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleTest.php @@ -19,7 +19,7 @@ use Composer\DependencyResolver\Pool; use Composer\Package\BasePackage; use Composer\Package\Link; use Composer\Repository\ArrayRepository; -use Composer\Semver\Constraint\EmptyConstraint; +use Composer\Semver\Constraint\MatchAllConstraint; use Composer\Test\TestCase; class RuleTest extends TestCase @@ -103,7 +103,7 @@ class RuleTest extends TestCase $repositorySetMock = $this->getMockBuilder('Composer\Repository\RepositorySet')->disableOriginalConstructor()->getMock(); $requestMock = $this->getMockBuilder('Composer\DependencyResolver\Request')->disableOriginalConstructor()->getMock(); - $emptyConstraint = new EmptyConstraint(); + $emptyConstraint = new MatchAllConstraint(); $emptyConstraint->setPrettyString('*'); $rule = new GenericRule(array($p1->getId(), -$p2->getId()), Rule::RULE_PACKAGE_REQUIRES, new Link('baz', 'foo', $emptyConstraint)); diff --git a/tests/Composer/Test/DependencyResolver/SolverTest.php b/tests/Composer/Test/DependencyResolver/SolverTest.php index a8f91910d..6ed4c6bbd 100644 --- a/tests/Composer/Test/DependencyResolver/SolverTest.php +++ b/tests/Composer/Test/DependencyResolver/SolverTest.php @@ -235,8 +235,8 @@ class SolverTest extends TestCase $this->repo->addPackage($newPackageA = $this->getPackage('A', '1.1')); $this->repo->addPackage($newPackageB = $this->getPackage('B', '1.1')); - $packageA->setRequires(array('b' => new Link('A', 'B', new EmptyConstraint(), 'requires'))); - $newPackageA->setRequires(array('b' => new Link('A', 'B', new EmptyConstraint(), 'requires'))); + $packageA->setRequires(array('b' => new Link('A', 'B', new MatchAllConstraint(), 'requires'))); + $newPackageA->setRequires(array('b' => new Link('A', 'B', new MatchAllConstraint(), 'requires'))); $this->reposComplete(); @@ -639,7 +639,7 @@ class SolverTest extends TestCase $this->reposComplete(); - $emptyConstraint = new EmptyConstraint(); + $emptyConstraint = new MatchAllConstraint(); $emptyConstraint->setPrettyString('*'); $this->request->requireName('A', $emptyConstraint); @@ -686,7 +686,7 @@ class SolverTest extends TestCase $msg = "\n"; $msg .= " Problem 1\n"; - $msg .= " - Root composer.json requires a [] -> satisfiable by A[1.0].\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 the constraint.\n"; $this->assertEquals($msg, $e->getPrettyString($this->repoSet, $this->request, $this->pool, false)); } @@ -715,7 +715,7 @@ class SolverTest extends TestCase $this->reposComplete(); - $emptyConstraint = new EmptyConstraint(); + $emptyConstraint = new MatchAllConstraint(); $emptyConstraint->setPrettyString('*'); $this->request->requireName('A', $emptyConstraint); diff --git a/tests/Composer/Test/Package/RootAliasPackageTest.php b/tests/Composer/Test/Package/RootAliasPackageTest.php index 5cfcd7179..459f935e5 100644 --- a/tests/Composer/Test/Package/RootAliasPackageTest.php +++ b/tests/Composer/Test/Package/RootAliasPackageTest.php @@ -14,7 +14,7 @@ namespace Composer\Test\Package; use Composer\Package\Link; use Composer\Package\RootAliasPackage; -use Composer\Semver\Constraint\EmptyConstraint; +use Composer\Semver\Constraint\MatchAllConstraint; use Composer\Test\TestCase; use Prophecy\Argument; @@ -27,7 +27,7 @@ class RootAliasPackageTest extends TestCase $alias = new RootAliasPackage($root->reveal(), '1.0', '1.0.0.0'); $this->assertEmpty($alias->getRequires()); - $links = array(new Link('a', 'b', new EmptyConstraint(), 'foo', 'self.version')); + $links = array(new Link('a', 'b', new MatchAllConstraint(), 'foo', 'self.version')); $alias->setRequires($links); $this->assertNotEmpty($alias->getRequires()); } @@ -39,7 +39,7 @@ class RootAliasPackageTest extends TestCase $alias = new RootAliasPackage($root->reveal(), '1.0', '1.0.0.0'); $this->assertEmpty($alias->getDevRequires()); - $links = array(new Link('a', 'b', new EmptyConstraint(), 'foo', 'self.version')); + $links = array(new Link('a', 'b', new MatchAllConstraint(), 'foo', 'self.version')); $alias->setDevRequires($links); $this->assertNotEmpty($alias->getDevRequires()); } @@ -51,7 +51,7 @@ class RootAliasPackageTest extends TestCase $alias = new RootAliasPackage($root->reveal(), '1.0', '1.0.0.0'); $this->assertEmpty($alias->getConflicts()); - $links = array(new Link('a', 'b', new EmptyConstraint(), 'foo', 'self.version')); + $links = array(new Link('a', 'b', new MatchAllConstraint(), 'foo', 'self.version')); $alias->setConflicts($links); $this->assertNotEmpty($alias->getConflicts()); } @@ -63,7 +63,7 @@ class RootAliasPackageTest extends TestCase $alias = new RootAliasPackage($root->reveal(), '1.0', '1.0.0.0'); $this->assertEmpty($alias->getProvides()); - $links = array(new Link('a', 'b', new EmptyConstraint(), 'foo', 'self.version')); + $links = array(new Link('a', 'b', new MatchAllConstraint(), 'foo', 'self.version')); $alias->setProvides($links); $this->assertNotEmpty($alias->getProvides()); } @@ -75,7 +75,7 @@ class RootAliasPackageTest extends TestCase $alias = new RootAliasPackage($root->reveal(), '1.0', '1.0.0.0'); $this->assertEmpty($alias->getReplaces()); - $links = array(new Link('a', 'b', new EmptyConstraint(), 'foo', 'self.version')); + $links = array(new Link('a', 'b', new MatchAllConstraint(), 'foo', 'self.version')); $alias->setReplaces($links); $this->assertNotEmpty($alias->getReplaces()); } diff --git a/tests/Composer/Test/Repository/InstalledRepositoryTest.php b/tests/Composer/Test/Repository/InstalledRepositoryTest.php index 4c63f0670..505278c3b 100644 --- a/tests/Composer/Test/Repository/InstalledRepositoryTest.php +++ b/tests/Composer/Test/Repository/InstalledRepositoryTest.php @@ -16,7 +16,7 @@ use Composer\Repository\InstalledRepository; use Composer\Repository\ArrayRepository; use Composer\Repository\InstalledArrayRepository; use Composer\Package\Link; -use Composer\Semver\Constraint\EmptyConstraint; +use Composer\Semver\Constraint\MatchAllConstraint; use Composer\Test\TestCase; class InstalledRepositoryTest extends TestCase @@ -31,8 +31,8 @@ class InstalledRepositoryTest extends TestCase $arrayRepoTwo->addPackage($bar = $this->getPackage('bar', '1')); $arrayRepoTwo->addPackage($bar2 = $this->getPackage('bar', '2')); - $foo->setReplaces(array(new Link('foo', 'provided', new EmptyConstraint()))); - $bar2->setProvides(array(new Link('bar', 'provided', new EmptyConstraint()))); + $foo->setReplaces(array(new Link('foo', 'provided', new MatchAllConstraint()))); + $bar2->setProvides(array(new Link('bar', 'provided', new MatchAllConstraint()))); $repo = new InstalledRepository(array($arrayRepoOne, $arrayRepoTwo)); diff --git a/tests/Composer/Test/Util/PackageSorterTest.php b/tests/Composer/Test/Util/PackageSorterTest.php index e93503436..8704a3af0 100644 --- a/tests/Composer/Test/Util/PackageSorterTest.php +++ b/tests/Composer/Test/Util/PackageSorterTest.php @@ -16,6 +16,7 @@ use Composer\Package\Link; use Composer\Package\Package; use Composer\Test\TestCase; use Composer\Util\PackageSorter; +use Composer\Semver\Constraint\MatchAllConstraint; class PackageSorterTest extends TestCase { @@ -120,7 +121,7 @@ class PackageSorterTest extends TestCase $links = array(); foreach ($requires as $requireName) { - $links[] = new Link($package->getName(), $requireName); + $links[] = new Link($package->getName(), $requireName, new MatchAllConstraint); } $package->setRequires($links); From f2befc46c9f68524bf2321b20a44e5cfa42a82a0 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 17:13:39 +0200 Subject: [PATCH 38/54] Avoid storing duplicate packages when loading the same package twice --- .../DependencyResolver/PoolBuilder.php | 20 ++++++++----------- ...t-expansion-works-with-exact-versions.test | 2 +- ...-not-loaded-if-not-required-expansion.test | 2 +- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 8b602b1aa..9f8f4280e 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -209,17 +209,6 @@ class PoolBuilder $this->unacceptableFixedPackages = $prePoolCreateEvent->getUnacceptableFixedPackages(); } - // Filter duplicate packages - // TODO: can we optimize this so that we don't even end up having dupes here? - $presentPackages = array(); - foreach ($this->packages as $i => $package) { - if (isset($presentPackages[$package->getUniqueName()])) { - unset($this->packages[$i]); - } else { - $presentPackages[$package->getUniqueName()] = true; - } - } - $pool = new Pool($this->packages, $this->unacceptableFixedPackages); $this->aliasMap = array(); @@ -235,7 +224,7 @@ class PoolBuilder { // Maybe it was already marked before but not loaded yet. In that case // we have to extend the constraint (we don't check if they match because - // MultiConstraint::create() will optimize anyway + // MultiConstraint::create() will optimize anyway) if (isset($this->packagesToLoad[$name]) && !Intervals::isSubsetOf($constraint, $this->packagesToLoad[$name])) { $constraint = MultiConstraint::create(array($this->packagesToLoad[$name], $constraint), false); } @@ -256,6 +245,13 @@ class PoolBuilder // yet so we get the required package versions $this->packagesToLoad[$name] = MultiConstraint::create(array($this->loadedPackages[$name], $constraint), false); unset($this->loadedPackages[$name]); + + // remove all already-loaded packages matching those to be loaded to avoid duplicates + foreach ($this->packages as $index => $pkg) { + if ($pkg->getName() === $name) { + unset($this->packages[$index]); + } + } } private function loadPackagesMarkedForLoading(Request $request, $repositories) diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test index 7a3046ae0..fdabd9c8f 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test @@ -23,7 +23,7 @@ Tests if version constraint is expanded. If not, dep/dep 3.0.0 would not be load --EXPECT-- [ "root/req-1.0.0.0", - "dep/dep-2.3.4.0", "dep/dep2-2.3.4.0", + "dep/dep-2.3.4.0", "dep/dep-2.3.5.0" ] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test index 198df20a4..53795c761 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test @@ -25,8 +25,8 @@ Tests if version constraint is expanded. If not, dep/dep 3.0.0 would not be load --EXPECT-- [ "root/req-1.0.0.0", + "dep/dep2-2.3.4.0", "dep/dep-2.3.4.0", "dep/dep-2.3.5.0", - "dep/dep2-2.3.4.0", "dep/dep-3.0.0.0" ] From b7f1550896759baeb52bc94ebeab6ce42f0162d1 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sat, 6 Jun 2020 15:19:55 +0200 Subject: [PATCH 39/54] Add test covering replacer unfixing --- .../partial-update-transitive-deps-unfix.test | 6 +-- ...artial-update-unfixing-with-replacers.test | 50 +++++++++++++++++++ .../DependencyResolver/PoolBuilderTest.php | 9 +++- 3 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-with-replacers.test diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test index 7e0cf2d2d..e075c2df2 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test @@ -8,9 +8,9 @@ Partially updating one root requirement with transitive deps fully updates anoth "root/dep": "*" }, "locked": [ - {"name": "root/update", "version": "1.0.1", "require": {"dep/dep": "1.*"}, "id": 1}, - {"name": "dep/dep", "version": "1.0.2", "require": {"root/dep": "1.*"}, "id": 2}, - {"name": "root/dep", "version": "1.0.3", "id": 3} + {"name": "root/update", "version": "1.0.1", "require": {"dep/dep": "1.*"}}, + {"name": "dep/dep", "version": "1.0.2", "require": {"root/dep": "1.*"}}, + {"name": "root/dep", "version": "1.0.3"} ], "allowList": [ "root/update" diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-with-replacers.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-with-replacers.test new file mode 100644 index 000000000..e7cbf742c --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-with-replacers.test @@ -0,0 +1,50 @@ +--TEST-- +Fixed packages and replacers get unfixed correctly (refs https://github.com/composer/composer/pull/8942) + +--REQUEST-- +{ + "require": { + "root/req1": "*", + "root/req3": "*" + }, + "locked": [ + {"name": "root/req1", "version": "1.0.0", "require": {"replacer/pkg": "1.*"}}, + {"name": "root/req3", "version": "1.0.0", "require": {"replaced/pkg": "1.*", "dep/dep": "2.*"}}, + {"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "*"}}, + {"name": "dep/dep", "version": "2.3.5"} + ], + "allowList": [ + "root/req1" + ], + "allowTransitiveDeps": true +} + +--FIXED-- +[ +] + +--PACKAGES-- +[ + {"name": "root/req1", "version": "1.0.0", "require": {"replacer/pkg": "1.*"}}, + {"name": "root/req1", "version": "1.1.0", "require": {"replacer/pkg": "1.*"}}, + {"name": "root/req3", "version": "1.0.0", "require": {"replaced/pkg": "1.*"}}, + {"name": "root/req3", "version": "1.1.0", "require": {"replaced/pkg": "1.*"}}, + {"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "*"}}, + {"name": "replacer/pkg", "version": "1.1.0", "replace": {"replaced/pkg": "*"}}, + {"name": "replaced/pkg", "version": "1.2.3"}, + {"name": "replaced/pkg", "version": "1.2.4"}, + {"name": "dep/dep", "version": "2.3.5"}, + {"name": "dep/dep", "version": "2.3.6"} +] + +--EXPECT-- +[ + "root/req3-1.0.0.0 (locked)", + "dep/dep-2.3.5.0 (locked)", + "root/req1-1.0.0.0", + "root/req1-1.1.0.0", + "replacer/pkg-1.0.0.0", + "replacer/pkg-1.1.0.0", + "replaced/pkg-1.2.3.0", + "replaced/pkg-1.2.4.0" +] diff --git a/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php b/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php index f02b99fc9..b756219a2 100644 --- a/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php +++ b/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php @@ -113,11 +113,16 @@ class PoolBuilderTest extends TestCase return $id; } + $suffix = ''; + if ($package->getRepository() instanceof LockArrayRepository) { + $suffix = ' (locked)'; + } + if ($package instanceof AliasPackage && $id = array_search($package->getAliasOf(), $packageIds, true)) { - return (string) $package->getName().'-'.$package->getVersion() .' alias of '.$id; + return (string) $package->getName().'-'.$package->getVersion() .' alias of '.$id . $suffix; } - return (string) $package; + return (string) $package . $suffix; }, $result); $this->assertSame($expect, $result); From 4b9b499ce5f7bee87a23fb49591a9078c92e9829 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sat, 6 Jun 2020 17:16:54 +0200 Subject: [PATCH 40/54] Fix issue loading aliases and fix markPackageNameForLoading when called twice in a row for same package it would overwrite the constraint the second time --- .../DependencyResolver/PoolBuilder.php | 70 ++++++++++++------- .../alias-priority-conflicting.test | 61 ++++++++++++++++ .../poolbuilder/alias-with-reference.test | 57 +++++++++++++++ ...minimum-stability-and-filter-packages.test | 2 +- .../DependencyResolver/PoolBuilderTest.php | 18 +++-- 5 files changed, 175 insertions(+), 33 deletions(-) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 9f8f4280e..c6ea7d73d 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -85,6 +85,8 @@ class PoolBuilder */ private $updateAllowWarned = array(); + private $indexCounter = 0; + /** * @param int[] $acceptableStabilities array of stability => BasePackage::STABILITY_* value * @psalm-param array $acceptableStabilities @@ -154,7 +156,7 @@ class PoolBuilder continue; } - $this->markPackageNameForLoading($packageName, $constraint); + $this->markPackageNameForLoading($request, $packageName, $constraint); } // clean up packagesToLoad for anything we manually marked loaded above @@ -216,19 +218,28 @@ class PoolBuilder $this->loadedPackages = array(); $this->packages = array(); $this->unacceptableFixedPackages = array(); + $this->indexCounter = 0; return $pool; } - private function markPackageNameForLoading($name, ConstraintInterface $constraint) + private function markPackageNameForLoading(Request $request, $name, ConstraintInterface $constraint) { // Maybe it was already marked before but not loaded yet. In that case // we have to extend the constraint (we don't check if they match because // MultiConstraint::create() will optimize anyway) - if (isset($this->packagesToLoad[$name]) && !Intervals::isSubsetOf($constraint, $this->packagesToLoad[$name])) { + if (isset($this->packagesToLoad[$name])) { + // Already marked for loading and this does not expand the constraint to be loaded, nothing to do + if (Intervals::isSubsetOf($constraint, $this->packagesToLoad[$name])) { + return; + } + + // extend the constraint to be loaded $constraint = MultiConstraint::create(array($this->packagesToLoad[$name], $constraint), false); } + // Not yet loaded or already marked for a reload, override the existing constraint + // (either it's a new one to load, or it has already been extended above) if (!isset($this->loadedPackages[$name])) { $this->packagesToLoad[$name] = $constraint; return; @@ -245,18 +256,18 @@ class PoolBuilder // yet so we get the required package versions $this->packagesToLoad[$name] = MultiConstraint::create(array($this->loadedPackages[$name], $constraint), false); unset($this->loadedPackages[$name]); - - // remove all already-loaded packages matching those to be loaded to avoid duplicates - foreach ($this->packages as $index => $pkg) { - if ($pkg->getName() === $name) { - unset($this->packages[$index]); - } - } } private function loadPackagesMarkedForLoading(Request $request, $repositories) { foreach ($this->packagesToLoad as $name => $constraint) { + // remove all already-loaded packages matching those to be loaded to avoid duplicates + foreach ($this->packages as $index => $pkg) { + if ($pkg->getName() === $name) { + $this->removeLoadedPackage($request, $pkg, $index); + } + } + $this->loadedPackages[$name] = $constraint; } @@ -287,9 +298,8 @@ class PoolBuilder private function loadPackage(Request $request, PackageInterface $package, $propagateUpdate = true) { - end($this->packages); - $index = key($this->packages) + 1; - $this->packages[] = $package; + $index = $this->indexCounter++; + $this->packages[$index] = $package; if ($package instanceof AliasPackage) { $this->aliasMap[spl_object_hash($package->getAliasOf())][$index] = $package; @@ -319,8 +329,9 @@ class PoolBuilder $aliasPackage = new AliasPackage($basePackage, $alias['alias_normalized'], $alias['alias']); $aliasPackage->setRootPackageAlias(true); - $this->packages[] = $aliasPackage; - $this->aliasMap[spl_object_hash($aliasPackage->getAliasOf())][$index+1] = $aliasPackage; + $newIndex = $this->indexCounter++; + $this->packages[$newIndex] = $aliasPackage; + $this->aliasMap[spl_object_hash($aliasPackage->getAliasOf())][$newIndex] = $aliasPackage; } foreach ($package->getRequires() as $link) { @@ -333,19 +344,19 @@ class PoolBuilder if ($request->getUpdateAllowTransitiveDependencies() && isset($this->skippedLoad[$require])) { if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$require])) { $this->unfixPackage($request, $require); - $this->markPackageNameForLoading($require, $linkConstraint); + $this->markPackageNameForLoading($request, $require, $linkConstraint); } elseif (!$request->getUpdateAllowTransitiveRootDependencies() && $this->isRootRequire($request, $require) && !isset($this->updateAllowWarned[$require])) { $this->updateAllowWarned[$require] = true; $this->io->writeError('Dependency "'.$require.'" is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies to include root dependencies.'); } } else { - $this->markPackageNameForLoading($require, $linkConstraint); + $this->markPackageNameForLoading($request, $require, $linkConstraint); } } else { // We also need to load the requirements of a fixed package // unless it was skipped if (!isset($this->skippedLoad[$require])) { - $this->markPackageNameForLoading($require, $linkConstraint); + $this->markPackageNameForLoading($request, $require, $linkConstraint); } } } @@ -358,7 +369,7 @@ class PoolBuilder if (isset($this->loadedPackages[$replace]) && isset($this->skippedLoad[$replace])) { if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$replace])) { $this->unfixPackage($request, $replace); - $this->markPackageNameForLoading($replace, $link->getConstraint()); + $this->markPackageNameForLoading($request, $replace, $link->getConstraint()); } elseif (!$request->getUpdateAllowTransitiveRootDependencies() && $this->isRootRequire($request, $replace) && !isset($this->updateAllowWarned[$replace])) { $this->updateAllowWarned[$replace] = true; $this->io->writeError('Dependency "'.$replace.'" is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies to include root dependencies.'); @@ -430,14 +441,7 @@ class PoolBuilder if (!($lockedPackage instanceof AliasPackage) && $lockedPackage->getName() === $name) { if (false !== $index = array_search($lockedPackage, $this->packages, true)) { $request->unfixPackage($lockedPackage); - unset($this->packages[$index]); - if (isset($this->aliasMap[spl_object_hash($lockedPackage)])) { - foreach ($this->aliasMap[spl_object_hash($lockedPackage)] as $aliasIndex => $aliasPackage) { - $request->unfixPackage($aliasPackage); - unset($this->packages[$aliasIndex]); - } - unset($this->aliasMap[spl_object_hash($lockedPackage)]); - } + $this->removeLoadedPackage($request, $lockedPackage, $index); } } } @@ -455,6 +459,18 @@ class PoolBuilder unset($this->loadedPackages[$name]); } + private function removeLoadedPackage(Request $request, PackageInterface $package, $index) + { + unset($this->packages[$index]); + if (isset($this->aliasMap[spl_object_hash($package)])) { + foreach ($this->aliasMap[spl_object_hash($package)] as $aliasIndex => $aliasPackage) { + $request->unfixPackage($aliasPackage); + unset($this->packages[$aliasIndex]); + } + unset($this->aliasMap[spl_object_hash($package)]); + } + } + private function getRootAliasesPerPackage(array $aliases) { $normalizedAliases = array(); diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test new file mode 100644 index 000000000..68136d6c2 --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test @@ -0,0 +1,61 @@ +--TEST-- +Check root aliases are loaded + +--ROOT-- +{ + "minimum-stability": "dev", + "aliases": [ + { + "package": "req/pkg", + "version": "dev-feature-foo", + "alias": "dev-master" + } + ] +} + + +--REQUEST-- +{ + "require": { + "package/a": "dev-master", + "req/pkg": "dev-feature-foo" + } +} + +--FIXED-- +[ +] + +--PACKAGES-- +[ + { + "name": "req/pkg", "version": "dev-feature-foo", + "source": { "reference": "feat.f", "type": "git", "url": "" } + }, + { + "name": "req/pkg", "version": "dev-master", + "extra": { "branch-alias": { "dev-master": "1.0.x-dev" } }, + "source": { "reference": "forked", "type": "git", "url": "" } + }, + { + "name": "req/pkg", "version": "dev-master", + "extra": { "branch-alias": { "dev-master": "1.0.x-dev" } }, + "source": { "reference": "master", "type": "git", "url": "" } + }, + { + "name": "package/a", "version": "dev-master", + "require": { "req/pkg": "dev-master" } + } +] + +--EXPECT-- +[ + "package/a-dev-master", + "package/a-9999999-dev (alias of dev-master)", + "req/pkg-dev-feature-foo#feat.f", + "req/pkg-dev-master#feat.f (alias of dev-feature-foo)", + "req/pkg-dev-master#forked", + "req/pkg-dev-master#master", + "req/pkg-1.0.9999999.9999999-dev#forked (alias of dev-master)", + "req/pkg-1.0.9999999.9999999-dev#master (alias of dev-master)" +] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test new file mode 100644 index 000000000..4401f869e --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test @@ -0,0 +1,57 @@ +--TEST-- +Check root aliases get selected correctly + +--ROOT-- +{ + "stability-flags": { + "a/aliased": "dev" + }, + "aliases": [ + { + "package": "a/aliased", + "version": "dev-master", + "alias": "1.0.0" + } + ], + "references": { + "a/aliased": "abcd" + } +} + + +--REQUEST-- +{ + "require": { + "a/aliased": "dev-master", + "b/requirer": "*" + } +} + +--FIXED-- +[ +] + +--PACKAGES-- +[ + { + "name": "a/aliased", "version": "dev-master", + "source": { "reference": "orig", "type": "git", "url": "" } + }, + { + "name": "a/aliased", "version": "1.0" + }, + { + "name": "b/requirer", "version": "1.0.0", + "require": { "a/aliased": "1.0.0" }, + "source": { "reference": "1.0.0", "type": "git", "url": "" } + } +] + +--EXPECT-- +[ + "b/requirer-1.0.0.0#1.0.0", + "a/aliased-dev-master#abcd", + "a/aliased-1.0.0.0#abcd (alias of dev-master)", + "a/aliased-1.0.0.0#abcd", + "a/aliased-9999999-dev#abcd (alias of dev-master)" +] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/stability-flags-take-over-minimum-stability-and-filter-packages.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/stability-flags-take-over-minimum-stability-and-filter-packages.test index b273753d2..3f6d8346c 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/stability-flags-take-over-minimum-stability-and-filter-packages.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/stability-flags-take-over-minimum-stability-and-filter-packages.test @@ -42,5 +42,5 @@ Stability flags apply 4, 5, 6, - "default/pkg-1.2.0.0 alias of 6" + "default/pkg-1.2.0.0 (alias of 6)" ] diff --git a/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php b/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php index b756219a2..75e2b76b0 100644 --- a/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php +++ b/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php @@ -41,6 +41,7 @@ class PoolBuilderTest extends TestCase $rootAliases = !empty($root['aliases']) ? $root['aliases'] : array(); $minimumStability = !empty($root['minimum-stability']) ? $root['minimum-stability'] : 'stable'; $stabilityFlags = !empty($root['stability-flags']) ? $root['stability-flags'] : array(); + $rootReferences = !empty($root['references']) ? $root['references'] : array(); $stabilityFlags = array_map(function ($stability) { return BasePackage::$stabilities[$stability]; }, $stabilityFlags); @@ -71,7 +72,7 @@ class PoolBuilderTest extends TestCase return $pkg; }; - $repositorySet = new RepositorySet($minimumStability, $stabilityFlags, $rootAliases); + $repositorySet = new RepositorySet($minimumStability, $stabilityFlags, $rootAliases, $rootReferences); $repositorySet->addRepository($repo = new ArrayRepository()); $repositorySet->addRepository($lockedRepo = new LockArrayRepository()); foreach ($packages as $package) { @@ -114,15 +115,22 @@ class PoolBuilderTest extends TestCase } $suffix = ''; + if ($package->getSourceReference()) { + $suffix = '#'.$package->getSourceReference(); + } if ($package->getRepository() instanceof LockArrayRepository) { - $suffix = ' (locked)'; + $suffix .= ' (locked)'; } - if ($package instanceof AliasPackage && $id = array_search($package->getAliasOf(), $packageIds, true)) { - return (string) $package->getName().'-'.$package->getVersion() .' alias of '.$id . $suffix; + if ($package instanceof AliasPackage) { + if ($id = array_search($package->getAliasOf(), $packageIds, true)) { + return (string) $package->getName().'-'.$package->getVersion() . $suffix . ' (alias of '.$id . ')'; + } + + return (string) $package->getName().'-'.$package->getVersion() . $suffix . ' (alias of '.$package->getAliasOf()->getVersion().')'; } - return (string) $package . $suffix; + return (string) $package->getName().'-'.$package->getVersion() . $suffix; }, $result); $this->assertSame($expect, $result); From 2fa58ccf963ca5f4f0e059d0be238d8f4226e519 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sat, 6 Jun 2020 17:18:42 +0200 Subject: [PATCH 41/54] Reduce amount of packages loaded by avoiding extensions of the constraint beyond the root constraint --- .../DependencyResolver/PoolBuilder.php | 29 ++++++++++++++++++- .../alias-priority-conflicting.test | 8 ++--- .../poolbuilder/alias-with-reference.test | 3 +- .../partial-update-transitive-deps-unfix.test | 17 +++++++---- 4 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index c6ea7d73d..91381bee2 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -80,6 +80,16 @@ class PoolBuilder private $unacceptableFixedPackages = array(); private $updateAllowList = array(); private $skippedLoad = array(); + + /** + * Keeps a list of dependencies which are root requirements, and as such + * have already their maximum required range loaded and can not be + * extended by markPackageNameForLoading + * + * Packages get cleared from this list if they get unfixed as in that case + * we need to actually load them + */ + private $maxExtendedReqs = array(); /** * @psalm-var array */ @@ -156,7 +166,8 @@ class PoolBuilder continue; } - $this->markPackageNameForLoading($request, $packageName, $constraint); + $this->packagesToLoad[$packageName] = $constraint; + $this->maxExtendedReqs[$packageName] = true; } // clean up packagesToLoad for anything we manually marked loaded above @@ -225,6 +236,21 @@ class PoolBuilder private function markPackageNameForLoading(Request $request, $name, ConstraintInterface $constraint) { + // Root require (which was not unfixed) already loaded the maximum range so no + // need to check anything here + if (isset($this->maxExtendedReqs[$name])) { + return; + } + + // Root requires can not be overruled by dependencies so there is no point in + // extending the loaded constraint for those. + // This is triggered when loading a root require which was fixed but got unfixed, then + // we make sure that we load at most the intervals covered by the root constraint. + $rootRequires = $request->getRequires(); + if (isset($rootRequires[$name]) && !Intervals::isSubsetOf($constraint, $rootRequires[$name])) { + $constraint = $rootRequires[$name]; + } + // Maybe it was already marked before but not loaded yet. In that case // we have to extend the constraint (we don't check if they match because // MultiConstraint::create() will optimize anyway) @@ -457,6 +483,7 @@ class PoolBuilder unset($this->skippedLoad[$name]); unset($this->loadedPackages[$name]); + unset($this->maxExtendedReqs[$name]); } private function removeLoadedPackage(Request $request, PackageInterface $package, $index) diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test index 68136d6c2..4386df5ac 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test @@ -50,12 +50,8 @@ Check root aliases are loaded --EXPECT-- [ - "package/a-dev-master", - "package/a-9999999-dev (alias of dev-master)", "req/pkg-dev-feature-foo#feat.f", "req/pkg-dev-master#feat.f (alias of dev-feature-foo)", - "req/pkg-dev-master#forked", - "req/pkg-dev-master#master", - "req/pkg-1.0.9999999.9999999-dev#forked (alias of dev-master)", - "req/pkg-1.0.9999999.9999999-dev#master (alias of dev-master)" + "package/a-dev-master", + "package/a-9999999-dev (alias of dev-master)" ] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test index 4401f869e..f7ef17b2f 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test @@ -49,9 +49,8 @@ Check root aliases get selected correctly --EXPECT-- [ - "b/requirer-1.0.0.0#1.0.0", "a/aliased-dev-master#abcd", "a/aliased-1.0.0.0#abcd (alias of dev-master)", - "a/aliased-1.0.0.0#abcd", + "b/requirer-1.0.0.0#1.0.0", "a/aliased-9999999-dev#abcd (alias of dev-master)" ] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test index e075c2df2..d3cfb0db1 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test @@ -5,12 +5,14 @@ Partially updating one root requirement with transitive deps fully updates anoth { "require": { "root/update": "*", - "root/dep": "*" + "root/dep": "1.0.*", + "root/dep2": "*" }, "locked": [ {"name": "root/update", "version": "1.0.1", "require": {"dep/dep": "1.*"}}, - {"name": "dep/dep", "version": "1.0.2", "require": {"root/dep": "1.*"}}, - {"name": "root/dep", "version": "1.0.3"} + {"name": "dep/dep", "version": "1.0.2", "require": {"root/dep": "1.*", "root/dep2": "1.*"}}, + {"name": "root/dep", "version": "1.0.3"}, + {"name": "root/dep2", "version": "1.0.3"} ], "allowList": [ "root/update" @@ -28,8 +30,10 @@ Partially updating one root requirement with transitive deps fully updates anoth {"name": "root/update", "version": "1.0.5", "require": {"dep/dep": "2.*"}}, {"name": "root/dep", "version": "1.0.6"}, {"name": "root/dep", "version": "2.0.7"}, - {"name": "dep/dep", "version": "1.0.8", "require": {"root/dep": "1.*"}}, - {"name": "dep/dep", "version": "2.0.9", "require": {"root/dep": "2.*"}} + {"name": "root/dep2", "version": "1.0.6"}, + {"name": "root/dep2", "version": "2.0.7"}, + {"name": "dep/dep", "version": "1.0.8", "require": {"root/dep": "1.*", "root/dep2": "1.*"}}, + {"name": "dep/dep", "version": "2.0.9", "require": {"root/dep": "2.*", "root/dep2": "2.*"}} ] --EXPECT-- @@ -39,5 +43,6 @@ Partially updating one root requirement with transitive deps fully updates anoth "dep/dep-1.0.8.0", "dep/dep-2.0.9.0", "root/dep-1.0.6.0", - "root/dep-2.0.7.0" + "root/dep2-1.0.6.0", + "root/dep2-2.0.7.0" ] From 23339e44b826e5ccd853cd2a63a8bb041ae2efc9 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 17 Jul 2020 15:35:21 +0200 Subject: [PATCH 42/54] Fix alias tests to use default-branches --- .../Fixtures/poolbuilder/alias-priority-conflicting.test | 9 ++++++--- .../Fixtures/poolbuilder/alias-with-reference.test | 3 ++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test index 4386df5ac..e8c0c7af6 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test @@ -35,16 +35,19 @@ Check root aliases are loaded { "name": "req/pkg", "version": "dev-master", "extra": { "branch-alias": { "dev-master": "1.0.x-dev" } }, - "source": { "reference": "forked", "type": "git", "url": "" } + "source": { "reference": "forked", "type": "git", "url": "" }, + "default-branch": true }, { "name": "req/pkg", "version": "dev-master", "extra": { "branch-alias": { "dev-master": "1.0.x-dev" } }, - "source": { "reference": "master", "type": "git", "url": "" } + "source": { "reference": "master", "type": "git", "url": "" }, + "default-branch": true }, { "name": "package/a", "version": "dev-master", - "require": { "req/pkg": "dev-master" } + "require": { "req/pkg": "dev-master" }, + "default-branch": true } ] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test index f7ef17b2f..51274b275 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test @@ -35,7 +35,8 @@ Check root aliases get selected correctly [ { "name": "a/aliased", "version": "dev-master", - "source": { "reference": "orig", "type": "git", "url": "" } + "source": { "reference": "orig", "type": "git", "url": "" }, + "default-branch": true }, { "name": "a/aliased", "version": "1.0" From 3577070efa925981343c67436c60ea6b68d6a6c6 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 17 Jul 2020 15:37:04 +0200 Subject: [PATCH 43/54] Fix docblocks --- src/Composer/Package/Link.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Composer/Package/Link.php b/src/Composer/Package/Link.php index 81ca86e8b..4af6a5a6a 100644 --- a/src/Composer/Package/Link.php +++ b/src/Composer/Package/Link.php @@ -32,7 +32,7 @@ class Link protected $target; /** - * @var ConstraintInterface|null + * @var ConstraintInterface */ protected $constraint; @@ -49,11 +49,11 @@ class Link /** * Creates a new package link. * - * @param string $source - * @param string $target - * @param ConstraintInterface|null $constraint Constraint applying to the target of this link - * @param string $description Used to create a descriptive string representation - * @param string|null $prettyConstraint + * @param string $source + * @param string $target + * @param ConstraintInterface $constraint Constraint applying to the target of this link + * @param string $description Used to create a descriptive string representation + * @param string|null $prettyConstraint */ public function __construct($source, $target, ConstraintInterface $constraint, $description = 'relates to', $prettyConstraint = null) { From 22367a68f998e212d557e317ea2ec6566f58340e Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Mon, 8 Jun 2020 15:46:03 +0200 Subject: [PATCH 44/54] Avoid loading same packages multiple times --- .../DependencyResolver/PoolBuilder.php | 19 +++++++------- src/Composer/Repository/ArrayRepository.php | 3 ++- .../Repository/ComposerRepository.php | 25 +++++++++++++------ .../Repository/CompositeRepository.php | 4 +-- src/Composer/Repository/FilterRepository.php | 4 +-- .../Repository/RepositoryInterface.php | 4 ++- ...t-expansion-works-with-exact-versions.test | 2 +- ...-not-loaded-if-not-required-expansion.test | 2 +- 8 files changed, 39 insertions(+), 24 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index be1a6ae65..e1a49cd5b 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -70,6 +70,10 @@ class PoolBuilder * @psalm-var array */ private $loadedPackages = array(); + /** + * @psalm-var array>> + */ + private $loadedPerRepo = array(); /** * @psalm-var Package[] */ @@ -227,8 +231,11 @@ class PoolBuilder $this->aliasMap = array(); $this->packagesToLoad = array(); $this->loadedPackages = array(); + $this->loadedPerRepo = array(); $this->packages = array(); $this->unacceptableFixedPackages = array(); + $this->maxExtendedReqs = array(); + $this->skippedLoad = array(); $this->indexCounter = 0; return $pool; @@ -287,20 +294,13 @@ class PoolBuilder private function loadPackagesMarkedForLoading(Request $request, $repositories) { foreach ($this->packagesToLoad as $name => $constraint) { - // remove all already-loaded packages matching those to be loaded to avoid duplicates - foreach ($this->packages as $index => $pkg) { - if ($pkg->getName() === $name) { - $this->removeLoadedPackage($request, $pkg, $index); - } - } - $this->loadedPackages[$name] = $constraint; } $packageBatch = $this->packagesToLoad; $this->packagesToLoad = array(); - foreach ($repositories as $repository) { + foreach ($repositories as $repoIndex => $repository) { if (empty($packageBatch)) { break; } @@ -310,13 +310,14 @@ class PoolBuilder if ($repository instanceof PlatformRepository || $repository === $request->getLockedRepository()) { continue; } - $result = $repository->loadPackages($packageBatch, $this->acceptableStabilities, $this->stabilityFlags); + $result = $repository->loadPackages($packageBatch, $this->acceptableStabilities, $this->stabilityFlags, isset($this->loadedPerRepo[$repoIndex]) ? $this->loadedPerRepo[$repoIndex] : array()); foreach ($result['namesFound'] as $name) { // avoid loading the same package again from other repositories once it has been found unset($packageBatch[$name]); } foreach ($result['packages'] as $package) { + $this->loadedPerRepo[$repoIndex][$package->getName()][$package->getVersion()] = $package; $this->loadPackage($request, $package); } } diff --git a/src/Composer/Repository/ArrayRepository.php b/src/Composer/Repository/ArrayRepository.php index 37a241cbc..24a7a38d8 100644 --- a/src/Composer/Repository/ArrayRepository.php +++ b/src/Composer/Repository/ArrayRepository.php @@ -50,7 +50,7 @@ class ArrayRepository implements RepositoryInterface /** * {@inheritDoc} */ - public function loadPackages(array $packageMap, array $acceptableStabilities, array $stabilityFlags) + public function loadPackages(array $packageMap, array $acceptableStabilities, array $stabilityFlags, array $alreadyLoaded = array()) { $packages = $this->getPackages(); @@ -61,6 +61,7 @@ class ArrayRepository implements RepositoryInterface if ( (!$packageMap[$package->getName()] || $packageMap[$package->getName()]->matches(new Constraint('==', $package->getVersion()))) && StabilityFilter::isPackageAcceptable($acceptableStabilities, $stabilityFlags, $package->getNames(), $package->getStability()) + && !isset($alreadyLoaded[$package->getName()][$package->getVersion()]) ) { // add selected packages which match stability requirements $result[spl_object_hash($package)] = $package; diff --git a/src/Composer/Repository/ComposerRepository.php b/src/Composer/Repository/ComposerRepository.php index 941cf08dd..2de2fc17d 100644 --- a/src/Composer/Repository/ComposerRepository.php +++ b/src/Composer/Repository/ComposerRepository.php @@ -340,13 +340,13 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito return $names; } - public function loadPackages(array $packageNameMap, array $acceptableStabilities, array $stabilityFlags) + public function loadPackages(array $packageNameMap, array $acceptableStabilities, array $stabilityFlags, array $alreadyLoaded = array()) { // this call initializes loadRootServerFile which is needed for the rest below to work $hasProviders = $this->hasProviders(); if (!$hasProviders && !$this->hasPartialPackages() && !$this->lazyProvidersUrl) { - return parent::loadPackages($packageNameMap, $acceptableStabilities, $stabilityFlags); + return parent::loadPackages($packageNameMap, $acceptableStabilities, $stabilityFlags, $alreadyLoaded); } $packages = array(); @@ -362,12 +362,13 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito continue; } - $candidates = $this->whatProvides($name, $acceptableStabilities, $stabilityFlags); + $candidates = $this->whatProvides($name, $acceptableStabilities, $stabilityFlags, $alreadyLoaded); foreach ($candidates as $candidate) { if ($candidate->getName() !== $name) { throw new \LogicException('whatProvides should never return a package with a different name than the requested one'); } $namesFound[$name] = true; + if (!$constraint || $constraint->matches(new Constraint('==', $candidate->getVersion()))) { $matches[spl_object_hash($candidate)] = $candidate; if ($candidate instanceof AliasPackage && !isset($matches[spl_object_hash($candidate->getAliasOf())])) { @@ -400,7 +401,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito } } - $result = $this->loadAsyncPackages($packageNameMap, $acceptableStabilities, $stabilityFlags); + $result = $this->loadAsyncPackages($packageNameMap, $acceptableStabilities, $stabilityFlags, $alreadyLoaded); $packages = array_merge($packages, $result['packages']); $namesFound = array_merge($namesFound, $result['namesFound']); } @@ -530,7 +531,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito * @param string $name package name * @return array|mixed */ - private function whatProvides($name, array $acceptableStabilities = null, array $stabilityFlags = null) + private function whatProvides($name, array $acceptableStabilities = null, array $stabilityFlags = null, array $alreadyLoaded = array()) { if (!$this->hasPartialPackages() || !isset($this->partialPackagesByName[$name])) { // skip platform packages, root package and composer-plugin-api @@ -622,6 +623,11 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito $version['version_normalized'] = $this->versionParser->normalize($version['version']); } + // avoid loading packages which have already been loaded + if (isset($alreadyLoaded[$name][$version['version_normalized']])) { + continue; + } + if ($this->isVersionAcceptable(null, $normalizedName, $version, $acceptableStabilities, $stabilityFlags)) { $versionsToLoad[$version['uid']] = $version; } @@ -679,7 +685,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito /** * @param array $packageNames array of package name => ConstraintInterface|null - if a constraint is provided, only packages matching it will be loaded */ - private function loadAsyncPackages(array $packageNames, array $acceptableStabilities = null, array $stabilityFlags = null) + private function loadAsyncPackages(array $packageNames, array $acceptableStabilities = null, array $stabilityFlags = null, array $alreadyLoaded = array()) { $this->loadRootServerFile(); @@ -722,7 +728,7 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito } $promises[] = $this->asyncFetchFile($url, $cacheKey, $lastModified) - ->then(function ($response) use (&$packages, &$namesFound, $contents, $realName, $constraint, $repo, $acceptableStabilities, $stabilityFlags) { + ->then(function ($response) use (&$packages, &$namesFound, $contents, $realName, $constraint, $repo, $acceptableStabilities, $stabilityFlags, $alreadyLoaded) { if (true === $response) { $response = $contents; } @@ -747,6 +753,11 @@ class ComposerRepository extends ArrayRepository implements ConfigurableReposito $version['version_normalized'] = $repo->versionParser->normalize($version['version']); } + // avoid loading packages which have already been loaded + if (isset($alreadyLoaded[$realName][$version['version_normalized']])) { + continue; + } + if ($repo->isVersionAcceptable($constraint, $realName, $version, $acceptableStabilities, $stabilityFlags)) { $versionsToLoad[] = $version; } diff --git a/src/Composer/Repository/CompositeRepository.php b/src/Composer/Repository/CompositeRepository.php index acabaffb0..eaa70db2b 100644 --- a/src/Composer/Repository/CompositeRepository.php +++ b/src/Composer/Repository/CompositeRepository.php @@ -102,13 +102,13 @@ class CompositeRepository implements RepositoryInterface /** * {@inheritDoc} */ - public function loadPackages(array $packageMap, array $acceptableStabilities, array $stabilityFlags) + public function loadPackages(array $packageMap, array $acceptableStabilities, array $stabilityFlags, array $alreadyLoaded = array()) { $packages = array(); $namesFound = array(); foreach ($this->repositories as $repository) { /* @var $repository RepositoryInterface */ - $result = $repository->loadPackages($packageMap, $acceptableStabilities, $stabilityFlags); + $result = $repository->loadPackages($packageMap, $acceptableStabilities, $stabilityFlags, $alreadyLoaded); $packages[] = $result['packages']; $namesFound[] = $result['namesFound']; } diff --git a/src/Composer/Repository/FilterRepository.php b/src/Composer/Repository/FilterRepository.php index 5221e55cb..361c2fe67 100644 --- a/src/Composer/Repository/FilterRepository.php +++ b/src/Composer/Repository/FilterRepository.php @@ -108,7 +108,7 @@ class FilterRepository implements RepositoryInterface /** * {@inheritDoc} */ - public function loadPackages(array $packageMap, array $acceptableStabilities, array $stabilityFlags) + public function loadPackages(array $packageMap, array $acceptableStabilities, array $stabilityFlags, array $alreadyLoaded = array()) { foreach ($packageMap as $name => $constraint) { if (!$this->isAllowed($name)) { @@ -120,7 +120,7 @@ class FilterRepository implements RepositoryInterface return array('namesFound' => array(), 'packages' => array()); } - $result = $this->repo->loadPackages($packageMap, $acceptableStabilities, $stabilityFlags); + $result = $this->repo->loadPackages($packageMap, $acceptableStabilities, $stabilityFlags, $alreadyLoaded); if (!$this->canonical) { $result['namesFound'] = array(); } diff --git a/src/Composer/Repository/RepositoryInterface.php b/src/Composer/Repository/RepositoryInterface.php index afb99ca62..54e6e04a6 100644 --- a/src/Composer/Repository/RepositoryInterface.php +++ b/src/Composer/Repository/RepositoryInterface.php @@ -75,11 +75,13 @@ interface RepositoryInterface extends \Countable * @psalm-param array $acceptableStabilities * @param int[] $stabilityFlags an array of package name => BasePackage::STABILITY_* value * @psalm-param array $stabilityFlags + * @param array[] $alreadyLoaded an array of package name => package version => package + * @psalm-param array> $alreadyLoaded * * @return array [namesFound => string[], packages => PackageInterface[]] * @psalm-return array{namesFound: string[], packages: PackageInterface[]} */ - public function loadPackages(array $packageNameMap, array $acceptableStabilities, array $stabilityFlags); + public function loadPackages(array $packageNameMap, array $acceptableStabilities, array $stabilityFlags, array $alreadyLoaded = array()); /** * Searches the repository for packages containing the query diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test index fdabd9c8f..7a3046ae0 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test @@ -23,7 +23,7 @@ Tests if version constraint is expanded. If not, dep/dep 3.0.0 would not be load --EXPECT-- [ "root/req-1.0.0.0", - "dep/dep2-2.3.4.0", "dep/dep-2.3.4.0", + "dep/dep2-2.3.4.0", "dep/dep-2.3.5.0" ] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test index 53795c761..198df20a4 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test @@ -25,8 +25,8 @@ Tests if version constraint is expanded. If not, dep/dep 3.0.0 would not be load --EXPECT-- [ "root/req-1.0.0.0", - "dep/dep2-2.3.4.0", "dep/dep-2.3.4.0", "dep/dep-2.3.5.0", + "dep/dep2-2.3.4.0", "dep/dep-3.0.0.0" ] From 071350286fb196722728511aecd00e1defa826c7 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Mon, 15 Jun 2020 13:00:39 +0200 Subject: [PATCH 45/54] Compact constraints to avoid ending up with very long multi constraints --- src/Composer/DependencyResolver/PoolBuilder.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index e1a49cd5b..9561bb73f 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -268,7 +268,7 @@ class PoolBuilder } // extend the constraint to be loaded - $constraint = MultiConstraint::create(array($this->packagesToLoad[$name], $constraint), false); + $constraint = Intervals::compactConstraint(MultiConstraint::create(array($this->packagesToLoad[$name], $constraint), false)); } // Not yet loaded or already marked for a reload, override the existing constraint @@ -287,7 +287,7 @@ class PoolBuilder // We have already loaded that package but not in the constraint that's // required. We extend the constraint and mark that package as not being loaded // yet so we get the required package versions - $this->packagesToLoad[$name] = MultiConstraint::create(array($this->loadedPackages[$name], $constraint), false); + $this->packagesToLoad[$name] = Intervals::compactConstraint(MultiConstraint::create(array($this->loadedPackages[$name], $constraint), false)); unset($this->loadedPackages[$name]); } From 00e268cdbfd1dde8c311fca3f22d49e51daeaffe Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 13 Aug 2020 16:45:43 +0200 Subject: [PATCH 46/54] Clear Intervals cache when we are done with it --- src/Composer/DependencyResolver/PoolBuilder.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 9561bb73f..416a8d1dd 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -238,6 +238,8 @@ class PoolBuilder $this->skippedLoad = array(); $this->indexCounter = 0; + Intervals::clear(); + return $pool; } From f516d36f6f9315725635fa5276774b3f70132a95 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Tue, 25 Aug 2020 15:37:28 +0200 Subject: [PATCH 47/54] Make sure Request::requireName can not be called twice for the same name --- src/Composer/DependencyResolver/Request.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Composer/DependencyResolver/Request.php b/src/Composer/DependencyResolver/Request.php index 332ead628..c3367e3d9 100644 --- a/src/Composer/DependencyResolver/Request.php +++ b/src/Composer/DependencyResolver/Request.php @@ -56,7 +56,14 @@ class Request public function requireName($packageName, ConstraintInterface $constraint = null) { $packageName = strtolower($packageName); - $this->requires[$packageName] = $constraint ? $constraint : new MatchAllConstraint(); + + if ($constraint === null) { + $constraint = new MatchAllConstraint(); + } + if (isset($this->requires[$packageName])) { + throw new \LogicException('Overwriting requires seems like a bug ('.$packageName.' '.$this->requires[$packageName]->getPrettyConstraint().' => '.$constraint->getPrettyConstraint().', check why it is happening, might be a root alias'); + } + $this->requires[$packageName] = $constraint; } /** From fcb9ef48998294aaf6d255d223adce0b4f7fee7e Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 28 Aug 2020 12:15:19 +0200 Subject: [PATCH 48/54] Allow defining multiple reppos in pool builder tests --- .../alias-priority-conflicting.test | 46 ++++++++++--------- .../poolbuilder/alias-with-reference.test | 30 ++++++------ ...t-expansion-works-with-exact-versions.test | 12 +++-- ...fixed-packages-do-not-load-from-repos.test | 16 ++++--- ...kages-replaced-do-not-load-from-repos.test | 14 +++--- ...-not-loaded-if-not-required-expansion.test | 16 ++++--- ...-not-loaded-if-not-required-recursive.test | 14 +++--- .../packages-that-do-not-exist.test | 8 ++-- ...-update-transitive-deps-no-root-unfix.test | 20 ++++---- .../partial-update-transitive-deps-unfix.test | 20 ++++---- ...artial-update-unfixing-with-replacers.test | 24 +++++----- .../Fixtures/poolbuilder/partial-update.test | 14 +++--- ...minimum-stability-and-filter-packages.test | 18 ++++---- .../DependencyResolver/PoolBuilderTest.php | 18 ++++---- 14 files changed, 149 insertions(+), 121 deletions(-) diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test index e8c0c7af6..5456f0baf 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-priority-conflicting.test @@ -26,29 +26,31 @@ Check root aliases are loaded [ ] ---PACKAGES-- +--PACKAGE-REPOS-- [ - { - "name": "req/pkg", "version": "dev-feature-foo", - "source": { "reference": "feat.f", "type": "git", "url": "" } - }, - { - "name": "req/pkg", "version": "dev-master", - "extra": { "branch-alias": { "dev-master": "1.0.x-dev" } }, - "source": { "reference": "forked", "type": "git", "url": "" }, - "default-branch": true - }, - { - "name": "req/pkg", "version": "dev-master", - "extra": { "branch-alias": { "dev-master": "1.0.x-dev" } }, - "source": { "reference": "master", "type": "git", "url": "" }, - "default-branch": true - }, - { - "name": "package/a", "version": "dev-master", - "require": { "req/pkg": "dev-master" }, - "default-branch": true - } + [ + { + "name": "req/pkg", "version": "dev-feature-foo", + "source": { "reference": "feat.f", "type": "git", "url": "" } + }, + { + "name": "req/pkg", "version": "dev-master", + "extra": { "branch-alias": { "dev-master": "1.0.x-dev" } }, + "source": { "reference": "forked", "type": "git", "url": "" }, + "default-branch": true + }, + { + "name": "req/pkg", "version": "dev-master", + "extra": { "branch-alias": { "dev-master": "1.0.x-dev" } }, + "source": { "reference": "master", "type": "git", "url": "" }, + "default-branch": true + }, + { + "name": "package/a", "version": "dev-master", + "require": { "req/pkg": "dev-master" }, + "default-branch": true + } + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test index 51274b275..f3cbca7f1 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/alias-with-reference.test @@ -31,21 +31,23 @@ Check root aliases get selected correctly [ ] ---PACKAGES-- +--PACKAGE-REPOS-- [ - { - "name": "a/aliased", "version": "dev-master", - "source": { "reference": "orig", "type": "git", "url": "" }, - "default-branch": true - }, - { - "name": "a/aliased", "version": "1.0" - }, - { - "name": "b/requirer", "version": "1.0.0", - "require": { "a/aliased": "1.0.0" }, - "source": { "reference": "1.0.0", "type": "git", "url": "" } - } + [ + { + "name": "a/aliased", "version": "dev-master", + "source": { "reference": "orig", "type": "git", "url": "" }, + "default-branch": true + }, + { + "name": "a/aliased", "version": "1.0" + }, + { + "name": "b/requirer", "version": "1.0.0", + "require": { "a/aliased": "1.0.0" }, + "source": { "reference": "1.0.0", "type": "git", "url": "" } + } + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test index 7a3046ae0..908b3602c 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/constraint-expansion-works-with-exact-versions.test @@ -12,12 +12,14 @@ Tests if version constraint is expanded. If not, dep/dep 3.0.0 would not be load [ ] ---PACKAGES-- +--PACKAGE-REPOS-- [ - {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.3.4"}}, - {"name": "dep/dep", "version": "2.3.4", "require": {"dep/dep2": "2.*"}}, - {"name": "dep/dep", "version": "2.3.5"}, - {"name": "dep/dep2", "version": "2.3.4", "require": {"dep/dep": "2.*"}} + [ + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.3.4"}}, + {"name": "dep/dep", "version": "2.3.4", "require": {"dep/dep2": "2.*"}}, + {"name": "dep/dep", "version": "2.3.5"}, + {"name": "dep/dep2", "version": "2.3.4", "require": {"dep/dep": "2.*"}} + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-do-not-load-from-repos.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-do-not-load-from-repos.test index 976587e5b..fb52a9b6d 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-do-not-load-from-repos.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-do-not-load-from-repos.test @@ -15,14 +15,16 @@ Fixed packages do not get loaded from the repos {"name": "dep/dep", "version": "2.1.5", "id": 2} ] ---PACKAGES-- +--PACKAGE-REPOS-- [ - {"name": "some/pkg", "version": "1.0.0"}, - {"name": "some/pkg", "version": "1.1.0"}, - {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, - {"name": "root/req", "version": "2.0.0", "require": {"dep/dep": "3.*"}}, - {"name": "dep/dep", "version": "2.3.4"}, - {"name": "dep/dep", "version": "3.0.1"} + [ + {"name": "some/pkg", "version": "1.0.0"}, + {"name": "some/pkg", "version": "1.1.0"}, + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "root/req", "version": "2.0.0", "require": {"dep/dep": "3.*"}}, + {"name": "dep/dep", "version": "2.3.4"}, + {"name": "dep/dep", "version": "3.0.1"} + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-replaced-do-not-load-from-repos.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-replaced-do-not-load-from-repos.test index bc403f83d..4847411ef 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-replaced-do-not-load-from-repos.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/fixed-packages-replaced-do-not-load-from-repos.test @@ -14,13 +14,15 @@ Packages replaced by fixed packages do not get loaded from the repos {"name": "some/pkg", "version": "1.0.3", "replace": {"dep/dep": "2.1.0"}, "id": 1} ] ---PACKAGES-- +--PACKAGE-REPOS-- [ - {"name": "some/pkg", "version": "1.0.0"}, - {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, - {"name": "root/req", "version": "2.0.0", "require": {"dep/dep": "3.*"}}, - {"name": "dep/dep", "version": "2.3.4"}, - {"name": "dep/dep", "version": "3.0.1"} + [ + {"name": "some/pkg", "version": "1.0.0"}, + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "root/req", "version": "2.0.0", "require": {"dep/dep": "3.*"}}, + {"name": "dep/dep", "version": "2.3.4"}, + {"name": "dep/dep", "version": "3.0.1"} + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test index 198df20a4..a41f4d1aa 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-expansion.test @@ -12,14 +12,16 @@ Tests if version constraint is expanded. If not, dep/dep 3.0.0 would not be load [ ] ---PACKAGES-- +--PACKAGE-REPOS-- [ - {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, - {"name": "dep/dep", "version": "2.3.4", "require": {"dep/dep2": "2.*"}}, - {"name": "dep/dep", "version": "2.3.5"}, - {"name": "dep/dep", "version": "3.0.0"}, - {"name": "dep/dep", "version": "4.0.0"}, - {"name": "dep/dep2", "version": "2.3.4", "require": {"dep/dep": "3.*"}} + [ + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "dep/dep", "version": "2.3.4", "require": {"dep/dep2": "2.*"}}, + {"name": "dep/dep", "version": "2.3.5"}, + {"name": "dep/dep", "version": "3.0.0"}, + {"name": "dep/dep", "version": "4.0.0"}, + {"name": "dep/dep2", "version": "2.3.4", "require": {"dep/dep": "3.*"}} + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-recursive.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-recursive.test index 65b762df1..0eb8bbb69 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-recursive.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/package-versions-are-not-loaded-if-not-required-recursive.test @@ -12,13 +12,15 @@ Test irrelevant package versions are not loaded recursively [ ] ---PACKAGES-- +--PACKAGE-REPOS-- [ - {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, - {"name": "dep/dep", "version": "2.3.4", "require": {"dep/dep2": "2.*"}}, - {"name": "dep/dep", "version": "3.0.1"}, - {"name": "dep/dep2", "version": "2.3.4"}, - {"name": "dep/dep2", "version": "3.0.1"} + [ + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "dep/dep", "version": "2.3.4", "require": {"dep/dep2": "2.*"}}, + {"name": "dep/dep", "version": "3.0.1"}, + {"name": "dep/dep2", "version": "2.3.4"}, + {"name": "dep/dep2", "version": "3.0.1"} + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/packages-that-do-not-exist.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/packages-that-do-not-exist.test index 490964fdf..bcf041002 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/packages-that-do-not-exist.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/packages-that-do-not-exist.test @@ -12,10 +12,12 @@ Test package is not found [ ] ---PACKAGES-- +--PACKAGE-REPOS-- [ - {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, - {"name": "dep/dep", "version": "2.3.4", "require": {"dep/dep3": "2.*"}} + [ + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "dep/dep", "version": "2.3.4", "require": {"dep/dep3": "2.*"}} + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-no-root-unfix.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-no-root-unfix.test index 360519edb..4f3f3c8c0 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-no-root-unfix.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-no-root-unfix.test @@ -23,16 +23,18 @@ Partially updating one root requirement with transitive deps without root requir [ ] ---PACKAGES-- +--PACKAGE-REPOS-- [ - {"name": "root/update", "version": "1.0.4", "require": {"dep/dep": "1.*", "dep2/dep2": "1.*"}}, - {"name": "root/update", "version": "1.0.5", "require": {"dep/dep": "2.*", "dep2/dep2": "2.*"}}, - {"name": "root/fix", "version": "1.0.6"}, - {"name": "root/fix", "version": "2.0.7"}, - {"name": "dep/dep", "version": "1.0.8", "require": {"root/fix": "1.*"}}, - {"name": "dep/dep", "version": "2.0.9", "require": {"root/fix": "2.*"}}, - {"name": "dep2/dep2", "version": "1.0.8", "require": {"dep/dep": "1.*"}}, - {"name": "dep2/dep2", "version": "2.0.9", "require": {"dep/dep": "2.*"}} + [ + {"name": "root/update", "version": "1.0.4", "require": {"dep/dep": "1.*", "dep2/dep2": "1.*"}}, + {"name": "root/update", "version": "1.0.5", "require": {"dep/dep": "2.*", "dep2/dep2": "2.*"}}, + {"name": "root/fix", "version": "1.0.6"}, + {"name": "root/fix", "version": "2.0.7"}, + {"name": "dep/dep", "version": "1.0.8", "require": {"root/fix": "1.*"}}, + {"name": "dep/dep", "version": "2.0.9", "require": {"root/fix": "2.*"}}, + {"name": "dep2/dep2", "version": "1.0.8", "require": {"dep/dep": "1.*"}}, + {"name": "dep2/dep2", "version": "2.0.9", "require": {"dep/dep": "2.*"}} + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test index d3cfb0db1..2db47d1bd 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-transitive-deps-unfix.test @@ -24,16 +24,18 @@ Partially updating one root requirement with transitive deps fully updates anoth [ ] ---PACKAGES-- +--PACKAGE-REPOS-- [ - {"name": "root/update", "version": "1.0.4", "require": {"dep/dep": "1.*"}}, - {"name": "root/update", "version": "1.0.5", "require": {"dep/dep": "2.*"}}, - {"name": "root/dep", "version": "1.0.6"}, - {"name": "root/dep", "version": "2.0.7"}, - {"name": "root/dep2", "version": "1.0.6"}, - {"name": "root/dep2", "version": "2.0.7"}, - {"name": "dep/dep", "version": "1.0.8", "require": {"root/dep": "1.*", "root/dep2": "1.*"}}, - {"name": "dep/dep", "version": "2.0.9", "require": {"root/dep": "2.*", "root/dep2": "2.*"}} + [ + {"name": "root/update", "version": "1.0.4", "require": {"dep/dep": "1.*"}}, + {"name": "root/update", "version": "1.0.5", "require": {"dep/dep": "2.*"}}, + {"name": "root/dep", "version": "1.0.6"}, + {"name": "root/dep", "version": "2.0.7"}, + {"name": "root/dep2", "version": "1.0.6"}, + {"name": "root/dep2", "version": "2.0.7"}, + {"name": "dep/dep", "version": "1.0.8", "require": {"root/dep": "1.*", "root/dep2": "1.*"}}, + {"name": "dep/dep", "version": "2.0.9", "require": {"root/dep": "2.*", "root/dep2": "2.*"}} + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-with-replacers.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-with-replacers.test index e7cbf742c..c4148d5ba 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-with-replacers.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-with-replacers.test @@ -23,18 +23,20 @@ Fixed packages and replacers get unfixed correctly (refs https://github.com/comp [ ] ---PACKAGES-- +--PACKAGE-REPOS-- [ - {"name": "root/req1", "version": "1.0.0", "require": {"replacer/pkg": "1.*"}}, - {"name": "root/req1", "version": "1.1.0", "require": {"replacer/pkg": "1.*"}}, - {"name": "root/req3", "version": "1.0.0", "require": {"replaced/pkg": "1.*"}}, - {"name": "root/req3", "version": "1.1.0", "require": {"replaced/pkg": "1.*"}}, - {"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "*"}}, - {"name": "replacer/pkg", "version": "1.1.0", "replace": {"replaced/pkg": "*"}}, - {"name": "replaced/pkg", "version": "1.2.3"}, - {"name": "replaced/pkg", "version": "1.2.4"}, - {"name": "dep/dep", "version": "2.3.5"}, - {"name": "dep/dep", "version": "2.3.6"} + [ + {"name": "root/req1", "version": "1.0.0", "require": {"replacer/pkg": "1.*"}}, + {"name": "root/req1", "version": "1.1.0", "require": {"replacer/pkg": "1.*"}}, + {"name": "root/req3", "version": "1.0.0", "require": {"replaced/pkg": "1.*"}}, + {"name": "root/req3", "version": "1.1.0", "require": {"replaced/pkg": "1.*"}}, + {"name": "replacer/pkg", "version": "1.0.0", "replace": {"replaced/pkg": "*"}}, + {"name": "replacer/pkg", "version": "1.1.0", "replace": {"replaced/pkg": "*"}}, + {"name": "replaced/pkg", "version": "1.2.3"}, + {"name": "replaced/pkg", "version": "1.2.4"}, + {"name": "dep/dep", "version": "2.3.5"}, + {"name": "dep/dep", "version": "2.3.6"} + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update.test index af6c5c689..a4d04ced5 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update.test @@ -20,13 +20,15 @@ Partially updating a root requirement without deps, still selects a new dependen [ ] ---PACKAGES-- +--PACKAGE-REPOS-- [ - {"name": "some/pkg", "version": "1.0.4"}, - {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, - {"name": "root/req", "version": "2.0.0", "require": {"dep/dep": "3.*"}}, - {"name": "dep/dep", "version": "2.3.4"}, - {"name": "dep/dep", "version": "3.0.1"} + [ + {"name": "some/pkg", "version": "1.0.4"}, + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "root/req", "version": "2.0.0", "require": {"dep/dep": "3.*"}}, + {"name": "dep/dep", "version": "2.3.4"}, + {"name": "dep/dep", "version": "3.0.1"} + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/stability-flags-take-over-minimum-stability-and-filter-packages.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/stability-flags-take-over-minimum-stability-and-filter-packages.test index 3f6d8346c..849033dcb 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/stability-flags-take-over-minimum-stability-and-filter-packages.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/stability-flags-take-over-minimum-stability-and-filter-packages.test @@ -24,15 +24,17 @@ Stability flags apply } } ---PACKAGES-- +--PACKAGE-REPOS-- [ - {"name": "flagged/pkg", "version": "1.0.0", "id": 1}, - {"name": "flagged/pkg", "version": "1.0.0-beta", "id": 2}, - {"name": "flagged/pkg", "version": "1.0.0-dev", "id": 3}, - {"name": "flagged/pkg", "version": "1.0.0-RC", "id": 4}, - {"name": "default/pkg", "version": "1.0.0", "id": 5}, - {"name": "default/pkg", "version": "1.0.0-RC", "id": 6}, - {"name": "default/pkg", "version": "1.0.0-alpha", "id": 7} + [ + {"name": "flagged/pkg", "version": "1.0.0", "id": 1}, + {"name": "flagged/pkg", "version": "1.0.0-beta", "id": 2}, + {"name": "flagged/pkg", "version": "1.0.0-dev", "id": 3}, + {"name": "flagged/pkg", "version": "1.0.0-RC", "id": 4}, + {"name": "default/pkg", "version": "1.0.0", "id": 5}, + {"name": "default/pkg", "version": "1.0.0-RC", "id": 6}, + {"name": "default/pkg", "version": "1.0.0-alpha", "id": 7} + ] ] --EXPECT-- diff --git a/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php b/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php index 75e2b76b0..ac7183eef 100644 --- a/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php +++ b/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php @@ -36,7 +36,7 @@ class PoolBuilderTest extends TestCase /** * @dataProvider getIntegrationTests */ - public function testPoolBuilder($file, $message, $expect, $root, $requestData, $packages, $fixed) + public function testPoolBuilder($file, $message, $expect, $root, $requestData, $packageRepos, $fixed) { $rootAliases = !empty($root['aliases']) ? $root['aliases'] : array(); $minimumStability = !empty($root['minimum-stability']) ? $root['minimum-stability'] : 'stable'; @@ -73,11 +73,13 @@ class PoolBuilderTest extends TestCase }; $repositorySet = new RepositorySet($minimumStability, $stabilityFlags, $rootAliases, $rootReferences); - $repositorySet->addRepository($repo = new ArrayRepository()); - $repositorySet->addRepository($lockedRepo = new LockArrayRepository()); - foreach ($packages as $package) { - $repo->addPackage($loadPackage($package)); + foreach ($packageRepos as $packages) { + $repositorySet->addRepository($repo = new ArrayRepository()); + foreach ($packages as $package) { + $repo->addPackage($loadPackage($package)); + } } + $repositorySet->addRepository($lockedRepo = new LockArrayRepository()); if (isset($requestData['locked'])) { foreach ($requestData['locked'] as $package) { @@ -153,7 +155,7 @@ class PoolBuilderTest extends TestCase $request = JsonFile::parseJson($testData['REQUEST']); $root = !empty($testData['ROOT']) ? JsonFile::parseJson($testData['ROOT']) : array(); - $packages = JsonFile::parseJson($testData['PACKAGES']); + $packageRepos = JsonFile::parseJson($testData['PACKAGE-REPOS']); $fixed = array(); if (!empty($testData['FIXED'])) { $fixed = JsonFile::parseJson($testData['FIXED']); @@ -163,7 +165,7 @@ class PoolBuilderTest extends TestCase die(sprintf('Test "%s" is not valid: '.$e->getMessage(), str_replace($fixturesDir.'/', '', $file))); } - $tests[basename($file)] = array(str_replace($fixturesDir.'/', '', $file), $message, $expect, $root, $request, $packages, $fixed); + $tests[basename($file)] = array(str_replace($fixturesDir.'/', '', $file), $message, $expect, $root, $request, $packageRepos, $fixed); } return $tests; @@ -178,7 +180,7 @@ class PoolBuilderTest extends TestCase 'ROOT' => false, 'REQUEST' => true, 'FIXED' => false, - 'PACKAGES' => true, + 'PACKAGE-REPOS' => true, 'EXPECT' => true, ); From c9201b8e408bc2b921af145e206c4cc87329edc7 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 28 Aug 2020 13:07:11 +0200 Subject: [PATCH 49/54] PoolBuilderTest: Allow setting filter options for repositories --- .../poolbuilder/must-expand-root-reqs.test | 36 +++++++++++++++++++ .../DependencyResolver/PoolBuilderTest.php | 13 +++++-- 2 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/must-expand-root-reqs.test diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/must-expand-root-reqs.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/must-expand-root-reqs.test new file mode 100644 index 000000000..fb52a9b6d --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/must-expand-root-reqs.test @@ -0,0 +1,36 @@ +--TEST-- +Fixed packages do not get loaded from the repos + +--REQUEST-- +{ + "require": { + "some/pkg": "*", + "root/req": "*" + } +} + +--FIXED-- +[ + {"name": "some/pkg", "version": "1.0.3", "id": 1}, + {"name": "dep/dep", "version": "2.1.5", "id": 2} +] + +--PACKAGE-REPOS-- +[ + [ + {"name": "some/pkg", "version": "1.0.0"}, + {"name": "some/pkg", "version": "1.1.0"}, + {"name": "root/req", "version": "1.0.0", "require": {"dep/dep": "2.*"}}, + {"name": "root/req", "version": "2.0.0", "require": {"dep/dep": "3.*"}}, + {"name": "dep/dep", "version": "2.3.4"}, + {"name": "dep/dep", "version": "3.0.1"} + ] +] + +--EXPECT-- +[ + 1, + 2, + "root/req-1.0.0.0", + "root/req-2.0.0.0" +] diff --git a/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php b/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php index ac7183eef..a72e42c96 100644 --- a/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php +++ b/tests/Composer/Test/DependencyResolver/PoolBuilderTest.php @@ -14,6 +14,7 @@ namespace Composer\Test\DependencyResolver; use Composer\IO\NullIO; use Composer\Repository\ArrayRepository; +use Composer\Repository\FilterRepository; use Composer\Repository\LockArrayRepository; use Composer\DependencyResolver\DefaultPolicy; use Composer\DependencyResolver\Pool; @@ -74,8 +75,16 @@ class PoolBuilderTest extends TestCase $repositorySet = new RepositorySet($minimumStability, $stabilityFlags, $rootAliases, $rootReferences); foreach ($packageRepos as $packages) { - $repositorySet->addRepository($repo = new ArrayRepository()); - foreach ($packages as $package) { + $repo = new ArrayRepository(); + if (isset($packages['canonical']) || isset($packages['only']) || isset($packages['exclude'])) { + $options = $packages; + $packages = $options['packages']; + unset($options['packages']); + $repositorySet->addRepository(new FilterRepository($repo, $options)); + } else { + $repositorySet->addRepository($repo); + } + foreach ($packages as $package) { $repo->addPackage($loadPackage($package)); } } From a1e1cd8fa4047a4f65abd853ccccc87c92733bba Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Fri, 28 Aug 2020 14:34:50 +0200 Subject: [PATCH 50/54] Do not trigger Intervals::isSubsetOf() over and over again for platform packages --- src/Composer/DependencyResolver/PoolBuilder.php | 5 +++++ src/Composer/Repository/PlatformRepository.php | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 416a8d1dd..2f8ff9c1c 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -245,6 +245,11 @@ class PoolBuilder private function markPackageNameForLoading(Request $request, $name, ConstraintInterface $constraint) { + // Skip platform requires at this stage + if (PlatformRepository::isPlatformPackage($name)) { + return; + } + // Root require (which was not unfixed) already loaded the maximum range so no // need to check anything here if (isset($this->maxExtendedReqs[$name])) { diff --git a/src/Composer/Repository/PlatformRepository.php b/src/Composer/Repository/PlatformRepository.php index ac6d49525..f6ec91864 100644 --- a/src/Composer/Repository/PlatformRepository.php +++ b/src/Composer/Repository/PlatformRepository.php @@ -593,4 +593,21 @@ class PlatformRepository extends ArrayRepository $this->addPackage($lib); } + + /** + * Check if a package name is a platform package. + * + * @param $name + * @return bool + */ + public static function isPlatformPackage($name) + { + static $cache = array(); + + if (isset($cache[$name])) { + return $cache[$name]; + } + + return $cache[$name] = (bool) preg_match(PlatformRepository::PLATFORM_PACKAGE_REGEX, $name); + } } From 6c4ed247dd31be17847339bfff80871d95924ea7 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 31 Aug 2020 16:00:49 +0200 Subject: [PATCH 51/54] Add a pool builder test for replaces across multiple repos --- .../poolbuilder/multi-repo-replace.test | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace.test diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace.test new file mode 100644 index 000000000..31850dcb6 --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace.test @@ -0,0 +1,95 @@ +--TEST-- +Check that replacers from additional repositories are loaded + +--REQUEST-- +{ + "require": { + "base/package": "^1.0", + "indirect/replacer": "^1.0" + } +} + +--FIXED-- +[ +] + +--PACKAGE-REPOS-- +[ + [ + { + "name": "base/package", + "version": "1.0.0", + "require": { + "shared/dep": "^1.2" + } + }, + { + "name": "shared/dep", + "version": "1.0.0" + }, + { + "name": "shared/dep", + "version": "1.2.0" + } + ], + [ + { + "name": "base/package", + "version": "1.1.0" + }, + { + "name": "shared/dep", + "version": "1.3.0" + } + ], + { + "canonical": false, + "packages": [ + { + "name": "indirect/replacer", + "version": "1.2.0", + "require": { + "replacer/package": "^1.2" + } + }, + { + "name": "replacer/package", + "version": "1.2.0", + "require": { + "shared/dep": "^1.1" + } + }, + { + "name": "shared/dep", + "version": "1.1.0" + } + ] + }, + [ + { + "name": "replacer/package", + "version": "1.0.0", + "require": { + "shared/dep": "^1.0" + } + }, + { + "name": "indirect/replacer", + "version": "1.0.0", + "require": { + "replacer/package": "^1.0" + } + } + ] +] + +--EXPECT-- +[ + "base/package-1.0.0.0", + "indirect/replacer-1.2.0.0", + "indirect/replacer-1.0.0.0", + "shared/dep-1.2.0.0", + "replacer/package-1.2.0.0", + "replacer/package-1.0.0.0", + "shared/dep-1.0.0.0" +] From 976fcd2eb438f21457555d92125b7ef958507d89 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 1 Sep 2020 12:07:12 +0200 Subject: [PATCH 52/54] PoolBuilderTest: Add case for multiple repositories and partial update with replace --- .../multi-repo-replace-partial-update.test | 107 ++++++++++++++++++ .../poolbuilder/multi-repo-replace.test | 3 + 2 files changed, 110 insertions(+) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace-partial-update.test diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace-partial-update.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace-partial-update.test new file mode 100644 index 000000000..01eda5215 --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace-partial-update.test @@ -0,0 +1,107 @@ +--TEST-- +Check that replacers from additional repositories are loaded when doing a partial update + +--REQUEST-- +{ + "require": { + "base/package": "^1.0", + "indirect/replacer": "^1.0" + }, + "locked": [ + {"name": "shared/dep", "version": "1.2.0", "id": 1}, + {"name": "indirect/replacer", "version": "1.2.0", "require": {"replacer/package": "^1.2"}, "id": 2}, + {"name": "replacer/package", "version": "1.2.0", "require": {"shared/dep": "^1.1"}, "replace": {"base/package": "1.2.0"}, "id": 3} + ], + "allowList": [ + "indirect/replacer" + ], + "allowTransitiveDepsNoRootRequire": true +} + +--FIXED-- +[ +] + +--PACKAGE-REPOS-- +[ + [ + { + "name": "base/package", + "version": "1.0.0", + "require": { + "shared/dep": "^1.2" + } + }, + { + "name": "shared/dep", + "version": "1.0.0" + }, + { + "name": "shared/dep", + "version": "1.2.0" + } + ], + [ + { + "name": "base/package", + "version": "1.1.0" + }, + { + "name": "shared/dep", + "version": "1.3.0" + } + ], + { + "canonical": false, + "packages": [ + { + "name": "indirect/replacer", + "version": "1.2.0", + "require": { + "replacer/package": "^1.2" + } + }, + { + "name": "replacer/package", + "version": "1.2.0", + "require": { + "shared/dep": "^1.1" + } + }, + { + "name": "shared/dep", + "version": "1.1.0" + } + ] + }, + [ + { + "name": "replacer/package", + "version": "1.0.0", + "require": { + "shared/dep": "^1.0" + }, + "replace": { + "base/package": "1.0.0" + } + }, + { + "name": "indirect/replacer", + "version": "1.0.0", + "require": { + "replacer/package": "^1.0" + } + } + ] +] + +--EXPECT-- +[ + "indirect/replacer-1.2.0.0", + "indirect/replacer-1.0.0.0", + "replacer/package-1.2.0.0", + "replacer/package-1.0.0.0", + "base/package-1.0.0.0", + "shared/dep-1.0.0.0", + "shared/dep-1.2.0.0" +] diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace.test index 31850dcb6..00c63e4f5 100644 --- a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace.test +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace.test @@ -71,6 +71,9 @@ Check that replacers from additional repositories are loaded "version": "1.0.0", "require": { "shared/dep": "^1.0" + }, + "replace": { + "base/package": "1.2.0" } }, { From b2670945bd853493951045820cc69940d859e192 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 1 Sep 2020 12:23:51 +0200 Subject: [PATCH 53/54] PoolBuilder: clear up comment, matching and identity are different for constraints --- src/Composer/DependencyResolver/PoolBuilder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 2f8ff9c1c..711500dc0 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -266,7 +266,7 @@ class PoolBuilder } // Maybe it was already marked before but not loaded yet. In that case - // we have to extend the constraint (we don't check if they match because + // we have to extend the constraint (we don't check if they are identical because // MultiConstraint::create() will optimize anyway) if (isset($this->packagesToLoad[$name])) { // Already marked for loading and this does not expand the constraint to be loaded, nothing to do From 7b990f37673ead8af4c6dd320727631d7f68bac6 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 1 Sep 2020 12:28:22 +0200 Subject: [PATCH 54/54] PoolBuilder: Move merging of constaints to be loaded into relevant section --- .../DependencyResolver/PoolBuilder.php | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 711500dc0..3c4547be7 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -265,22 +265,22 @@ class PoolBuilder $constraint = $rootRequires[$name]; } - // Maybe it was already marked before but not loaded yet. In that case - // we have to extend the constraint (we don't check if they are identical because - // MultiConstraint::create() will optimize anyway) - if (isset($this->packagesToLoad[$name])) { - // Already marked for loading and this does not expand the constraint to be loaded, nothing to do - if (Intervals::isSubsetOf($constraint, $this->packagesToLoad[$name])) { - return; - } - - // extend the constraint to be loaded - $constraint = Intervals::compactConstraint(MultiConstraint::create(array($this->packagesToLoad[$name], $constraint), false)); - } - // Not yet loaded or already marked for a reload, override the existing constraint // (either it's a new one to load, or it has already been extended above) if (!isset($this->loadedPackages[$name])) { + // Maybe it was already marked before but not loaded yet. In that case + // we have to extend the constraint (we don't check if they are identical because + // MultiConstraint::create() will optimize anyway) + if (isset($this->packagesToLoad[$name])) { + // Already marked for loading and this does not expand the constraint to be loaded, nothing to do + if (Intervals::isSubsetOf($constraint, $this->packagesToLoad[$name])) { + return; + } + + // extend the constraint to be loaded + $constraint = Intervals::compactConstraint(MultiConstraint::create(array($this->packagesToLoad[$name], $constraint), false)); + } + $this->packagesToLoad[$name] = $constraint; return; }