From 82e2a679bfedcab2b4932420dc52367724998467 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 17 Jan 2020 17:30:36 +0100 Subject: [PATCH 1/6] Add TODO note --- 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 d3d14e629..1a41dcd98 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -72,6 +72,9 @@ class PoolBuilder } } + // TODO make sure if a fixed package replaces X packages they are not loaded again in loadNames + // perhaps loadPackage needs to mark them as loadedNames when loading a fixed package? + foreach ($request->getRequires() as $packageName => $constraint) { if (isset($this->loadedNames[$packageName])) { continue; From d58653627a70984f9d38787253bc23dcb6704f1d Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 17 Jan 2020 17:39:36 +0100 Subject: [PATCH 2/6] Optimize loading of deps from fixed packages --- .../DependencyResolver/PoolBuilder.php | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 1a41dcd98..1abaa4113 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -76,15 +76,29 @@ class PoolBuilder // perhaps loadPackage needs to mark them as loadedNames when loading a fixed package? foreach ($request->getRequires() as $packageName => $constraint) { + // fixed packages do not need to get filtered as they are pinned already if (isset($this->loadedNames[$packageName])) { continue; } - // TODO currently lock above is always NULL if we adjust that, this needs to merge constraints - // TODO does it really make sense that we can have install requests for the same package that is actively locked with non-matching constraints? - // also see the solver-problems.test test case - $constraint = array_key_exists($packageName, $loadNames) ? null : $constraint; - $loadNames[$packageName] = $constraint; - $this->nameConstraints[$packageName] = $constraint ? new MultiConstraint(array($constraint), false) : null; + + $loadNames[$packageName] = null; + if ($constraint) { + if (!array_key_exists($packageName, $this->nameConstraints)) { + $this->nameConstraints[$packageName] = new MultiConstraint(array($constraint), false); + } elseif ($this->nameConstraints[$packageName]) { + // TODO addConstraint function? + $this->nameConstraints[$packageName] = new MultiConstraint(array_merge(array($constraint), $this->nameConstraints[$packageName]->getConstraints()), false); + } + } + } + + // all the merged constraints from install requests + fixed packages can be applied + // when loading package metadata already, as these are set in stone + foreach ($this->nameConstraints as $package => $constraint) { + if ($constraint !== null && array_key_exists($package, $loadNames)) { + $loadNames[$package] = $constraint; + unset($this->nameConstraints[$package]); + } } while (!empty($loadNames)) { @@ -210,6 +224,8 @@ class PoolBuilder $loadNames[$require] = null; } if ($linkConstraint = $link->getConstraint()) { + // TODO check if linkConstraint is EmptyConstraint then set to null as well? + if (!array_key_exists($require, $this->nameConstraints)) { $this->nameConstraints[$require] = new MultiConstraint(array($linkConstraint), false); } elseif ($this->nameConstraints[$require]) { From 59bc957e760b21e0987dfb87ecb6d995f6596c62 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Wed, 29 Jan 2020 17:57:45 +0100 Subject: [PATCH 3/6] Simplify loading of fixed and root require packages in pool builder additionally mark all packages replaced by fixed packages as loaded as there is no need to load those names at all, since the fixed package will provide them --- .../DependencyResolver/PoolBuilder.php | 38 ++++++++++--------- .../Fixtures/installer/solver-problems.test | 2 + 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 1abaa4113..dad0da6ba 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -53,14 +53,21 @@ class PoolBuilder public function buildPool(array $repositories, Request $request) { - $pool = new Pool(); - - // TODO do we really want the request here? kind of want a root requirements thingy instead $loadNames = array(); foreach ($request->getFixedPackages() as $package) { + // TODO do we need to add this to nameConstraints at all? $this->nameConstraints[$package->getName()] = null; $this->loadedNames[$package->getName()] = true; - unset($loadNames[$package->getName()]); + + // 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; + } + + // TODO in how far can we do the above for conflicts? It's more tricky cause conflicts can be limited to + // specific versions while replace is a conflict with all versions of the name + if ( $package->getRepository() instanceof RootPackageRepository || $package->getRepository() instanceof PlatformRepository @@ -72,24 +79,14 @@ class PoolBuilder } } - // TODO make sure if a fixed package replaces X packages they are not loaded again in loadNames - // perhaps loadPackage needs to mark them as loadedNames when loading a fixed package? - foreach ($request->getRequires() as $packageName => $constraint) { - // fixed packages do not need to get filtered as they are pinned already + // 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] = null; - if ($constraint) { - if (!array_key_exists($packageName, $this->nameConstraints)) { - $this->nameConstraints[$packageName] = new MultiConstraint(array($constraint), false); - } elseif ($this->nameConstraints[$packageName]) { - // TODO addConstraint function? - $this->nameConstraints[$packageName] = new MultiConstraint(array_merge(array($constraint), $this->nameConstraints[$packageName]->getConstraints()), false); - } - } + $loadNames[$packageName] = $constraint; + $this->nameConstraints[$packageName] = $constraint ? new MultiConstraint(array($constraint), false) : null; } // all the merged constraints from install requests + fixed packages can be applied @@ -101,6 +98,13 @@ class PoolBuilder } } + // clean up loadNames for anything we manually marked loaded above + foreach ($loadNames as $name => $void) { + if (isset($this->loadedNames[$name])) { + unset($loadNames[$name]); + } + } + while (!empty($loadNames)) { foreach ($loadNames as $name => $void) { $this->loadedNames[$name] = true; diff --git a/tests/Composer/Test/Fixtures/installer/solver-problems.test b/tests/Composer/Test/Fixtures/installer/solver-problems.test index 005047127..11aede917 100644 --- a/tests/Composer/Test/Fixtures/installer/solver-problems.test +++ b/tests/Composer/Test/Fixtures/installer/solver-problems.test @@ -116,6 +116,8 @@ Your requirements could not be resolved to an installable set of packages. - Root composer.json requires unstable/package 2.*, found unstable/package[2.0.0-alpha] but it does not match your minimum-stability. Problem 3 - Root composer.json requires non-existent/pkg, it could not be found in any version, there may be a typo in the package name. + Problem 4 + - The requested package dependency/pkg could not be found in any version, there may be a typo in the package name. Problem 4 - Root composer.json requires stable-requiree-excluded/pkg 1.0.1, found stable-requiree-excluded/pkg[1.0.1] but the package is fixed to 1.0.0 (lock file version) by a partial update and that version does not match. Make sure you whitelist it for update. Problem 5 From bbdbb3517b59802390f74e47a499c6faf5149117 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Wed, 29 Jan 2020 18:00:56 +0100 Subject: [PATCH 4/6] PoolBuilder: Drop name constraints loop, already set earlier in same code --- src/Composer/DependencyResolver/PoolBuilder.php | 10 +--------- .../Test/Fixtures/installer/solver-problems.test | 2 -- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index dad0da6ba..3b11de76e 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -86,18 +86,10 @@ class PoolBuilder } $loadNames[$packageName] = $constraint; + // TODO do we even need to set these in name constraints or can we skip them entirely? $this->nameConstraints[$packageName] = $constraint ? new MultiConstraint(array($constraint), false) : null; } - // all the merged constraints from install requests + fixed packages can be applied - // when loading package metadata already, as these are set in stone - foreach ($this->nameConstraints as $package => $constraint) { - if ($constraint !== null && array_key_exists($package, $loadNames)) { - $loadNames[$package] = $constraint; - unset($this->nameConstraints[$package]); - } - } - // clean up loadNames for anything we manually marked loaded above foreach ($loadNames as $name => $void) { if (isset($this->loadedNames[$name])) { diff --git a/tests/Composer/Test/Fixtures/installer/solver-problems.test b/tests/Composer/Test/Fixtures/installer/solver-problems.test index 11aede917..005047127 100644 --- a/tests/Composer/Test/Fixtures/installer/solver-problems.test +++ b/tests/Composer/Test/Fixtures/installer/solver-problems.test @@ -116,8 +116,6 @@ Your requirements could not be resolved to an installable set of packages. - Root composer.json requires unstable/package 2.*, found unstable/package[2.0.0-alpha] but it does not match your minimum-stability. Problem 3 - Root composer.json requires non-existent/pkg, it could not be found in any version, there may be a typo in the package name. - Problem 4 - - The requested package dependency/pkg could not be found in any version, there may be a typo in the package name. Problem 4 - Root composer.json requires stable-requiree-excluded/pkg 1.0.1, found stable-requiree-excluded/pkg[1.0.1] but the package is fixed to 1.0.0 (lock file version) by a partial update and that version does not match. Make sure you whitelist it for update. Problem 5 From 8a6382d78ddeb5c2497400526b3501551ecb6b35 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 12 Mar 2020 18:12:05 +0100 Subject: [PATCH 5/6] Remove unnecessary TODOs and skip EmptyConstraint like null --- src/Composer/DependencyResolver/PoolBuilder.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 3b11de76e..b570e6305 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -20,6 +20,7 @@ use Composer\Package\Version\StabilityFilter; use Composer\Repository\PlatformRepository; use Composer\Repository\RootPackageRepository; use Composer\Semver\Constraint\Constraint; +use Composer\Semver\Constraint\EmptyConstraint; use Composer\Semver\Constraint\MultiConstraint; use Composer\EventDispatcher\EventDispatcher; use Composer\Plugin\PrePoolCreateEvent; @@ -55,7 +56,6 @@ class PoolBuilder { $loadNames = array(); foreach ($request->getFixedPackages() as $package) { - // TODO do we need to add this to nameConstraints at all? $this->nameConstraints[$package->getName()] = null; $this->loadedNames[$package->getName()] = true; @@ -86,7 +86,6 @@ class PoolBuilder } $loadNames[$packageName] = $constraint; - // TODO do we even need to set these in name constraints or can we skip them entirely? $this->nameConstraints[$packageName] = $constraint ? new MultiConstraint(array($constraint), false) : null; } @@ -219,9 +218,9 @@ class PoolBuilder if (!isset($this->loadedNames[$require])) { $loadNames[$require] = null; } - if ($linkConstraint = $link->getConstraint()) { - // TODO check if linkConstraint is EmptyConstraint then set to null as well? + $linkConstraint = $link->getConstraint() + if ($linkConstraint && !($linkConstraint instanceof EmptyConstraint)) { if (!array_key_exists($require, $this->nameConstraints)) { $this->nameConstraints[$require] = new MultiConstraint(array($linkConstraint), false); } elseif ($this->nameConstraints[$require]) { From cf5513f28d350424ce6ba2ce1fbbadf6eab9be9f Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 13 Mar 2020 11:50:35 +0100 Subject: [PATCH 6/6] Fix syntax error --- 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 b570e6305..718f340e7 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -219,7 +219,7 @@ class PoolBuilder $loadNames[$require] = null; } - $linkConstraint = $link->getConstraint() + $linkConstraint = $link->getConstraint(); if ($linkConstraint && !($linkConstraint instanceof EmptyConstraint)) { if (!array_key_exists($require, $this->nameConstraints)) { $this->nameConstraints[$require] = new MultiConstraint(array($linkConstraint), false);