[RFC 0147] Redefine unstable version format and comparison

[rhendric]

#107, any% speedrun

Rendered

[AndersonTorres]

Woohoo! Can I drop the older one? :3

[rhendric]

Woohoo! Can I drop the older one? :3

I mean, if you're asking me, I'm a nobody and this is my first RFC; you're a committer and have worked on several. I defer entirely to your judgment. Maybe having two approaches in parallel makes it more likely that one will succeed, and 107 is farther along than this, obviously. But if you're sick of trying to keep 107 going, that would make sense to me also.

[kevincox]

If you as the author think that this RFC is preferable feel free to close the old one. It can always be reopened to copied to a new RFC if interest in that approach reappears.

[Ericson2314]

I rather we stop using parseDrvName, and have a "compare just versions" that no longer tries to ignore the name part.

Then we don't have to create any specifial cases for "unstable", right?

[rhendric]

I rather we stop using parseDrvName, and have a "compare just versions" that no longer tries to ignore the name part.

Example? How would you compare "pkg-unstable-2023-05-27" to "pkg-1.2.0" (if the existing wording of the manual is followed) or "pkg-1.2-unstable-2023-05-27" to "pkg-1.2.1" (if NixOS/nixpkgs#234201 is adopted)?

[jtojnar]

Example? How would you compare "pkg-unstable-2023-05-27" to "pkg-1.2.0" (if the existing wording of the manual is followed) or "pkg-1.2-unstable-2023-05-27" to "pkg-1.2.1" (if NixOS/nixpkgs#234201 is adopted)?

1. You would not compare name attributes, you would compare version attribute of the derivation, when present.
2. nix-env --install without -A should be deprecated and -iA should gain the support for recording the attributes used to install.
3. nix-env --upgrade should not really compare versions at all, --always should be always used. Because:
   • Nixpkgs can update security-critical dependencies without bumping the installed package version.
   • Or Nixpkgs can revert updates when they bring critical issues noticed after merging.
   • And Nixpkgs may switch from ancient stable versions of a project to recent git snapshots, and back when releases reappear. So unstable-2023-05-27 and 1.2.0 are really incomparable and we would want to always switch here anyway.

Alternately, we could deprecate nix-env completely in favour of flakes but I am not sure how far off that is. And it probably will not be removed for a while after anyway.

Either way, parseDrvName is an ugly hack and we should not add more project-wide infrastructure to improve nix-env. Just improve nix-env directly, it will make its eventual deprecation more clear-cut.

[rhendric]

So much for nix-env. What should a Nixpkgs package do if it wants to compare the version of a dependency against a known version string, to permit/forbid the derivation or to enable/disable a feature or patch? If my package uses lib.versionAtLeast some-dep.version "1.2.1", and some-dep.version is either "unstable-2023-05-27" (status quo) or "1.2-unstable-2023-05-27" (NixOS/nixpkgs#234201), how do we get the result of the versionAtLeast call to be false?

Nixpkgs could have its own version comparison logic, deviating from builtins.compareVersions; would that be tolerated?

[alyssais]

So much for `nix-env`. What should a Nixpkgs package do if it wants to compare the version of a dependency against a known version string, to permit/forbid the derivation or to enable/disable a feature or patch? If my package uses `lib.versionAtLeast some-dep.version "1.2.1"`, and `some-dep.version` is either `"unstable-2023-05-27"` (status quo) or `"1.2-unstable-2023-05-27"` (NixOS/nixpkgs#234201), how do we get the result of the `versionAtLeast` call to be false?

This was discussed at length in the previous RFC, but basically: you cannot have a single function that compares versions correctly for arbitrary packages, because different packages have different conventions. It's actually very rare (if it even happens at all) that this situation would come up in Nixpkgs. Usually, we package some number of stable versions of a package (often 1), and sometimes a single unstable version (usually HEAD). In which case to distinguish patches that should only be applied to unstable, you can just pass an "unstable" boolean argument to your common builder function, or you can check if the version starts with "unstable". To apply patches only to some stable versions, you can use lib.versionAtLeast or lib.versionOlder if they work for that particular package's versioning scheme, but if they don't (because they _can't_ work for every package), that's okay too. For uncommon cases, you can implement the specific logic needed by those packages.

[jtojnar]

If my package uses lib.versionAtLeast some-dep.version "1.2.1", and some-dep.version is either "unstable-2023-05-27" (status quo) or "1.2-unstable-2023-05-27" (NixOS/nixpkgs#234201), how do we get the result of the versionAtLeast call to be false?

builtins.compareVersions "1.2-unstable-2023-05-27" "1.2.1" already behaves as you would expect so no changes to compareVersions should be necessary.

And you do not even need parseDrvName as version is easily accessible during evaluation. Which is, as I understand it, what John is saying.

So the only part that is could be needed is prefixing the version string with the last stable version for unstable packages. Which is precisely the bikeshed-prone part #107 got stuck on.

[rhendric]

builtins.compareVersions "1.2-unstable-2023-05-27" "1.2.1" already behaves as you would expect so no changes to compareVersions should be necessary.

That's true, and I had thought it wasn't. Mea culpa.

Yeah, great, if we can ignore nix-env and parseDrvName, then everything I care about is under ‘future work’ here or is captured in NixOS/nixpkgs#234201. Closing until and unless that PR runs into objections that would require a larger change to resolve.