AcademySoftwareFoundation / rez

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

[Feature] Support for correct pre-release version sorting #653

Open bpabel opened 5 years ago

bpabel commented 5 years ago

Both PEP 440 versions and SemVar 2.0 versions may contain pre-release (and other) version information besides just maj/min/patch.

Both PEP 440 and SemVar 2.0 utilize different sorting behavior for pre-releases than the default alphanumeric sort used by rez. For example, this is the expected sort order under PEP 440:

The default rez sort will sort them like this, with the base version before the pre-releases, and also doesn't support the pre-release separator being optional:

To maintain backwards compatibility, a config option could be added to enable support for this sorting behavior -- maybe use_pep440_versioning? With the expectation that if the package version isn't a valid PEP 440 version, it simply falls back to the normal alphanumeric sorting behavior.

This also raises another question of whether the version module should be vendored or not and instead be moved into rez proper, since it's already a pretty significant fork from the original and if we want it to be config aware, it should probably be moved into rez.

It may make sense to separate this ticket (and the config option) into two different options -- one for PEP 440 and one for SemVar 2.0, since they are different enough that someone may want one behavior and not the other. For this ticket, I'm mainly concerned with PEP 440 style versioning. I included SemVar 2.0 just to show it's not an issue unique to python packaging.

nerdvegas commented 5 years ago

This has come up before, but it's a harder problem than it appears on the surface.

The issue isn't with ordering so much, it's more about how rez version objects function. They're a kind of hierarchical mathematical object, and the solver completely relies on their behaviour. I won't get into the messy details just now, suffice to say that attempting to fix this gets really messy if you're doing that in the version submodule - edge cases appear that make no sense (eg 1.0.0-a1 is smaller than 1.0.0, yet 1.0.0 is a superset of 1.0.0-a1).

I think an approach that might work is to make use of package orderers for this. We could have a PEP440 orderer, which you could then associate by default with a package family. In order to do this we would need to:

Also, the 'version' submodule isn't actually 3rd party. I wrote it, and it's only in vendor/ because I'd been planning on releasing it as a separate package, but never did.

A

On Tue, Jun 25, 2019 at 3:48 AM Brendan Abel notifications@github.com wrote:

Both PEP 440 versions and SemVar 2.0 versions may contain pre-release (and other) version information besides just maj/min/patch.

Both PEP 440 and SemVar 2.0 utilize different sorting behavior for pre-releases than the default alphanumeric sort used by rez. For example, this is the expected sort order under PEP 440:

  • 1.0.0-a1
  • 1.0.0b1
  • 1.0.0.rc1
  • 1.0.0

The default rez sort will sort them like this, with the base version before the pre-releases, and also doesn't support the pre-release separator being optional:

  • 1.0.0
  • 1.0.0-a1
  • 1.0.0.rc1
  • 1.0.0b1

To maintain backwards compatibility, a config option could be added to enable support for this sorting behavior -- maybe use_pep440_versioning? With the expectation that if the package version isn't a valid PEP 440 version, it simply falls back to the normal alphanumeric sorting behavior.

This also raises another question of whether the version module should be vendored or not and instead be moved into rez proper, since it's already a pretty significant fork from the original and if we want it to be config aware, it should probably be moved into rez.

It may make sense to separate this ticket (and the config option) into two different options -- one for PEP 440 and one for SemVar 2.0, since they are different enough that someone may want one behavior and not the other. For this ticket, I'm mainly concerned with PEP 440 style versioning. I included SemVar 2.0 just to show it's not an issue unique to python packaging.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/653?email_source=notifications&email_token=AAMOUSSKZMS4FAHYUSNMCRDP4ECH7A5CNFSM4H3A4EEKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4G3LJXYQ, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMOUSQS7SOWQ3Z66KAELA3P4ECH7ANCNFSM4H3A4EEA .

aboellinger commented 2 years ago

Hi !

Running into the same issue myself, as I am used to version stuff following the semver specification.

It is unclear to me what the current preferred approach would be to handle prereleases in rez packages ?

Cheers

ColinKennedy commented 2 years ago

This comes up fairly often and probably deserves a specific video to address it. There's more than one way to handle betas / pre-releases and this is my personal opinion, having done this for prereleases often at a company.

