Merge pull request #8811 from Seldaek/fix-transaction-order

Fix transaction order

src/Composer/DependencyResolver/Transaction.php

@@ -295,7 +295,7 @@ class Transaction
     {
         $uninstOps = array();
         foreach ($operations as $idx => $op) {
-            if ($op instanceof Operation\UninstallOperation) {
+            if ($op instanceof Operation\UninstallOperation || $op instanceof Operation\MarkAliasUninstalledOperation) {
                 $uninstOps[] = $op;
                 unset($operations[$idx]);
             }

tests/Composer/Test/DependencyResolver/TransactionTest.php

@@ -53,6 +53,7 @@ class TransactionTest extends TestCase
         $expectedOperations = array(
             array('job' => 'uninstall', 'package' => $packageC),
             array('job' => 'uninstall', 'package' => $packageE),
+            array('job' => 'markAliasUninstalled', 'package' => $packageEalias),
             array('job' => 'install', 'package' => $packageA0first),
             array('job' => 'update', 'from' => $packageB, 'to' => $packageBnew),
             array('job' => 'install', 'package' => $packageG),
@@ -60,7 +61,6 @@ class TransactionTest extends TestCase
             array('job' => 'markAliasInstalled', 'package' => $packageFalias1),
             array('job' => 'markAliasInstalled', 'package' => $packageFalias2),
             array('job' => 'install', 'package' => $packageD),
-            array('job' => 'markAliasUninstalled', 'package' => $packageEalias),
         );
 
         $transaction = new Transaction($presentPackages, $resultPackages);

tests/Composer/Test/Fixtures/installer/partial-update-installs-from-lock-even-missing.test

@@ -97,9 +97,9 @@ update b/b
     "platform-dev": []
 }
 --EXPECT--
+Marking a/a (2.1.x-dev oldmaster-a) as uninstalled, alias of a/a (dev-master oldmaster-a)
+Marking b/b (2.1.x-dev oldmaster-b) as uninstalled, alias of b/b (dev-master oldmaster-b)
 Upgrading a/a (dev-master oldmaster-a => dev-master newmaster-a)
 Marking a/a (2.2.x-dev newmaster-a) as installed, alias of a/a (dev-master newmaster-a)
 Upgrading b/b (dev-master oldmaster-b => dev-master newmaster-b2)
 Marking b/b (2.3.x-dev newmaster-b2) as installed, alias of b/b (dev-master newmaster-b2)
-Marking a/a (2.1.x-dev oldmaster-a) as uninstalled, alias of a/a (dev-master oldmaster-a)
-Marking b/b (2.1.x-dev oldmaster-b) as uninstalled, alias of b/b (dev-master oldmaster-b)

tests/Composer/Test/Fixtures/installer/update-alias-lock2.test

@@ -0,0 +1,84 @@
+--TEST--
+Updating an aliased package where the old alias matches the new package should not fail
+--COMPOSER--
+{
+    "repositories": [
+        {
+            "type": "package",
+            "package": [
+                {
+                    "name": "a/a", "version": "1.10.x-dev",
+                    "extra": { "branch-alias": { "dev-master": "1.10.x-dev" } },
+                    "source": { "type": "git", "url": "", "reference": "downgradedref" }
+                },
+                {
+                    "name": "a/a", "version": "dev-master",
+                    "extra": { "branch-alias": { "dev-master": "2.x-dev" } },
+                    "source": { "type": "git", "url": "", "reference": "newref" }
+                }
+            ]
+        }
+    ],
+    "require": {
+        "a/a": "^1.0"
+    },
+    "minimum-stability": "dev"
+}
+--LOCK--
+{
+    "_": "outdated lock file, should not have to be loaded in an update",
+    "packages": [
+        {
+            "name": "a/a", "version": "dev-master",
+            "extra": { "branch-alias": { "dev-master": "1.10.x-dev" } },
+            "source": { "type": "git", "url": "", "reference": "installedref" }
+        }
+    ],
+    "packages-dev": [],
+    "aliases": [],
+    "minimum-stability": "dev",
+    "stability-flags": [],
+    "prefer-stable": false,
+    "prefer-lowest": false
+}
+--INSTALLED--
+[
+    {
+        "name": "a/a", "version": "dev-master",
+        "extra": { "branch-alias": { "dev-master": "1.10.x-dev" } },
+        "source": { "type": "git", "url": "", "reference": "installedref" }
+    }
+]
+--RUN--
+update
+--EXPECT-LOCK--
+{
+    "packages": [
+        {
+            "name": "a/a", "version": "1.10.x-dev",
+            "extra": { "branch-alias": { "dev-master": "1.10.x-dev" } },
+            "source": { "type": "git", "url": "", "reference": "downgradedref" },
+            "type": "library"
+        }
+    ],
+    "packages-dev": [],
+    "aliases": [],
+    "minimum-stability": "dev",
+    "stability-flags": [],
+    "prefer-stable": false,
+    "prefer-lowest": false,
+    "platform": [],
+    "platform-dev": []
+}
+--EXPECT-INSTALLED--
+[
+    {
+        "name": "a/a", "version": "1.10.x-dev",
+        "source": { "type": "git", "url": "", "reference": "downgradedref" },
+        "type": "library",
+        "extra": { "branch-alias": { "dev-master": "1.10.x-dev" } }
+    }
+]
+--EXPECT--
+Marking a/a (1.10.x-dev installedref) as uninstalled, alias of a/a (dev-master installedref)
+Downgrading a/a (dev-master installedref => 1.10.x-dev downgradedref)

tests/Composer/Test/Fixtures/installer/update-alias.test

@@ -33,5 +33,5 @@ Update aliased package to non-aliased version
 --RUN--
 update
 --EXPECT--
-Upgrading a/a (dev-master master => dev-foo foo)
 Marking a/a (1.0.x-dev master) as uninstalled, alias of a/a (dev-master master)
+Upgrading a/a (dev-master master => dev-foo foo)

tests/Composer/Test/Fixtures/installer/update-allow-list-with-dependencies-alias.test

@@ -91,9 +91,9 @@ update new/pkg --with-all-dependencies
     "platform-dev": []
 }
 --EXPECT--
+Marking current/dep2 (1.0.x-dev) as uninstalled, alias of current/dep2 (dev-foo)
 Marking current/dep (1.1.0) as installed, alias of current/dep (dev-master)
 Upgrading current/dep2 (dev-foo => dev-master)
 Marking current/dep2 (1.1.2) as installed, alias of current/dep2 (dev-master)
 Marking current/dep2 (2.x-dev) as installed, alias of current/dep2 (dev-master)
 Installing new/pkg (1.0.0)
-Marking current/dep2 (1.0.x-dev) as uninstalled, alias of current/dep2 (dev-foo)

tests/Composer/Test/Fixtures/installer/update-downgrades-unstable-packages.test

@@ -46,5 +46,5 @@ Downgrading from unstable to more stable package should work even if already ins
 --RUN--
 update
 --EXPECT--
-Downgrading a/a (dev-master abcd => 1.0.0)
 Marking a/a (9999999-dev abcd) as uninstalled, alias of a/a (dev-master abcd)
+Downgrading a/a (dev-master abcd => 1.0.0)

tests/Composer/Test/Fixtures/installer/update-no-dev-still-resolves-dev.test

@@ -61,8 +61,8 @@ Updates with --no-dev but we still end up with a complete lock file including de
 update --no-dev
 --EXPECT--
 Removing a/b (1.0.0)
+Marking dev/pkg (1.0.x-dev old) as uninstalled, alias of dev/pkg (dev-master old)
 Upgrading a/a (1.0.0 => 1.0.1)
 Installing a/c (1.0.0)
 Upgrading dev/pkg (dev-master old => dev-master new)
 Marking dev/pkg (1.1.x-dev new) as installed, alias of dev/pkg (dev-master new)
-Marking dev/pkg (1.0.x-dev old) as uninstalled, alias of dev/pkg (dev-master old)

tests/Composer/Test/Fixtures/installer/updating-dev-from-lock-removes-old-deps.test

@@ -42,5 +42,5 @@ Installing locked dev packages should remove old dependencies
 install
 --EXPECT--
 Removing a/dependency (dev-master ref)
-Upgrading a/devpackage (dev-master oldref => dev-master newref)
 Marking a/dependency (9999999-dev ref) as uninstalled, alias of a/dependency (dev-master ref)
+Upgrading a/devpackage (dev-master oldref => dev-master newref)

tests/Composer/Test/InstallerTest.php

@@ -184,7 +184,7 @@ class InstallerTest extends TestCase
     /**
      * @dataProvider getIntegrationTests
      */
-    public function testIntegration($file, $message, $condition, $composerConfig, $lock, $installed, $run, $expectLock, $expectOutput, $expect, $expectResult)
+    public function testIntegration($file, $message, $condition, $composerConfig, $lock, $installed, $run, $expectLock, $expectInstalled, $expectOutput, $expect, $expectResult)
     {
         if ($condition) {
             eval('$res = '.$condition.';');
@@ -327,6 +327,23 @@ class InstallerTest extends TestCase
             $this->assertEquals($expectLock, $actualLock);
         }
 
+        if ($expectInstalled !== null) {
+            $actualInstalled = array();
+            $dumper = new ArrayDumper();
+
+            foreach ($repositoryManager->getLocalRepository()->getCanonicalPackages() as $package) {
+                $package = $dumper->dump($package);
+                unset($package['version_normalized']);
+                $actualInstalled[] = $package;
+            }
+
+            usort($actualInstalled, function ($a, $b) {
+                return strcmp($a['name'], $b['name']);
+            });
+
+            $this->assertSame($expectInstalled, $actualInstalled);
+        }
+
         $installationManager = $composer->getInstallationManager();
         $this->assertSame(rtrim($expect), implode("\n", $installationManager->getTrace()));
 
@@ -355,6 +372,7 @@ class InstallerTest extends TestCase
                 $installedDev = array();
                 $lock = array();
                 $expectLock = array();
+                $expectInstalled = null;
                 $expectResult = 0;
 
                 $message = $testData['TEST'];
@@ -393,6 +411,9 @@ class InstallerTest extends TestCase
                         $expectLock = JsonFile::parseJson($testData['EXPECT-LOCK']);
                     }
                 }
