Fixed Nix version and testing of multiple versions (and fix them for nixVersions.minimum) #83
Conversation
|
This does increase the closure size (gzip-compressed from 16MB to 33MB), because a Nix version is now included, but I value reproducibility more, so I think that's fine :) |
default.nix
Outdated
| @@ -45,6 +45,10 @@ let | |||
| settings.formatter.shfmt.options = [ "--space-redirects" ]; | |||
| }; | |||
|
|
|||
| # The resulting package is built to always use this Nix version, such that the result is reproducible | |||
| # TODO: Switch this to pkgs.nixVersions.minimum, because that's what Nixpkgs CI should use | |||
There was a problem hiding this comment.
I'm interested in the reasoning behind this "should". My experience is that it's better if CI uses the most recent version of its dependencies, since that usually means that fixes can occur at the right level. Using the minimum version, in my experience, leads to hard-to-remove workarounds for old bugs, which don't contribute to a better product. And the process of upgrading the minimum version has a nasty habit of finding odd new local minima, which then need to be worked around again.
The current Nixpkgs way of doing things -- having a stable version, which moves forward on a ~six monthly cadence in normal times -- is my preferred CI set up.
There was a problem hiding this comment.
Oh yeah, we totally shouldn't have to use the minimum version here. The minimum version should only be used for the part of CI that checks that everything evaluates, which is ofborg (though that actually use the minimum version!).
Removed the TODO now :)
| // Empty dir, needed so that no warnings are printed when testing older Nix versions | ||
| // that don't recognise certain newer keys in nix.conf | ||
| let nix_conf_dir = tempdir()?; |
There was a problem hiding this comment.
A cute way to do this would be to take a dependency on pkgs.emptyDirectory.
There was a problem hiding this comment.
Agreed, but since we're not in Nix here, it would be a bit cumbersome, so I think it's fine as-is :P
Yeah, it's in the noise. |
This makes the binary always use the same Nix version, instead of getting it from PATH. This makes it more reproducible, because issues from different Nix versions can't occur anymore, e.g. #78
While the parent commit makes it use a fixed Nix version, we should also run test for other Nix versions.
This is important because that's the Nix version that should really be used in CI
|
Ohh, we should be able avoid having to install Nix in Nixpkgs GitHub workflow then! |
No, you still need it for the |
This is a precursor to #79:
PATH, so we shouldn't ever have issues like nixpkgs-check-by-name reports IO error for Nix versions 2.21+ #78 anymorenixVersions.minimum, which we should switch to in the future, because it's what Nixpkgs CI should be using.This also makes it very easy to add further Nix versions to be tested, which is what #79 can then benefit from.
This work is sponsored by Antithesis ✨