Browse Source

Fully switch to spl_object_hash in lock transaction

The pool builder tries to be minimal so it's fine for present/locked
packages not be assigned a solver/pool id. Adding a test to verify
correct creation of uninstall jobs
Nils Adermann 5 years ago
parent
commit
e308f043b9

+ 21 - 13
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;
                 }
             }
         }

+ 67 - 0
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)