Improved package sorting #2644
Improved package sorting #2644
Conversation
| $computing = array(); | ||
| $computed = array(); | ||
| $compute_importance = function($name) use(&$compute_importance, &$computing, &$computed, $usageList) { | ||
| # reusing computed importance |
There was a problem hiding this comment.
please use // for comments, not #
|
Is it really needed to use a stable sort ? If 2 packages have the same weight, we don't care about the order in which their files are loaded, as it means that they don't depend on each other. So the unstable sort written in C should be enough for us. |
|
About stable sort: Although packages might not depend from one another, one might want them to be defined in a certain order. Imagine two unrelated packages patching the helper of a third package. Using stable sort, the developer is able to choose which should overwrite the helper in the end. But you're right, it might not be as important as it is with the current algorithm. |
|
I think the stable sort is quite important actually because packages on top in your composer.json should be reuqired first. It was already a problem in the past, so I guess this new algo will still work for that use case, while improving the dependency sort. |
|
Merging since it seems reasonable, let's hope it doesn't backfire. Thanks! |
|
Yes, let's hope :) I just noticed that the permissions of the files were changed from 100644 to 100755 by my commit. Sorry about that. Also, did you have time to review my PR for the issue #2615 ? |
|
Yeah I fixed the mode while merging no worries, regarding the other issue I'll try to review again but don't have so much time. |
Hi,
While I was trying to resolve an issue that caused files defined by a same package to be scattered across the autoload files array, I noticed that the package sorting algorithm needed to be improved because the position (or importance) of the packages wasn't computed recursively and the resulting positions were sorted using
asortwhich produced unpredictable results for packages with the same position, because as you know the sorting algorithm used by PHP is not stable (from the manual: If two members compare as equal, their relative order in the sorted array is undefined).This new algorithm has two purpose: compute positions (or weights) recursively, so that a package required by a package of weight -10 has at least a weight of -11; use a stable sorting algorithm so that the order of packages with the same weight is preserved.
The following is a comparison of the current algorithm with the proposal algorithm. I'm using the icybee/modules-nodes 2.x for testing purpose, because testing the package with the current Composer is causing a fatal error since the files of the icanboogie/http package are loaded after the files of the icanboogie/icanboogie package, although
icanboogie/icanboogierequiresicanboogie/http.With this proposal,
icanboogie/commonis rightfully at the top since it is used by many packages, although sometimes indirectly; andicanboogie/httpis well beforeicanboogie/icanboogiesince it is used byicanboogie/routingandicanboogie/icanboogieand by other packages as well although indirectly. Also notice that the order of packages with the same weight is preserved.Note: This pull request is similar to the PR #2642, but it features only the diff for the
sortPackageMap()mehtod.