diff --git a/src/Composer/DependencyResolver/LockTransaction.php b/src/Composer/DependencyResolver/LockTransaction.php index 293f4a43b..9115e1533 100644 --- a/src/Composer/DependencyResolver/LockTransaction.php +++ b/src/Composer/DependencyResolver/LockTransaction.php @@ -67,6 +67,7 @@ class LockTransaction protected function calculateOperations() { $operations = array(); + $ignoreRemove = array(); $lockMeansUpdateMap = $this->findPotentialUpdates(); foreach ($this->decisions as $i => $decision) { @@ -77,17 +78,17 @@ class LockTransaction // wanted & !present if ($literal > 0 && !isset($this->presentMap[spl_object_hash($package)])) { - if (isset($lockMeansUpdateMap[abs($literal)]) && !$package instanceof AliasPackage) { + if (isset($lockMeansUpdateMap[spl_object_hash($package)]) && !$package instanceof AliasPackage) { // TODO we end up here sometimes because we prefer the remote package now to get up to date metadata // TODO define some level of identity here for what constitutes an update and what can be ignored? new kind of metadata only update? - $target = $lockMeansUpdateMap[abs($literal)]; + $target = $lockMeansUpdateMap[spl_object_hash($package)]; if ($package->getName() !== $target->getName() || $package->getVersion() !== $target->getVersion()) { $operations[] = new Operation\UpdateOperation($target, $package, $reason); } // avoid updates to one package from multiple origins - $ignoreRemove[$lockMeansUpdateMap[abs($literal)]->id] = true; - unset($lockMeansUpdateMap[abs($literal)]); + $ignoreRemove[spl_object_hash($lockMeansUpdateMap[spl_object_hash($package)])] = true; + unset($lockMeansUpdateMap[spl_object_hash($package)]); } else { if ($package instanceof AliasPackage) { $operations[] = new Operation\MarkAliasInstalledOperation($package, $reason); @@ -103,7 +104,7 @@ class LockTransaction $reason = $decision[Decisions::DECISION_REASON]; $package = $this->pool->literalToPackage($literal); - if ($literal <= 0 && isset($this->presentMap[spl_object_hash($package)]) && !isset($ignoreRemove[$package->id])) { + if ($literal <= 0 && isset($this->presentMap[spl_object_hash($package)]) && !isset($ignoreRemove[spl_object_hash($package)])) { if ($package instanceof AliasPackage) { $operations[] = new Operation\MarkAliasUninstalledOperation($package, $reason); } else { @@ -112,6 +113,17 @@ class LockTransaction } } + foreach ($this->presentMap as $package) { + if ($package->id === -1 && !isset($ignoreRemove[spl_object_hash($package)])) { + // TODO pass reason parameter to these two operations? + if ($package instanceof AliasPackage) { + $operations[] = new Operation\MarkAliasUninstalledOperation($package); + } else { + $operations[] = new Operation\UninstallOperation($package); + } + } + } + $this->setResultPackages(); return $operations; @@ -207,15 +219,11 @@ class LockTransaction // TODO can't we just look at existing rules? $updates = $this->policy->findUpdatePackages($this->pool, $package); - $literals = array($package->id); - - foreach ($updates as $update) { - $literals[] = $update->id; - } + $updatesAndPackage = array_merge(array($package), $updates); - foreach ($literals as $updateLiteral) { - if (!isset($lockMeansUpdateMap[$updateLiteral])) { - $lockMeansUpdateMap[$updateLiteral] = $package; + foreach ($updatesAndPackage as $update) { + if (!isset($lockMeansUpdateMap[spl_object_hash($update)])) { + $lockMeansUpdateMap[spl_object_hash($update)] = $package; } } } diff --git a/tests/Composer/Test/Fixtures/installer/update-removes-unused-locked-dep.test b/tests/Composer/Test/Fixtures/installer/update-removes-unused-locked-dep.test new file mode 100644 index 000000000..808afb02e --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/update-removes-unused-locked-dep.test @@ -0,0 +1,67 @@ +--TEST-- +A composer update should remove unused locked dependencies from the lock file and remove unused installed deps from disk +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "a/a", "version": "1.0.0" }, + { "name": "b/b", "version": "1.0.0" } + ] + } + ], + "require": { + "a/a": "*" + } +} +--LOCK-- +{ + "packages": [ + { "name": "a/a", "version": "1.0.0" }, + { "name": "b/b", "version": "1.0.0" } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": [], + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} +--INSTALLED-- +[ + { "name": "a/a", "version": "1.0.0" }, + { "name": "b/b", "version": "1.0.0" }, + { "name": "c/c", "version": "1.0.0" } +] +--RUN-- +update +--EXPECT-LOCK-- +{ + "packages": [ + { "name": "a/a", "version": "1.0.0", "type": "library" } + ], + "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 +Updating dependencies +Lock file operations: 0 installs, 0 updates, 1 removal + - Uninstalling b/b (1.0.0) +Writing lock file +Installing dependencies from lock file (including require-dev) +Package operations: 0 installs, 0 updates, 2 removals +Generating autoload files + +--EXPECT-- +Uninstalling c/c (1.0.0) +Uninstalling b/b (1.0.0)