AcademySoftwareFoundation / rez

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

Add a package orderer that matches Python's version specifier spec (PEP440) #1706

Open isohedronpipeline opened 6 months ago

isohedronpipeline commented 6 months ago

This PR adds a package order to sort packages according to pypa rules, with an option for risk tolerance of prerelease options, appropriate for how users consume packages from PyPI. By default, users only get release versions, but depending on their risk tolerance, they can opt into the latest RC, beta, or alpha releases as well.

PEP440PackageOrder

This adds support for sorting packages according to PyPA / Packaging Version Specifiers

Specifically, such that 1.1.0 > 1.1.0rc2 > 1.1.0b3 > 1.1.0a4. As a refresher, PyPA prerelease suffixes are work towards creating the main version. i.e. 1.1.0a1 is the first version of the branch that will eventually be 1.1.0. 1.1.0 and 1.1.0a1 will not be developed at the same time.

This package orderer allows the user to specify a prerelease risk tolerance, defaulting to sorting full release versions to the front. The package order may be configured to allow prerelease of a certain level ahead of fully released versions as well. For example, given the versions [1.0b2, 1.0a1, 1.0, 1.1, 1.0rc1, 1.1b2, 1.2b1], an orderer initialized with prerelease=None would give the order: [1.1, 1.0, 1.2b1, 1.1b2, 1.0rc1, 1.0b2, 1.0a1]. an orderer initialized with prerelease="b" would give the order: [1.2b1, 1.1, 1.1b2, 1.0, 1.0rc1, 1.0b2, 1.0a1].

codecov[bot] commented 6 months ago

Codecov Report

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

Project coverage is 58.33%. Comparing base (3c75a19) to head (0a139ce). Report is 2 commits behind head on main.

Files Patch % Lines
src/rez/package_order.py 93.93% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1706 +/- ## ========================================== + Coverage 58.25% 58.33% +0.08% ========================================== Files 126 126 Lines 17157 17189 +32 Branches 3504 3509 +5 ========================================== + Hits 9995 10028 +33 + Misses 6496 6495 -1 Partials 666 666 ```

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

isohedronpipeline commented 6 months ago

Thanks for creating this. I'm a little bit puzzled by the implementation of the PEP440 orderers and left some questions/comments. Also note how I refer to PEP440 and not PyPA. I'm not sure if we should call it PyPA or Python or PEP440.

Yeah, I thought about the name of it for a little while too. I settled on pypa because: 1) Calling it python or packaging would be pretty confusing considering that lots of (most?) rez packages are in python and a package has a more well understood meaning in the context of rez.
2) pep440 was another option, but since the official name for the version scheme is actually pypa, I thought it was more appropriate. Pep440 is the proposal number, but pypa is the actual rules that covern the scheme: https://www.pypa.io/en/latest/ 3) Even though pypa is not immediately recognizable for most people, neither is pep440 and in time "pypa" will be more recgonizable than the historical document that spawned it.

I didn't review any of the custom orderer stuff.

Could you split both in two different pull requests please? They don't seem related and one (the PEP440 one) will be much easier to get consensus on if we want that or not than the other (the custom orderer).

Sure, I'll split it out later this week.

I'm not sure if there is any specific issue or worry that you want to discuss about why it would be hard to get consensus on it or why we specifically would not want it. It doesn't do anything different from the existing version_split package orderer, it just solves the problem in a more general/flexible way.

They both serve pretty important functions and I'd love to see those features available to the rest of the community. I can split them up for the sake of isolating the conversation, but the custom sort order unit tests won't work until pypa is also merged in.

JeanChristopheMorinPerso commented 6 months ago

pep440 was another option, but since the official name for the version scheme is actually pypa, I thought it was more appropriate. Pep440 is the proposal number, but pypa is the actual rules that covern the scheme

I'll be pedantic, but PyPA is a group of people (to be a member, you need to be a maintainer of a project under the PyPA GitHub organization), not rules. They host pip, setuptools, flit, etc and they also host the packaging specs. They also don't "govern" it any specs. Any change to a spec needs a new PEP (unless the change is really really minor/cosmetic) and the packaging specs/PEPs fall under one (or two I don't remember correctly) PEP delegates that the Steering Council chose. (details are maybe slightly different, but what I wrote gives a general idea).

All that to say that Python or PEP440 would probably be a more appropriate/representative name.

I'm not sure if there is any specific issue or worry that you want to discuss about why it would be hard to get consensus on it or why we specifically would not want it. It doesn't do anything different from the existing version_split package orderer, it just solves the problem in a more general/flexible way.

The custom orderer would potentially open the door to managing a production configuration inside rez, which as we have discussed, is something that has been discussed multiple times and is going to be a hot topic.

Ordering based on PEP440 is also not necessarily guaranteed to be accepted, because it also raises multiple questions, but it still has more chances to appear in a release than the custom orderer. Additionally, the two are completely unrelated.

So splitting the two will at least simplify the discussions and it will also help us decide what we want to see released or not.

isohedronpipeline commented 6 months ago

pep440 was another option, but since the official name for the version scheme is actually pypa, I thought it was more appropriate. Pep440 is the proposal number, but pypa is the actual rules that covern the scheme

I'll be pedantic, but PyPA is a group of people (to be a member, you need to be a maintainer of a project under the PyPA GitHub organization), not rules. They host pip, setuptools, flit, etc and they also host the packaging specs. They also don't "govern" it any specs. Any change to a spec needs a new PEP (unless the change is really really minor/cosmetic) and the packaging specs/PEPs fall under one (or two I don't remember correctly) PEP delegates that the Steering Council chose. (details are maybe slightly different, but what I wrote gives a general idea).

All that to say that Python or PEP440 would probably be a more appropriate/representative name.

I've renamed it to PEP440PackageOrder, but I'm not it's the right choice. It directly uses the packaging rules, which are dictated by pypa, instead of recreating the pep440 rules internally. Calling it python is even worse than calling it pypa I think, since that is much more likely to support a different versioning scheme than PyPA, to say nothing of the obvious confusion that would arise from it.

I'm not sure if there is any specific issue or worry that you want to discuss about why it would be hard to get consensus on it or why we specifically would not want it. It doesn't do anything different from the existing version_split package orderer, it just solves the problem in a more general/flexible way.

The custom orderer would potentially open the door to managing a production configuration inside rez, which as we have discussed, is something that has been discussed multiple times and is going to be a hot topic.

I suppose so. I still don't understand the sentiment, since the other package orderers are literally for configuration of package resolution and there is no other way to achieve the goal of configuring second order requirements without explicitly fully baking environments, but I guess that's what a discussion is for.

Ordering based on PEP440 is also not necessarily guaranteed to be accepted, because it also raises multiple questions, but it still has more chances to appear in a release than the custom orderer. Additionally, the two are completely unrelated.

So splitting the two will at least simplify the discussions and it will also help us decide what we want to see released or not.

The custom package orderer can take advantage of the pep440 sorting. Since pep440 sorting is the first package orderer that actually not a configuration level orderer, there aren't other orderers that are suitable for that part of the test. I'm happy to separate them though. Custom will just need to come after pep440.