+                if (!empty($testData['EXPECT-INSTALLED'])) {
+                    $expectInstalled = JsonFile::parseJson($testData['EXPECT-INSTALLED']);
+                }
                 $expectOutput = isset($testData['EXPECT-OUTPUT']) ? $testData['EXPECT-OUTPUT'] : null;
                 $expect = $testData['EXPECT'];
                 if (!empty($testData['EXPECT-EXCEPTION'])) {
@@ -409,7 +430,7 @@ class InstallerTest extends TestCase
                 die(sprintf('Test "%s" is not valid: '.$e->getMessage(), str_replace($fixturesDir.'/', '', $file)));
             }
 
-            $tests[basename($file)] = array(str_replace($fixturesDir.'/', '', $file), $message, $condition, $composer, $lock, $installed, $run, $expectLock, $expectOutput, $expect, $expectResult);
+            $tests[basename($file)] = array(str_replace($fixturesDir.'/', '', $file), $message, $condition, $composer, $lock, $installed, $run, $expectLock, $expectInstalled, $expectOutput, $expect, $expectResult);
         }
 
         return $tests;
@@ -427,6 +448,7 @@ class InstallerTest extends TestCase
             'INSTALLED' => false,
             'RUN' => true,
             'EXPECT-LOCK' => false,
+            'EXPECT-INSTALLED' => false,
             'EXPECT-OUTPUT' => false,
             'EXPECT-EXIT-CODE' => false,
             'EXPECT-EXCEPTION' => false,

tests/Composer/Test/Mock/InstallationManagerMock.php

@@ -66,7 +66,9 @@ class InstallationManagerMock extends InstallationManager
         $this->updated[] = array($operation->getInitialPackage(), $operation->getTargetPackage());
         $this->trace[] = strip_tags((string) $operation);
         $repo->removePackage($operation->getInitialPackage());
-        $repo->addPackage(clone $operation->getTargetPackage());
+        if (!$repo->hasPackage($operation->getTargetPackage())) {
+            $repo->addPackage(clone $operation->getTargetPackage());
+        }
     }
 
     public function uninstall(RepositoryInterface $repo, UninstallOperation $operation)