Merge pull request #10617 from Seldaek/pkg_sort

More deterministic package sorting
composer · Mar 15, 2022 · 4cc4a28 · 4cc4a28
2 parents 58deb39 + dbcdb4a
commit 4cc4a28
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 28 deletions.
diff --git a/src/Composer/Autoload/AutoloadGenerator.php b/src/Composer/Autoload/AutoloadGenerator.php
@@ -1379,7 +1379,7 @@ function ($item) use ($include) {
     /**
      * Sorts packages by dependency weight
      *
-     * Packages of equal weight retain the original order
+     * Packages of equal weight are sorted alphabetically
      *
      * @param array<int, array{0: PackageInterface, 1: string}> $packageMap
      * @return array<int, array{0: PackageInterface, 1: string}>

diff --git a/src/Composer/Util/PackageSorter.php b/src/Composer/Util/PackageSorter.php
@@ -13,13 +13,14 @@
 namespace Composer\Util;
 
 use Composer\Package\PackageInterface;
+use Composer\Package\RootPackageInterface;
 
 class PackageSorter
 {
     /**
      * Sorts packages by dependency weight
      *
-     * Packages of equal weight retain the original order
+     * Packages of equal weight are sorted alphabetically
      *
      * @param  PackageInterface[] $packages
      * @return PackageInterface[] sorted array
@@ -29,7 +30,11 @@ public static function sortPackages(array $packages)
         $usageList = array();
 
         foreach ($packages as $package) {
-            foreach (array_merge($package->getRequires(), $package->getDevRequires()) as $link) {
+            $links = $package->getRequires();
+            if ($package instanceof RootPackageInterface) {
+                $links = array_merge($links, $package->getDevRequires());
+            }
+            foreach ($links as $link) {
                 $target = $link->getTarget();
                 $usageList[$target][] = $package->getName();
             }
@@ -62,39 +67,26 @@ public static function sortPackages(array $packages)
             return $weight;
         };
 
-        $weightList = array();
+        $weightedPackages = array();
 
         foreach ($packages as $index => $package) {
-            $weight = $computeImportance($package->getName());
-            $weightList[$index] = $weight;
+            $name = $package->getName();
+            $weight = $computeImportance($name);
+            $weightedPackages[] = array('name' => $name, 'weight' => $weight, 'index' => $index);
         }
 
-        $stable_sort = function (&$array) {
-            static $transform, $restore;
-
-            $i = 0;
-
-            if (!$transform) {
-                $transform = function (&$v, $k) use (&$i) {
-                    $v = array($v, ++$i, $k, $v);
-                };
-
-                $restore = function (&$v) {
-                    $v = $v[3];
-                };
+        usort($weightedPackages, function ($a, $b) {
+            if ($a['weight'] !== $b['weight']) {
+                return $a['weight'] - $b['weight'];
             }
 
-            array_walk($array, $transform);
-            asort($array);
-            array_walk($array, $restore);
-        };
-
-        $stable_sort($weightList);
+            return strnatcasecmp($a['name'], $b['name']);
+        });
 
         $sortedPackages = array();
 
-        foreach (array_keys($weightList) as $index) {
-            $sortedPackages[] = $packages[$index];
+        foreach ($weightedPackages as $pkg) {
+            $sortedPackages[] = $packages[$pkg['index']];
         }
 
         return $sortedPackages;

diff --git a/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php b/tests/Composer/Test/Autoload/AutoloadGeneratorTest.php
@@ -1017,6 +1017,14 @@ public function testFilesAutoloadOrderByDependencies()
         $packages[] = $c = new Package('c/lorem', '1.0', '1.0');
         $packages[] = $e = new Package('e/e', '1.0', '1.0');
 
+        // expected order:
+        // c requires nothing
+        // d requires c
+        // b requires c & d
+        // e requires c
+        // z requires c
+        // (b, e, z ordered alphabetically)
+
         $z->setAutoload(array('files' => array('testA.php')));
         $z->setRequires(array('c/lorem' => new Link('z/foo', 'c/lorem', new MatchAllConstraint())));
 

diff --git a/tests/Composer/Test/Autoload/Fixtures/autoload_static_files_by_dependency.php b/tests/Composer/Test/Autoload/Fixtures/autoload_static_files_by_dependency.php
@@ -9,9 +9,9 @@ class ComposerStaticInitFilesAutoloadOrder
     public static $files = array (
         'bfdd693009729d60c830ff8d79129635' => __DIR__ . '/..' . '/c/lorem/testC.php',
         '61e6098c8cafe404d6cf19e59fc2b788' => __DIR__ . '/..' . '/d/d/testD.php',
-        '8bceec6fdc149a1a6acbf7ad3cfed51c' => __DIR__ . '/..' . '/z/foo/testA.php',
         'c5466e580c6c2403f225c43b6a21a96f' => __DIR__ . '/..' . '/b/bar/testB.php',
         '69dfc37c40a853a7cbac6c9d2367c5f4' => __DIR__ . '/..' . '/e/e/testE.php',
+        '8bceec6fdc149a1a6acbf7ad3cfed51c' => __DIR__ . '/..' . '/z/foo/testA.php',
         'ab280164f4754f5dfdb0721de84d737f' => __DIR__ . '/../..' . '/root2.php',
     );
 

diff --git a/tests/Composer/Test/Util/PackageSorterTest.php b/tests/Composer/Test/Util/PackageSorterTest.php
@@ -99,6 +99,34 @@ public function sortingOrdersDependenciesHigherThanPackageDataProvider()
                     'foo/bar6',
                 ),
             ),
+            'circular deps sorted alphabetically if weighted equally' => array(
+                array(
+                    $this->createPackage('foo/bar1', array('circular/part1')),
+                    $this->createPackage('foo/bar2', array('circular/part2')),
+                    $this->createPackage('circular/part1', array('circular/part2')),
+                    $this->createPackage('circular/part2', array('circular/part1')),
+                ),
+                array(
+                    'circular/part1',
+                    'circular/part2',
+                    'foo/bar1',
+                    'foo/bar2',
+                ),
+            ),
+            'equal weight sorted alphabetically' => array(
+                array(
+                    $this->createPackage('foo/bar10', array('foo/dep')),
+                    $this->createPackage('foo/bar2', array('foo/dep')),
+                    $this->createPackage('foo/baz', array('foo/dep')),
+                    $this->createPackage('foo/dep', array()),
+                ),
+                array(
+                    'foo/dep',
+                    'foo/bar2',
+                    'foo/bar10',
+                    'foo/baz',
+                ),
+            ),
         );
     }