AcademySoftwareFoundation / rez

An integrated package configuration, build and deployment system for software
https://rez.readthedocs.io
Apache License 2.0
932 stars 330 forks source link

Package Orderers now apply to variants #1684

Closed isohedronpipeline closed 5 months ago

isohedronpipeline commented 5 months ago

Fixes #369 Fixes #1683

This PR fixes this bug so that package orderers are applied to all packages and variants uniformly in all circumstances, for more expected results.

Carries on the work originally started by @pmolodo with #355 and #413.

Here is an example of how one might take advantage of package ordering applying to variants: Consider a package python which has versions ['2.6.4', '2.7.16', '3.7.9', '3.9.6', '3.10.8'] The default sorted package orderer would give us this sorted list: ['3.10.8', '3.9.6', '3.7.9', '2.7.16', '2.6.4'] And we end up choosing the first possible option in the resolve: 3.10.8

If we apply the following package order:

package_orderers = [
    {
       "type": "per_family",
       "orderers": [
            {
                "packages": ["python"],
                "type": "version_split",
                "first_version": "2.7.16"
            }
        ]
    }
]

Then we would get packages ordered like this: ['2.7.16', '2.6.4', '3.10.8', '3.9.6', '3.7.9'], where we start at the version_split first version. Which in turn would resolve to 2.7.16. So far so good. This is the current behavior.

Now consider a package pipeline which has variants:

[
    ["python-2.6.4"],
    ["python-2.7.16"],
    ["python-3.7.9"],
    ["python-3.9.6"],
]

If you do rez env pipeline we would get python-3.9.6 in the resolve, even if we have a package orderer specifically saying we want to prefer python 2.7.16. In order to get the correctly requested version of python, we must include it in the resolve: rez env pipeline python-2.7.16, which largely defeats the purpose of the package orderer, since the point of it is to inform the choices of packages for second order package requests.

@JeanChristopheMorinPerso and I have discussed the possible speed implications. There should be neglible differences, even for very large repositories.

Variants were already being sorted, but this change just changes them to be sorted according to a sort key, in a uniform way to first order requests, instead of simply by version. The sort key for more complicated package orderers are cached anyway, so it really should be very quick.

Since this will change the way packages are resolved, there is a small danger of people no longer getting the same resolve they used to. However, it is the belief of @JeanChristopheMorinPerso, @maxnbk, and I that anyone who is using package ordering would have expected this to be the correct result anyway, and the number of people using package orderers is small enough.

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 87.26415% with 27 lines in your changes are missing coverage. Please review.

Project coverage is 58.23%. Comparing base (c216f9d) to head (2993f66). Report is 3 commits behind head on main.

Files Patch % Lines
src/rez/package_order.py 86.84% 20 Missing and 5 partials :warning:
src/rez/resolved_context.py 50.00% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1684 +/- ## ========================================== + Coverage 58.05% 58.23% +0.18% ========================================== Files 126 126 Lines 17035 17143 +108 Branches 3490 3502 +12 ========================================== + Hits 9889 9983 +94 - Misses 6482 6494 +12 - Partials 664 666 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.