From 13b7527fca1d804688d38d76999e2d4827bc94c9 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 12 Nov 2021 12:42:34 +0100 Subject: [PATCH 1/3] Fix unlocking of replacers when a replaced package is unlocked in partial updates --- .../DependencyResolver/PoolBuilder.php | 27 +++-- .../partial-update-unfixing-replacers.test | 102 ++++++++++++++++++ 2 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-replacers.test diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index bd06af28b..7840d13bf 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -535,7 +535,19 @@ class PoolBuilder // as long as it was not unfixed yet && isset($this->skippedLoad[$this->skippedLoad[$name]]) ) { - $this->unlockPackage($request, $this->skippedLoad[$name]); + $replacerName = $this->skippedLoad[$name]; + $this->unlockPackage($request, $replacerName); + + if ($this->isRootRequire($request, $replacerName)) { + $this->markPackageNameForLoading($request, $replacerName, new MatchAllConstraint); + } else { + foreach ($this->packages as $loadedPackage) { + $requires = $loadedPackage->getRequires(); + if (isset($requires[$replacerName])) { + $this->markPackageNameForLoading($request, $replacerName, $requires[$replacerName]->getConstraint()); + } + } + } } unset($this->skippedLoad[$name], $this->loadedPackages[$name], $this->maxExtendedReqs[$name]); @@ -551,11 +563,14 @@ class PoolBuilder // also loaded, as they were previously ignored because the locked (now unlocked) package already // satisfied their requirements foreach ($request->getFixedOrLockedPackages() as $fixedOrLockedPackage) { - if ($fixedOrLockedPackage !== $lockedPackage && isset($this->skippedLoad[$fixedOrLockedPackage->getName()])) { - foreach ($fixedOrLockedPackage->getRequires() as $requireLink) { - if ($requireLink->getTarget() === $lockedPackage->getName()) { - $this->markPackageNameForLoading($request, $lockedPackage->getName(), $requireLink->getConstraint()); - } + if ($fixedOrLockedPackage === $lockedPackage) { + continue; + } + + if (isset($this->skippedLoad[$fixedOrLockedPackage->getName()])) { + $requires = $fixedOrLockedPackage->getRequires(); + if (isset($requires[$lockedPackage->getName()])) { + $this->markPackageNameForLoading($request, $lockedPackage->getName(), $requires[$lockedPackage->getName()]->getConstraint()); } } } diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-replacers.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-replacers.test new file mode 100644 index 000000000..26176228e --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-replacers.test @@ -0,0 +1,102 @@ +--TEST-- +Partially updating with deps a root requirement which depends on packages should load all available versions for the path repo packages' dependencies. + +--REQUEST-- +{ + "require": { + "root/update": "*", + "root-required/replacer": "*", + "replacer/pkg": "*", + "root/noupdate": "*" + }, + "locked": [ + { + "name": "replacer/pkg", + "version": "1.0.0", + "require": { + "root-required/replacer-replaced": "1.*", + "transitive/dep-of-replacer": "1.*", + "transitive/replacer-replaced": "1.*" + }, + "replace": { + "replaced/pkg": "1.0.0" + } + }, + { + "name": "pkg/with-replaced-deps", + "version": "1.0.0", + "require": { + "transitive/dep": "1.*", + "root-required/replacer-replaced": "1.*", + "transitive/replacer-replaced": "1.*" + } + }, + {"name": "root/update", "version": "1.0.1", "require": {"pkg/with-replaced-deps": ">=1.0.1", "replaced/pkg": "*"}}, + {"name": "root/noupdate", "version": "1.0.1", "require": {"transitive/replacer": "^2"}}, + {"name": "transitive/dep", "version": "1.0.0"}, + {"name": "root-required/replacer", "version": "1.0.0", "replace": {"root-required/replacer-replaced": "1.0.0"}}, + {"name": "transitive/dep-of-replacer", "version": "1.0.0"}, + {"name": "transitive/replacer", "version": "1.0.0", "replace": {"transitive/replacer-replaced": "1.0.0"}} + ], + "allowList": [ + "root/update" + ], + "allowTransitiveDeps": true +} + +--FIXED-- +[ +] + +--PACKAGE-REPOS-- +[ + [ + { + "name": "replacer/pkg", + "version": "2.0.0", + "require": { + "root-required/replacer-replaced": ">=1.0.3", + "transitive/dep-of-replacer": "2.*", + "transitive/replacer-replaced": "2.*" + }, + "replace": { + "replaced/pkg": "2.0.0" + } + }, + { + "name": "pkg/with-replaced-deps", + "version": "2.0.0", + "require": { + "transitive/dep": "2.*", + "root-required/replacer-replaced": "2.*", + "transitive/replacer-replaced": ">=1.0.3" + } + }, + {"name": "root/update", "version": "1.0.4", "require": {"pkg/with-replaced-deps": ">=1.0.1", "replaced/pkg": "*"}}, + {"name": "root/noupdate", "version": "1.0.1", "require": {"transitive/replacer": "^2"}}, + {"name": "transitive/dep", "version": "1.0.0"}, + {"name": "transitive/dep", "version": "2.0.2"}, + {"name": "root-required/replacer", "version": "1.0.0", "replace": {"root-required/replacer-replaced": "1.0.0"}}, + {"name": "root-required/replacer", "version": "1.0.3", "replace": {"root-required/replacer-replaced": "1.0.3"}}, + {"name": "root-required/replacer", "version": "2.0.4", "replace": {"root-required/replacer-replaced": "2.0.4"}}, + {"name": "transitive/dep-of-replacer", "version": "1.0.0"}, + {"name": "transitive/dep-of-replacer", "version": "2.0.2"}, + {"name": "transitive/replacer", "version": "1.0.0", "replace": {"transitive/replacer-replaced": "1.0.0"}}, + {"name": "transitive/replacer", "version": "1.0.3", "replace": {"transitive/replacer-replaced": "1.0.3"}}, + {"name": "transitive/replacer", "version": "2.0.4", "replace": {"transitive/replacer-replaced": "2.0.4"}} + ] +] + +--EXPECT-- +[ + "root/noupdate-1.0.1.0 (locked)", + "root/update-1.0.4.0", + "replacer/pkg-2.0.0.0", + "pkg/with-replaced-deps-2.0.0.0", + "transitive/dep-2.0.2.0", + "root-required/replacer-1.0.0.0", + "root-required/replacer-1.0.3.0", + "root-required/replacer-2.0.4.0", + "transitive/dep-of-replacer-2.0.2.0", + "transitive/replacer-2.0.4.0" +] From 3242de2438ec69feec2fa881f5b475bc813c7049 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Tue, 23 Nov 2021 12:58:59 +0100 Subject: [PATCH 2/3] Backport tests from #9538 and fix everything --- .../DependencyResolver/PoolBuilder.php | 112 +++++++++++---- ...ulti-repo-replace-partial-update-all.test} | 4 +- ...ate-unfixing-with-replacers-providers.test | 87 ++++++++++++ .../installer/github-issues-4795.test | 2 +- ...tial-update-with-dependencies-provide.test | 65 +++++++++ ...tial-update-with-dependencies-replace.test | 62 ++++++++ .../partial-update-with-deps-warns-root.test | 133 ++++++++++++++++++ tests/deprecations-8.1.json | 4 +- 8 files changed, 439 insertions(+), 30 deletions(-) rename tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/{multi-repo-replace-partial-update.test => multi-repo-replace-partial-update-all.test} (96%) create mode 100644 tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-with-replacers-providers.test create mode 100644 tests/Composer/Test/Fixtures/installer/partial-update-with-dependencies-provide.test create mode 100644 tests/Composer/Test/Fixtures/installer/partial-update-with-dependencies-replace.test create mode 100644 tests/Composer/Test/Fixtures/installer/partial-update-with-deps-warns-root.test diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index 7840d13bf..cc211b9e7 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -99,7 +99,7 @@ class PoolBuilder private $unacceptableFixedOrLockedPackages = array(); /** @var string[] */ private $updateAllowList = array(); - /** @var array */ + /** @var array> */ private $skippedLoad = array(); /** @@ -158,9 +158,8 @@ class PoolBuilder $request->lockPackage($lockedPackage); $lockedName = $lockedPackage->getName(); // remember which packages we skipped loading remote content for in this partial update - $this->skippedLoad[$lockedName] = $lockedName; - foreach ($lockedPackage->getReplaces() as $link) { - $this->skippedLoad[$link->getTarget()] = $lockedName; + foreach ($lockedPackage->getNames() as $name) { + $this->skippedLoad[$name][] = $lockedPackage; } } } @@ -422,12 +421,18 @@ class PoolBuilder // looking at a package which needs to be updated we need to unlock 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()) { - if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$require])) { + $skippedRootRequires = $this->getSkippedRootRequires($request, $require); + + if ($request->getUpdateAllowTransitiveRootDependencies() || !$skippedRootRequires) { $this->unlockPackage($request, $require); $this->markPackageNameForLoading($request, $require, $linkConstraint); - } elseif (!isset($this->updateAllowWarned[$this->skippedLoad[$require]])) { - $this->updateAllowWarned[$this->skippedLoad[$require]] = true; - $this->io->writeError('Dependency "'.$this->skippedLoad[$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 (-W) to include root dependencies.'); + } else { + foreach ($skippedRootRequires as $rootRequire) { + if (!isset($this->updateAllowWarned[$rootRequire])) { + $this->updateAllowWarned[$rootRequire] = true; + $this->io->writeError('Dependency '.$rootRequire.' is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies (-W) to include root dependencies.'); + } + } } } } else { @@ -441,12 +446,18 @@ class PoolBuilder foreach ($package->getReplaces() as $link) { $replace = $link->getTarget(); if (isset($this->loadedPackages[$replace], $this->skippedLoad[$replace])) { - if ($request->getUpdateAllowTransitiveRootDependencies() || !$this->isRootRequire($request, $this->skippedLoad[$replace])) { + $skippedRootRequires = $this->getSkippedRootRequires($request, $replace); + + if ($request->getUpdateAllowTransitiveRootDependencies() || !$skippedRootRequires) { $this->unlockPackage($request, $replace); $this->markPackageNameForLoading($request, $replace, $link->getConstraint()); - } elseif ($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 (-W) to include root dependencies.'); + } else { + foreach ($skippedRootRequires as $rootRequire) { + if (!isset($this->updateAllowWarned[$rootRequire])) { + $this->updateAllowWarned[$rootRequire] = true; + $this->io->writeError('Dependency '.$rootRequire.' is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies (-W) to include root dependencies.'); + } + } } } } @@ -466,6 +477,56 @@ class PoolBuilder return isset($rootRequires[$name]); } + /** + * @param string $name + * @return string[] + */ + private function getSkippedRootRequires(Request $request, $name) + { + if (!isset($this->skippedLoad[$name])) { + return array(); + } + + $rootRequires = $request->getRequires(); + $matches = array(); + + if (isset($rootRequires[$name])) { + return array_map(function (PackageInterface $package) use ($name) { + if ($name !== $package->getName()) { + foreach ($package->getReplaces() as $link) { + if ($link->getTarget() === $name) { + return $package->getName() .' (via replace of '.$name.')'; + } + } + return $package->getName() .' (via provide of '.$name.')'; + } + + return $package->getName(); + }, $this->skippedLoad[$name]); + } + + foreach ($this->skippedLoad[$name] as $providedBy) { + foreach ($providedBy->getNames() as $providedName) { + if (isset($rootRequires[$providedName])) { + if ($name !== $providedBy->getName()) { + foreach ($providedBy->getReplaces() as $link) { + if ($link->getTarget() === $name) { + $matches[] = $providedBy->getName() .' (via replace of '.$name.')'; + continue 2; + } + } + $matches[] = $providedBy->getName() .' (via provide of '.$name.')'; + continue; + } + + $matches[] = $providedBy->getName(); + } + } + } + + return $matches; + } + /** * Checks whether the update allow list allows this package in the lock file to be updated * @@ -529,22 +590,23 @@ class PoolBuilder */ private function unlockPackage(Request $request, $name) { - if ( + foreach ($this->skippedLoad[$name] as $providedBy) { + $providedBy = $providedBy->getName(); // if we unfixed a replaced package name, we also need to unfix the replacer itself - $this->skippedLoad[$name] !== $name // as long as it was not unfixed yet - && isset($this->skippedLoad[$this->skippedLoad[$name]]) - ) { - $replacerName = $this->skippedLoad[$name]; - $this->unlockPackage($request, $replacerName); + if ($providedBy !== $name && isset($this->skippedLoad[$providedBy])) { + if ($request->getUpdateAllowTransitiveRootDependencies() || (!$this->isRootRequire($request, $name) && !$this->isRootRequire($request, $providedBy))) { + $this->unlockPackage($request, $providedBy); - if ($this->isRootRequire($request, $replacerName)) { - $this->markPackageNameForLoading($request, $replacerName, new MatchAllConstraint); - } else { - foreach ($this->packages as $loadedPackage) { - $requires = $loadedPackage->getRequires(); - if (isset($requires[$replacerName])) { - $this->markPackageNameForLoading($request, $replacerName, $requires[$replacerName]->getConstraint()); + if ($this->isRootRequire($request, $providedBy)) { + $this->markPackageNameForLoading($request, $providedBy, new MatchAllConstraint); + } else { + foreach ($this->packages as $loadedPackage) { + $requires = $loadedPackage->getRequires(); + if (isset($requires[$providedBy])) { + $this->markPackageNameForLoading($request, $providedBy, $requires[$providedBy]->getConstraint()); + } + } } } } 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-all.test similarity index 96% rename from tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace-partial-update.test rename to tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/multi-repo-replace-partial-update-all.test index 2e1705cc0..72e88141a 100644 --- 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-all.test @@ -1,5 +1,5 @@ --TEST-- -Check that replacers from additional repositories are loaded when doing a partial update +Check that replacers from additional repositories are loaded when doing a partial update allowing all transitive deps --REQUEST-- { @@ -15,7 +15,7 @@ Check that replacers from additional repositories are loaded when doing a partia "allowList": [ "indirect/replacer" ], - "allowTransitiveDepsNoRootRequire": true + "allowTransitiveDeps": true } --FIXED-- diff --git a/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-with-replacers-providers.test b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-with-replacers-providers.test new file mode 100644 index 000000000..12e4245dc --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/Fixtures/poolbuilder/partial-update-unfixing-with-replacers-providers.test @@ -0,0 +1,87 @@ +--TEST-- +Check that replacers/providers which replace/provide a root requirement do not get unlocked + +--REQUEST-- +{ + "require": { + "base/package": "^1.0", + "base/package2": "^1.0", + "indirect/replacer": "^1.0" + }, + "locked": [ + {"name": "shared/dep", "version": "1.2.0", "id": 1}, + {"name": "shared/dep2", "version": "1.2.0", "id": 2}, + {"name": "indirect/replacer", "version": "1.2.0", "require": {"replacer/package": "^1.2", "provider/package": "^1.2"}, "id": 3}, + {"name": "replacer/package", "version": "1.2.0", "require": {"shared/dep": "^1.1"}, "replace": {"base/package": "1.2.0"}, "id": 4}, + {"name": "provider/package", "version": "1.2.0", "require": {"shared/dep2": "^1.1"}, "provide": {"base/package2": "1.2.0"}, "id": 5} + ], + "allowList": [ + "indirect/replacer" + ], + "allowTransitiveDepsNoRootRequire": true +} + +--FIXED-- +[ +] + +--PACKAGE-REPOS-- +[ + [ + { + "name": "base/package", + "version": "1.0.0", + "require": { + "shared/dep": "^1.2" + } + }, + { + "name": "base/package2", + "version": "1.0.0", + "require": { + "shared/dep2": "^1.2" + } + }, + { + "name": "shared/dep", + "version": "1.0.0" + }, + { + "name": "shared/dep", + "version": "1.2.0" + }, + { + "name": "shared/dep2", + "version": "1.0.0" + }, + { + "name": "shared/dep2", + "version": "1.2.0" + }, + { + "name": "indirect/replacer", + "version": "1.2.0", + "require": { + "replacer/package": "^1.2" + } + }, + { + "name": "indirect/replacer", + "version": "1.0.0", + "require": { + "replacer/package": "^1.0" + } + } + ] +] + +--EXPECT-- +[ + 1, + 4, + 5, + "base/package2-1.0.0.0", + "indirect/replacer-1.2.0.0", + "indirect/replacer-1.0.0.0", + "shared/dep2-1.2.0.0" +] diff --git a/tests/Composer/Test/Fixtures/installer/github-issues-4795.test b/tests/Composer/Test/Fixtures/installer/github-issues-4795.test index 36f4fc76d..b619785f0 100644 --- a/tests/Composer/Test/Fixtures/installer/github-issues-4795.test +++ b/tests/Composer/Test/Fixtures/installer/github-issues-4795.test @@ -50,7 +50,7 @@ update b/b --with-dependencies --EXPECT-OUTPUT-- Loading composer repositories with package information -Dependency "a/a" is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies (-W) to include root dependencies. +Dependency a/a is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies (-W) to include root dependencies. Updating dependencies Nothing to modify in lock file Writing lock file diff --git a/tests/Composer/Test/Fixtures/installer/partial-update-with-dependencies-provide.test b/tests/Composer/Test/Fixtures/installer/partial-update-with-dependencies-provide.test new file mode 100644 index 000000000..8d45e9cd9 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/partial-update-with-dependencies-provide.test @@ -0,0 +1,65 @@ +--TEST-- +Ensure a partial update of a dependency updates dependencies which provide its requirements. + +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + {"name": "root/req1", "version": "1.0.0", "require": {"virtual/pkg1": "1.*"}}, + {"name": "root/req1", "version": "2.0.0", "require": {"virtual/pkg1": "2.*"}}, + {"name": "root/req2", "version": "1.0.0", "require": {"dep/pkg1": "1.*", "dep/pkg2": "1.*"}}, + {"name": "dep/pkg1", "version": "1.0.0", "provide": {"virtual/pkg1": "1.0.0"}}, + {"name": "dep/pkg1", "version": "1.2.0", "provide": {"virtual/pkg1": "2.0.0"}}, + {"name": "dep/pkg2", "version": "1.0.0", "provide": {"virtual/pkg1": "1.0.0"}}, + {"name": "dep/pkg2", "version": "1.2.0", "provide": {"virtual/pkg1": "2.0.0"}} + ] + } + ], + "require": { + "root/req1": "*", + "root/req2": "*" + } +} +--LOCK-- +{ + "packages": [ + {"name": "dep/pkg1", "version": "1.0.0", "type": "library", "provide": {"virtual/pkg1": "1.0.0"}}, + {"name": "dep/pkg2", "version": "1.0.0", "type": "library", "provide": {"virtual/pkg1": "1.0.0"}}, + {"name": "root/req1", "version": "1.0.0", "type": "library", "require": {"virtual/pkg1": "1.*"}}, + {"name": "root/req2", "version": "1.0.0", "type": "library", "require": {"dep/pkg1": "1.*", "dep/pkg2": "1.*"}} + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": {}, + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} +--RUN-- +update root/req1 --with-dependencies +--EXPECT-LOCK-- +{ + "packages": [ + {"name": "dep/pkg1", "version": "1.2.0", "type": "library", "provide": {"virtual/pkg1": "2.0.0"}}, + {"name": "dep/pkg2", "version": "1.2.0", "type": "library", "provide": {"virtual/pkg1": "2.0.0"}}, + {"name": "root/req1", "version": "2.0.0", "type": "library", "require": {"virtual/pkg1": "2.*"}}, + {"name": "root/req2", "version": "1.0.0", "type": "library", "require": {"dep/pkg1": "1.*", "dep/pkg2": "1.*"}} + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": [], + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} +--EXPECT-- +Installing dep/pkg1 (1.2.0) +Installing dep/pkg2 (1.2.0) +Installing root/req1 (2.0.0) +Installing root/req2 (1.0.0) diff --git a/tests/Composer/Test/Fixtures/installer/partial-update-with-dependencies-replace.test b/tests/Composer/Test/Fixtures/installer/partial-update-with-dependencies-replace.test new file mode 100644 index 000000000..39994422d --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/partial-update-with-dependencies-replace.test @@ -0,0 +1,62 @@ +--TEST-- +Ensure a partial update of a dependency updates dependencies which replace one of its requirements. + +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + {"name": "root/req1", "version": "1.0.0", "require": {"replaced/pkg1": "1.*"}}, + {"name": "root/req1", "version": "2.0.0", "require": {"replaced/pkg1": "2.*"}}, + {"name": "root/req2", "version": "1.0.0", "require": {"dep/pkg1": "1.*"}}, + {"name": "replaced/pkg1", "version": "1.0.0"}, + {"name": "replaced/pkg1", "version": "2.0.0"}, + {"name": "dep/pkg1", "version": "1.0.0", "replace": {"replaced/pkg1": "1.0.0"}}, + {"name": "dep/pkg1", "version": "1.2.0", "replace": {"replaced/pkg1": "2.0.0"}} + ] + } + ], + "require": { + "root/req1": "*", + "root/req2": "*" + } +} +--LOCK-- +{ + "packages": [ + {"name": "dep/pkg1", "version": "1.0.0", "type": "library", "replace": {"replaced/pkg1": "1.0.0"}}, + {"name": "root/req1", "version": "1.0.0", "type": "library", "require": {"replaced/pkg1": "1.*"}}, + {"name": "root/req2", "version": "1.0.0", "type": "library", "require": {"dep/pkg1": "1.*"}} + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": {}, + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} +--RUN-- +update root/req1 --with-dependencies +--EXPECT-LOCK-- +{ + "packages": [ + {"name": "dep/pkg1", "version": "1.2.0", "type": "library", "replace": {"replaced/pkg1": "2.0.0"}}, + {"name": "root/req1", "version": "2.0.0", "type": "library", "require": {"replaced/pkg1": "2.*"}}, + {"name": "root/req2", "version": "1.0.0", "type": "library", "require": {"dep/pkg1": "1.*"}} + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": [], + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} +--EXPECT-- +Installing dep/pkg1 (1.2.0) +Installing root/req1 (2.0.0) +Installing root/req2 (1.0.0) diff --git a/tests/Composer/Test/Fixtures/installer/partial-update-with-deps-warns-root.test b/tests/Composer/Test/Fixtures/installer/partial-update-with-deps-warns-root.test new file mode 100644 index 000000000..b1aa96b84 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/partial-update-with-deps-warns-root.test @@ -0,0 +1,133 @@ +--TEST-- +Ensure that a partial update of a dependency does not conflict if the only way to proceed is using an old locked version. + +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { + "name": "update/pkg1", "version": "1.0.0", "require": { + "dep/pkg2": "*", + "root/pkg3": "*", + "root/provided-pkg4": "*", + "root/replaced-pkg8": "*" + } + }, + { + "name": "update/pkg1", "version": "2.0.0", "require": { + "dep/pkg2": "*", + "root/pkg3": "*", + "root/provided-pkg4": "*", + "root/replaced-pkg8": "*" + } + }, + {"name": "dep/pkg2", "version": "1.0.0"}, + {"name": "dep/pkg2", "version": "2.0.0"}, + {"name": "root/pkg3", "version": "1.0.0"}, + {"name": "root/pkg3", "version": "2.0.0"}, + {"name": "dep/pkg5", "version": "1.0.0", "provide": {"root/provided-pkg4": "1.0.0"}}, + {"name": "dep/pkg5", "version": "2.0.0", "provide": {"root/provided-pkg4": "2.0.0"}}, + {"name": "dep/pkg6", "version": "1.0.0", "provide": {"root/provided-pkg4": "1.0.0"}}, + {"name": "dep/pkg6", "version": "2.0.0", "provide": {"root/provided-pkg4": "2.0.0"}}, + {"name": "root/pkg7", "version": "1.0.0", "require": {"dep/pkg5": "*", "dep/pkg6": "*"}}, + {"name": "dep/pkg9", "version": "1.0.0", "replace": {"root/replaced-pkg8": "1.0.0"}}, + {"name": "dep/pkg9", "version": "2.0.0", "replace": {"root/replaced-pkg8": "2.0.0"}}, + {"name": "root/replaced-pkg8", "version": "1.0.0"}, + {"name": "root/replaced-pkg8", "version": "2.0.0"}, + {"name": "root/pkg10", "version": "1.0.0", "require": {"dep/pkg9": "*"}} + ] + } + ], + "require": { + "update/pkg1": "*", + "root/pkg3": "*", + "root/provided-pkg4": "*", + "root/pkg7": "*", + "root/replaced-pkg8": "*", + "root/pkg10": "*" + } +} +--LOCK-- +{ + "packages": [ + { + "name": "update/pkg1", "version": "1.0.0", "type": "library", "require": { + "dep/pkg2": "*", + "root/pkg3": "*", + "root/provided-pkg4": "*", + "root/replaced-pkg8": "*" + } + }, + {"name": "dep/pkg2", "version": "1.0.0", "type": "library"}, + {"name": "root/pkg3", "version": "1.0.0", "type": "library"}, + {"name": "dep/pkg5", "version": "1.0.0", "type": "library", "provide": {"root/provided-pkg4": "1.0.0"}}, + {"name": "dep/pkg6", "version": "1.0.0", "type": "library", "provide": {"root/provided-pkg4": "1.0.0"}}, + {"name": "root/pkg7", "version": "1.0.0", "type": "library", "require": {"dep/pkg5": "*", "dep/pkg6": "*"}}, + {"name": "dep/pkg9", "version": "1.0.0", "type": "library", "replace": {"root/replaced-pkg8": "1.0.0"}}, + {"name": "root/pkg10", "version": "1.0.0", "type": "library", "require": {"dep/pkg9": "*"}} + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": {}, + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} +--RUN-- +update update/pkg1 --with-dependencies +--EXPECT-LOCK-- +{ + "packages": [ + {"name": "dep/pkg2", "version": "2.0.0", "type": "library"}, + {"name": "dep/pkg5", "version": "1.0.0", "type": "library", "provide": {"root/provided-pkg4": "1.0.0"}}, + {"name": "dep/pkg6", "version": "1.0.0", "type": "library", "provide": {"root/provided-pkg4": "1.0.0"}}, + {"name": "dep/pkg9", "version": "1.0.0", "type": "library", "replace": {"root/replaced-pkg8": "1.0.0"}}, + {"name": "root/pkg10", "version": "1.0.0", "type": "library", "require": {"dep/pkg9": "*"}}, + {"name": "root/pkg3", "version": "1.0.0", "type": "library"}, + {"name": "root/pkg7", "version": "1.0.0", "type": "library", "require": {"dep/pkg5": "*", "dep/pkg6": "*"}}, + { + "name": "update/pkg1", "version": "2.0.0", "type": "library", "require": { + "dep/pkg2": "*", + "root/pkg3": "*", + "root/provided-pkg4": "*", + "root/replaced-pkg8": "*" + } + } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": [], + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} +--EXPECT-OUTPUT-- +Loading composer repositories with package information +Dependency root/pkg3 is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies (-W) to include root dependencies. +Dependency dep/pkg5 (via provide of root/provided-pkg4) is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies (-W) to include root dependencies. +Dependency dep/pkg6 (via provide of root/provided-pkg4) is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies (-W) to include root dependencies. +Dependency dep/pkg9 (via replace of root/replaced-pkg8) is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies (-W) to include root dependencies. +Updating dependencies +Lock file operations: 0 installs, 2 updates, 0 removals + - Upgrading dep/pkg2 (1.0.0 => 2.0.0) + - Upgrading update/pkg1 (1.0.0 => 2.0.0) +Writing lock file +Installing dependencies from lock file (including require-dev) +Package operations: 8 installs, 0 updates, 0 removals +Generating autoload files + +--EXPECT-- +Installing dep/pkg9 (1.0.0) +Installing root/pkg10 (1.0.0) +Installing dep/pkg6 (1.0.0) +Installing dep/pkg5 (1.0.0) +Installing root/pkg7 (1.0.0) +Installing root/pkg3 (1.0.0) +Installing dep/pkg2 (2.0.0) +Installing update/pkg1 (2.0.0) diff --git a/tests/deprecations-8.1.json b/tests/deprecations-8.1.json index f920fe827..87f06bf69 100644 --- a/tests/deprecations-8.1.json +++ b/tests/deprecations-8.1.json @@ -77,12 +77,12 @@ { "location": "Composer\\Test\\InstallerTest::testIntegrationWithRawPool", "message": "preg_match(): Passing null to parameter #4 ($flags) of type int is deprecated", - "count": 1728 + "count": 1776 }, { "location": "Composer\\Test\\InstallerTest::testIntegrationWithPoolOptimizer", "message": "preg_match(): Passing null to parameter #4 ($flags) of type int is deprecated", - "count": 1728 + "count": 1776 }, { "location": "Composer\\Test\\Package\\Archiver\\ArchivableFilesFinderTest::testManualExcludes", From bbc442b0ade931cc1fcf4f7ee99a8368932f18af Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Tue, 23 Nov 2021 16:02:34 +0100 Subject: [PATCH 3/3] Undo changes to providers, only unlock replacers when unlocking a given name --- .../DependencyResolver/PoolBuilder.php | 56 ++++++++----------- ...tial-update-with-dependencies-provide.test | 14 ++--- .../partial-update-with-deps-warns-root.test | 2 - 3 files changed, 31 insertions(+), 41 deletions(-) diff --git a/src/Composer/DependencyResolver/PoolBuilder.php b/src/Composer/DependencyResolver/PoolBuilder.php index cc211b9e7..f05a62fc4 100644 --- a/src/Composer/DependencyResolver/PoolBuilder.php +++ b/src/Composer/DependencyResolver/PoolBuilder.php @@ -156,10 +156,10 @@ class PoolBuilder foreach ($request->getLockedRepository()->getPackages() as $lockedPackage) { if (!$this->isUpdateAllowed($lockedPackage)) { $request->lockPackage($lockedPackage); - $lockedName = $lockedPackage->getName(); + $this->skippedLoad[$lockedPackage->getName()][] = $lockedPackage; // remember which packages we skipped loading remote content for in this partial update - foreach ($lockedPackage->getNames() as $name) { - $this->skippedLoad[$name][] = $lockedPackage; + foreach ($lockedPackage->getReplaces() as $link) { + $this->skippedLoad[$link->getTarget()][] = $lockedPackage; } } } @@ -493,33 +493,25 @@ class PoolBuilder if (isset($rootRequires[$name])) { return array_map(function (PackageInterface $package) use ($name) { if ($name !== $package->getName()) { - foreach ($package->getReplaces() as $link) { - if ($link->getTarget() === $name) { - return $package->getName() .' (via replace of '.$name.')'; - } - } - return $package->getName() .' (via provide of '.$name.')'; + return $package->getName() .' (via replace of '.$name.')'; } return $package->getName(); }, $this->skippedLoad[$name]); } - foreach ($this->skippedLoad[$name] as $providedBy) { - foreach ($providedBy->getNames() as $providedName) { - if (isset($rootRequires[$providedName])) { - if ($name !== $providedBy->getName()) { - foreach ($providedBy->getReplaces() as $link) { - if ($link->getTarget() === $name) { - $matches[] = $providedBy->getName() .' (via replace of '.$name.')'; - continue 2; - } - } - $matches[] = $providedBy->getName() .' (via provide of '.$name.')'; - continue; + foreach ($this->skippedLoad[$name] as $packageOrReplacer) { + if (isset($rootRequires[$packageOrReplacer->getName()])) { + $matches[] = $packageOrReplacer->getName(); + } + foreach ($packageOrReplacer->getReplaces() as $link) { + if (isset($rootRequires[$link->getTarget()])) { + if ($name !== $packageOrReplacer->getName()) { + $matches[] = $packageOrReplacer->getName() .' (via replace of '.$name.')'; + } else { + $matches[] = $packageOrReplacer->getName(); } - - $matches[] = $providedBy->getName(); + break; } } } @@ -590,21 +582,21 @@ class PoolBuilder */ private function unlockPackage(Request $request, $name) { - foreach ($this->skippedLoad[$name] as $providedBy) { - $providedBy = $providedBy->getName(); + foreach ($this->skippedLoad[$name] as $packageOrReplacer) { // if we unfixed a replaced package name, we also need to unfix the replacer itself // as long as it was not unfixed yet - if ($providedBy !== $name && isset($this->skippedLoad[$providedBy])) { - if ($request->getUpdateAllowTransitiveRootDependencies() || (!$this->isRootRequire($request, $name) && !$this->isRootRequire($request, $providedBy))) { - $this->unlockPackage($request, $providedBy); + if ($packageOrReplacer->getName() !== $name && isset($this->skippedLoad[$packageOrReplacer->getName()])) { + $replacerName = $packageOrReplacer->getName(); + if ($request->getUpdateAllowTransitiveRootDependencies() || (!$this->isRootRequire($request, $name) && !$this->isRootRequire($request, $replacerName))) { + $this->unlockPackage($request, $replacerName); - if ($this->isRootRequire($request, $providedBy)) { - $this->markPackageNameForLoading($request, $providedBy, new MatchAllConstraint); + if ($this->isRootRequire($request, $replacerName)) { + $this->markPackageNameForLoading($request, $replacerName, new MatchAllConstraint); } else { foreach ($this->packages as $loadedPackage) { $requires = $loadedPackage->getRequires(); - if (isset($requires[$providedBy])) { - $this->markPackageNameForLoading($request, $providedBy, $requires[$providedBy]->getConstraint()); + if (isset($requires[$replacerName])) { + $this->markPackageNameForLoading($request, $replacerName, $requires[$replacerName]->getConstraint()); } } } diff --git a/tests/Composer/Test/Fixtures/installer/partial-update-with-dependencies-provide.test b/tests/Composer/Test/Fixtures/installer/partial-update-with-dependencies-provide.test index 8d45e9cd9..c332ca045 100644 --- a/tests/Composer/Test/Fixtures/installer/partial-update-with-dependencies-provide.test +++ b/tests/Composer/Test/Fixtures/installer/partial-update-with-dependencies-provide.test @@ -1,5 +1,5 @@ --TEST-- -Ensure a partial update of a dependency updates dependencies which provide its requirements. +Ensure a partial update of a dependency does NOT update dependencies which provide its requirements. --COMPOSER-- { @@ -44,9 +44,9 @@ update root/req1 --with-dependencies --EXPECT-LOCK-- { "packages": [ - {"name": "dep/pkg1", "version": "1.2.0", "type": "library", "provide": {"virtual/pkg1": "2.0.0"}}, - {"name": "dep/pkg2", "version": "1.2.0", "type": "library", "provide": {"virtual/pkg1": "2.0.0"}}, - {"name": "root/req1", "version": "2.0.0", "type": "library", "require": {"virtual/pkg1": "2.*"}}, + {"name": "dep/pkg1", "version": "1.0.0", "type": "library", "provide": {"virtual/pkg1": "1.0.0"}}, + {"name": "dep/pkg2", "version": "1.0.0", "type": "library", "provide": {"virtual/pkg1": "1.0.0"}}, + {"name": "root/req1", "version": "1.0.0", "type": "library", "require": {"virtual/pkg1": "1.*"}}, {"name": "root/req2", "version": "1.0.0", "type": "library", "require": {"dep/pkg1": "1.*", "dep/pkg2": "1.*"}} ], "packages-dev": [], @@ -59,7 +59,7 @@ update root/req1 --with-dependencies "platform-dev": [] } --EXPECT-- -Installing dep/pkg1 (1.2.0) -Installing dep/pkg2 (1.2.0) -Installing root/req1 (2.0.0) +Installing dep/pkg1 (1.0.0) +Installing dep/pkg2 (1.0.0) +Installing root/req1 (1.0.0) Installing root/req2 (1.0.0) diff --git a/tests/Composer/Test/Fixtures/installer/partial-update-with-deps-warns-root.test b/tests/Composer/Test/Fixtures/installer/partial-update-with-deps-warns-root.test index b1aa96b84..d8d49a246 100644 --- a/tests/Composer/Test/Fixtures/installer/partial-update-with-deps-warns-root.test +++ b/tests/Composer/Test/Fixtures/installer/partial-update-with-deps-warns-root.test @@ -110,8 +110,6 @@ update update/pkg1 --with-dependencies --EXPECT-OUTPUT-- Loading composer repositories with package information Dependency root/pkg3 is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies (-W) to include root dependencies. -Dependency dep/pkg5 (via provide of root/provided-pkg4) is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies (-W) to include root dependencies. -Dependency dep/pkg6 (via provide of root/provided-pkg4) is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies (-W) to include root dependencies. Dependency dep/pkg9 (via replace of root/replaced-pkg8) is also a root requirement. Package has not been listed as an update argument, so keeping locked at old version. Use --with-all-dependencies (-W) to include root dependencies. Updating dependencies Lock file operations: 0 installs, 2 updates, 0 removals