From 7c891701e6ca13a79e0131f06c9eddfd9a315c9b Mon Sep 17 00:00:00 2001 From: Helmut Hummel Date: Tue, 31 Mar 2020 13:29:22 +0200 Subject: [PATCH] Fix package sorting PackageSorter weighs the importance of a package by counting how many times it is required by other packages. This works by calculating the weight for each package name. However currently the package index of the package array is currently passed the weigh function, which basically disables package sorting. The reason for that is, that a package repository previously returned the package list as associative array with package name as keys, but currently just as an array with integer keys. Therefore we must extract the package name from the package before passing it to the weigh function. --- src/Composer/Util/PackageSorter.php | 10 +- .../Composer/Test/Util/PackageSorterTest.php | 129 ++++++++++++++++++ 2 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 tests/Composer/Test/Util/PackageSorterTest.php diff --git a/src/Composer/Util/PackageSorter.php b/src/Composer/Util/PackageSorter.php index 8d8c9a06c..204a35b51 100644 --- a/src/Composer/Util/PackageSorter.php +++ b/src/Composer/Util/PackageSorter.php @@ -55,9 +55,9 @@ class PackageSorter $weightList = array(); - foreach ($packages as $name => $package) { - $weight = $computeImportance($name); - $weightList[$name] = $weight; + foreach ($packages as $index => $package) { + $weight = $computeImportance($package->getName()); + $weightList[$index] = $weight; } $stable_sort = function (&$array) { @@ -84,8 +84,8 @@ class PackageSorter $sortedPackages = array(); - foreach (array_keys($weightList) as $name) { - $sortedPackages[] = $packages[$name]; + foreach (array_keys($weightList) as $index) { + $sortedPackages[] = $packages[$index]; } return $sortedPackages; } diff --git a/tests/Composer/Test/Util/PackageSorterTest.php b/tests/Composer/Test/Util/PackageSorterTest.php new file mode 100644 index 000000000..e93503436 --- /dev/null +++ b/tests/Composer/Test/Util/PackageSorterTest.php @@ -0,0 +1,129 @@ + + * Jordi Boggiano + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Composer\Test\Util; + +use Composer\Package\Link; +use Composer\Package\Package; +use Composer\Test\TestCase; +use Composer\Util\PackageSorter; + +class PackageSorterTest extends TestCase +{ + public function testSortingDoesNothingWithNoDependencies() + { + $packages[] = $this->createPackage('foo/bar1', array()); + $packages[] = $this->createPackage('foo/bar2', array()); + $packages[] = $this->createPackage('foo/bar3', array()); + $packages[] = $this->createPackage('foo/bar4', array()); + + $sortedPackages = PackageSorter::sortPackages($packages); + + self::assertSame($packages, $sortedPackages); + } + + public function sortingOrdersDependenciesHigherThanPackageDataProvider() + { + return array( + 'one package is dep' => array( + array( + $this->createPackage('foo/bar1', array('foo/bar4')), + $this->createPackage('foo/bar2', array('foo/bar4')), + $this->createPackage('foo/bar3', array('foo/bar4')), + $this->createPackage('foo/bar4', array()), + ), + array( + 'foo/bar4', + 'foo/bar1', + 'foo/bar2', + 'foo/bar3', + ), + ), + 'one package has more deps' => array( + array( + $this->createPackage('foo/bar1', array('foo/bar2')), + $this->createPackage('foo/bar2', array('foo/bar4')), + $this->createPackage('foo/bar3', array('foo/bar4')), + $this->createPackage('foo/bar4', array()), + ), + array( + 'foo/bar4', + 'foo/bar2', + 'foo/bar1', + 'foo/bar3', + ), + ), + 'package is required by many, but requires one other' => array( + array( + $this->createPackage('foo/bar1', array('foo/bar3')), + $this->createPackage('foo/bar2', array('foo/bar3')), + $this->createPackage('foo/bar3', array('foo/bar4')), + $this->createPackage('foo/bar4', array()), + $this->createPackage('foo/bar5', array('foo/bar3')), + $this->createPackage('foo/bar6', array('foo/bar3')), + ), + array( + 'foo/bar4', + 'foo/bar3', + 'foo/bar1', + 'foo/bar2', + 'foo/bar5', + 'foo/bar6', + ), + ), + 'one package has many requires' => array( + array( + $this->createPackage('foo/bar1', array('foo/bar2')), + $this->createPackage('foo/bar2', array()), + $this->createPackage('foo/bar3', array('foo/bar4')), + $this->createPackage('foo/bar4', array()), + $this->createPackage('foo/bar5', array('foo/bar2')), + $this->createPackage('foo/bar6', array('foo/bar2')), + ), + array( + 'foo/bar2', + 'foo/bar4', + 'foo/bar1', + 'foo/bar3', + 'foo/bar5', + 'foo/bar6', + ), + ), + ); + } + + /** + * @dataProvider sortingOrdersDependenciesHigherThanPackageDataProvider + * @param array $packages + * @param array $expectedOrderedList + */ + public function testSortingOrdersDependenciesHigherThanPackage($packages, $expectedOrderedList) + { + $sortedPackages = PackageSorter::sortPackages($packages); + $sortedPackageNames = array_map(function ($package) { return $package->getName(); }, $sortedPackages); + + self::assertSame($expectedOrderedList, $sortedPackageNames); + } + + private function createPackage($name, $requires) + { + $package = new Package($name, '1.0.0.0', '1.0.0'); + + $links = array(); + foreach ($requires as $requireName) { + $links[] = new Link($package->getName(), $requireName); + } + $package->setRequires($links); + + return $package; + } +}