The jist is to make a new major / minor / patch as you normally would. So instead of making prerelease version 1.2.0-rc.1, simply name it 1.2.0. Use rez-build --prefix to install it to a non-release folder. Now include that new, extra path in the resolver and ask artists to use that (how you do this will vary depending on what your pipeline does.

If it's all terminal-based, you could ...

  1. make a .rxt, using

    REZ_PACKAGES_PATH=/foo/bar/bazz:`rez-config release_packages_path` rez-env --output /network/path/to/context.rxt

    then copy that .rxt somewhere over the network, and then ask users to load it, using rez-env --input /network/path/to/context.rxt

  2. Build to a network folder with rez-build --prefix /foo/bar/bazz and then ask the users who will test it, please include /foo/bar/bazz in your rez-env command, e.g.

    rez-env --paths /foo/bar/bazz:`rez-config release_packages_path`

I personally find (1) cleaner than (2) but that's probably down to preference.

If users interact with Rez via some GUI or some proprietary solution then I'm not 100% sure how to recommend it, but basically you'd want to do the equivalent of (2), but in GUI form. Ideally, you'd want to change the paths under the hood so users can continue to use their usual button(s) to load tools and not have to think too hard about loading things the "right" way.

Neither solution is perfect due to some current Rez behavior (https://github.com/nerdvegas/rez/issues/1263#issuecomment-1081422278) but you'd hit that particular problem no matter what method you use.

In short, just treat release candidates like regular packages but install them to another path. Temporarily append that path to those candidate package(s) so they're included in user resolves. Then once you're happy with those packages, release them as normal and then remove the temporary path completely. Now the released packages will get picked up instead of the beta / rc / etc. It's consistent, reliable, and your versions are kept simple. The only thing "-beta", "-rc", etc is convey a meaning of "this isn't 100% ready yet and is being tested". If a package loads from a non-standard release path, it conveys the same meaning, in my opinion.

flord commented 2 years ago

When using the extra path solution, you need a different extra package repository for each package, otherwise you risk loading other beta packages in your environment. I'm going to install rez in a new setup in the coming days and I am considering solving this problem by simply having a mendatory tail string to every version. We already use "-r1" for DCC packages to version the different rez-package versions of a given DCC version. Why not use it everywhere? I haven't decided yet on which letter I'm going to use, but let's say I use "z". For every rez package I release, I use the version syntax ... like so:

bpabel commented 2 years ago

@ColinKennedy The workflow you described may be a good alternative for internal development, but often the alpha/beta/prerelease versions aren't within our control. They come from upstream projects.

The config features @nerdvegas described in the comment above for specifying package orderers in the config globally and per family have been implemented now. Using the package orderer system is probably the best way forward to handle this issue right now. I don't think that specifying package orderers by family in the config is going to be a scalable solution, and it's not a config option that can be overridden at the package level. I did a little testing with a custom pep440 package orderer. The issue I found is that any orderer that expects some type of strict structured format needs to have a fallback mechanism to handle packages and package families that don't follow that convention. So you can't really have a "pep-440" orderer, it has to be a "pep-440 prioritized" orderer with a fallback to some other ordering system for non-conforming packages. Either that or just disallow build/release/resolves with non-conforming packages and raise errors for them.

ColinKennedy commented 2 years ago

When using the extra path solution, you need a different extra package repository for each package, otherwise you risk loading other beta packages in your environment.

@flord To be clear to anyone reading this thread, you need one package path for all beta packages which you mean to test, otherwise you risk loading other beta packages in your environment. So if you have 5 betas / RCs you want to test at once, you install them into 1, single path and then include that as previously mentioned.

Anyway, yes you'd need to choose a new folder each time otherwise previous betas may be chosen. I usually make a path which matches the ticket that the betas / RCs are associated with. e.g. /path/to/network/PIPE-1234-some_beta_description

ColinKennedy commented 2 years ago

but often the alpha/beta/prerelease versions aren't within our control. They come from upstream projects.

@bpabel I should've clarified in my original posting - what I described actually works with or without beta / rc suffixes. In other words, you can use my suggestion and simply continue to use 1.2.0-beta.1, if you absolutely have to. The "using only base SemVer" is optional.

However if you do so, you run into the same problem as you originally described, which is where a package orderer comes into play. Allan mentioned that both here and in the other thread I'd linked to (https://github.com/nerdvegas/rez/issues/1263#issuecomment-1086347517)

specifying package orderers by family in the config is going to be a scalable solution, and it's not a config option that can be overridden at the package level.

I agree and would rather the orderer prefer package families from specific paths over others. That way we don't have to specify the packages at all and can simply install to a known location and it'll pick up the betas as needed. Again, https://github.com/nerdvegas/rez/issues/1263 goes into that more in detail.

It sounds like we're converging on a solution, it's a matter of adding this behavior, getting it out to everyone, and documenting how / why it's useful.

herronelou commented 6 months ago

Just tagging that this is related to #302