Add Nix versioning scheme - #212
Conversation
Signed-off-by: Samiser <github@me.samiser.xyz>
There was a problem hiding this comment.
As a former Nix maintainer and current maintainer of the Nixpkgs security tracker, this looks correct.
But in order to avoid confusion, I recommend adding documentation along the lines of
Compare version strings of software distributed through Nixpkgs. Refer to
Nixpkgs contributor documentation on how versioning is handled there:
https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md#versioning
Note that Nix version handling is not authoritative for versioning syntax or
semantics in Nixpkgs. It's merely a compatibility constraint for supporting
unmaintained legacy Nix use cases around `nix-env`. But one can assume that
versions of a given package from any Nixpkgs release will be ordered correctly
by Nix, which is why it's included here.
There was a problem hiding this comment.
How about we generate that on the fly in the test? We're adding the dependency and shell out to nix anyway. If there's a way to express that it's a test-only dependency, that would be ideal.
There was a problem hiding this comment.
so the dependency/shelling out is just for the test generation which is done offline (doesn't run in CI). this keeps the test suite deterministic without requiring a nix installation. i committed the json to match the existing convention and to avoid adding the dependency on nix into CI itself (and im not sure there even is a way to add such a dependency via the project code).
i would be open to making the json smaller by reducing the amount of atoms for the generated combinations but i was thinking i'll see what the aboutcode folks think (maybe its just fine for the json to be long? 😅)
Signed-off-by: Samiser <github@me.samiser.xyz>
Shriprasad-P
left a comment
Shriprasad-P
left a comment
There was a problem hiding this comment.
Review
PR: Add Nix versioning scheme
Touched: README.rst, etc/scripts/gen_nix_version_cmp.py, lgpl-2.1.LICENSE, setup.cfg, src/univers/nix.py
- Includes tests — helpful for locking behavior.
- Size: +33141/-1.
Drive-by review after reading the diff.
3 participants
Adds
NixVersionandNixVersionRange(vers:nix/), ordered exactly as Nix'sbuiltins.compareVersions. The algorithm is embedded assrc/univers/nix.pywith.ABOUT/.NOTICEprovenance, following thearch.pyconvention.Also I know the diff line count looks daunting (sorry) but the vast majority of the diff is test data (see Testing).
Motivation
The NixOS security tracker is looking at using univers for CVE constraint checking, and real nixpkgs versions like
2.3pre1or9.8p1follow an ordering no existing scheme reproduces (any string is valid, onlypremarks a pre-release, a leadingvis significant).Testing
The fixture is all 2,025 pairs of a 45-string corpus with expected values produced by the reference implementation itself via
nix-instantiate.etc/scripts/gen_nix_version_cmp.pyregenerates it byte-identically, and CI does not need Nix. That design is why the fixture is ~32k lines, but the rest of the diff is ~400.I know this is longer than other existing test data, but it's just the nature of the combinatorics. If it's too long I could potentially reduce the number of atoms, but I would argue it's better to be as comprehensive as possible.
Other Notes
The ported algorithm is LGPL-2.1-or-later, so
setup.cfgand the root license files are updated accordingly. Nix has no native range notation, so there is nofrom_native.NixVersionis also unhashable likeArchLinuxVersion, for the same reason.