Init `nixVersions.minimum` and fix `lib` tests for all Nix versions by infinisil · Pull Request #234070 · NixOS/nixpkgs

infinisil · 2023-05-25T20:47:00Z

Description of changes

The lib.filesystem.pathType implementation now uses the new builtins.readFileType when available (in Nix 2.14) since #224834. But the tests introduced in that same PR weren't verified to work with 2.14, discovered in #233439, so this PR fixes that:

$ PATH=$(nix-build -A nixVersions.nix_2_14)/bin:"$PATH" lib/tests/filesystem.sh
test case failed:  lib.filesystem.pathType "/tmp/tmp.NXl2RIGW9m/non-existent" == error:
       … while calling the 'readFileType' builtin

         at «string»:1:46:

            1| { path }: let lib = import <nixpkgs/lib>; in lib.filesystem.pathType path
             |                                              ^

       error: getting status of '/tmp/tmp.NXl2RIGW9m/non-existent': No such file or directory, but error: evaluation aborted with the following error message: 'lib.filesystem.pathType: Path /tmp/tmp.NXl2RIGW9m/non-existent does not exist.' was expected

In addition, this PR fixes the tests for Nix version 2.3, since that's the minimum supported Nix version in Nixpkgs according to lib/minver.nix

And in order to ensure this doesn't happen again in the future, I created nixVersions.minimum to point to the Nix version from lib/minver.nix, and set up the lib tests to run with all of nixVersions.minimum, the default one, and nixVersions.unstable.

This work is sponsored by Antithesis ✨

Things done

Ran the above command successfully

lib/tests/sources.sh

lib/tests/filesystem.sh

roberth · 2023-05-26T19:34:38Z

lib/tests/release.nix

Just recurseIntoAttrs would be nice. That way you can run all of them, or just the nix version you're interested in.

Doesn't quite work without extra complexity because the part consuming this result uses it in buildInputs:

nixpkgs/pkgs/top-level/make-tarball.nix

Line 24 in 5f93f5f

buildInputs = with pkgs; [ nix.out jq lib-tests brotli ];

which can't handle such recursive attribute sets:

$ nix-build pkgs/top-level/release.nix -A tarball error: Dependency is not of a valid type: element 3 of buildInputs for nixpkgs-tarball-23.11pre1234.abcdef (use '--show-trace' to show detailed location information)

(how have I never seen this error before :o)

Maybe this has something useful?

lib: Working with nested attrsets of packages #202517

I can't believe that this is not a solved problem!

lib/tests/sources.sh

SuperSandro2000 · 2023-06-01T11:34:35Z

pkgs/tools/package-management/nix/default.nix

Suggested change

if ! self ? ${attribute} then

throw "The minimum supported Nix version is ${minver} (declared in lib/minver.nix), but pkgs.nixVersions.${attribute} does not exist."

else

nix;

if self ? ${attribute} then

nix

else

throw "The minimum supported Nix version is ${minver} (declared in lib/minver.nix), but pkgs.nixVersions.${attribute} does not exist.";

I think for code that performs validation, it's good for the error case to be in the then, even if that leads to a negation.

SuperSandro2000 · 2023-06-01T11:34:56Z

pkgs/tools/package-management/nix/default.nix

Suggested change

major = lib.versions.major minver;

minor = lib.versions.minor minver;

attribute = "nix_${major}_${minor}";

attribute = "nix_${lib.versions.major minver}_${lib.versions.minor minver}";

nit

roberth · 2023-06-01T11:42:06Z

pkgs/tools/package-management/nix/default.nix

Suggested change

# The minimum Nix version supported by Nixpkgs

# The minimum Nix version supported by Nixpkgs

# Note that some functionality *might* have been backported into this Nix version,

# making this package an inaccurate representation of what features are available

# in the actual lowest minver.nix *patch* version.

We generally don't backport a lot, and probably 2.3.0(1?) is not relevant anymore anyway, but testing against this may give a false sense of ~~security~~ correctness.

Good point, ideally we'd pin the first supported Nix version somehow. Don't want to go into that for now though, I applied this suggestion :)

roberth

Just the comment in the minimum version package. Otherwise lgtm.

Co-Authored-By: Robert Hensing <[email protected]>

The previous commits ensure that the tests also succeed with those versions

vcunat · 2023-06-22T09:59:51Z

Backport to 23.05 needed? It still won't pass with latest Nix version, which seems problematic, e.g. NixOS/nix#8569

github-actions · 2023-06-22T10:07:32Z

Backport failed for release-23.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin release-23.05
git worktree add -d .worktree/backport-234070-to-release-23.05 origin/release-23.05
cd .worktree/backport-234070-to-release-23.05
git checkout -b backport-234070-to-release-23.05
ancref=$(git merge-base 7d9999f2c178882a4656283c12a7501243626bf7 013acf2396f451c73b6cd6b0148df21cbaa4c165)
git cherry-pick -x $ancref..013acf2396f451c73b6cd6b0148df21cbaa4c165

vcunat · 2023-06-22T10:13:29Z

(Apparently, parts of this were already backported in the PR shown above.)

vcunat · 2023-06-22T12:24:47Z

So at least this one?
#239188

lib.filesystem.pathType: Fix tests for Nix >= 2.14

04db358

infinisil requested a review from roberth May 25, 2023 20:47

infinisil requested a review from edolstra as a code owner May 25, 2023 20:47

infinisil mentioned this pull request May 25, 2023

nix: 2.13.3 -> 2.15.1 #233439

Merged

12 tasks

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels May 25, 2023

infinisil force-pushed the pathType-tests branch from 8236994 to 22a5e80 Compare May 26, 2023 16:36

infinisil changed the title ~~lib.filesystem.pathType: Fix tests for Nix >= 2.14~~ Init nixVersions.minimum and fix lib tests for all Nix versions May 26, 2023

infinisil force-pushed the pathType-tests branch from 22a5e80 to 7c74ae3 Compare May 26, 2023 16:40

infinisil commented May 26, 2023

View reviewed changes

lib/tests/sources.sh Outdated Show resolved Hide resolved

roberth reviewed May 26, 2023

View reviewed changes

infinisil force-pushed the pathType-tests branch from 7c74ae3 to b53c340 Compare May 31, 2023 20:09

infinisil requested a review from roberth May 31, 2023 20:09

SuperSandro2000 reviewed Jun 1, 2023

View reviewed changes

roberth reviewed Jun 1, 2023

View reviewed changes

infinisil and others added 4 commits June 1, 2023 18:07

nixVersions.minver: init

29124b8

Co-Authored-By: Robert Hensing <[email protected]>

lib/tests/filesystem.sh: Check success and failure separately

de0c112

lib/tests: Fix when run with Nix 2.3

0b6021e

lib/tests: Also run with nixVersions.minimum and nixVersions.unstable

013acf2

The previous commits ensure that the tests also succeed with those versions

infinisil force-pushed the pathType-tests branch from b53c340 to 013acf2 Compare June 1, 2023 16:07

roberth approved these changes Jun 1, 2023

View reviewed changes

roberth merged commit fb21e6d into NixOS:master Jun 1, 2023

infinisil deleted the pathType-tests branch June 1, 2023 18:04

lorenzleutgeb mentioned this pull request Jun 7, 2023

[23.05] nixVersions: 2.15.0 -> 2.15.1, nixVersions: add 2.16.1, nixVersions.minver: init #236430

Merged

roberth added the backport release-23.05 label Jun 22, 2023

vcunat mentioned this pull request Jun 22, 2023

lib/tests: compatibility with more Nix versions #239188

Merged

12 tasks

-    if ! self ? ${attribute} then
-      throw "The minimum supported Nix version is ${minver} (declared in lib/minver.nix), but pkgs.nixVersions.${attribute} does not exist."
-    else
-      nix;
+    if self ? ${attribute} then
+      nix
+    else
+      throw "The minimum supported Nix version is ${minver} (declared in lib/minver.nix), but pkgs.nixVersions.${attribute} does not exist.";

-      major = lib.versions.major minver;
-      minor = lib.versions.minor minver;
-      attribute = "nix_${major}_${minor}";
+      attribute = "nix_${lib.versions.major minver}_${lib.versions.minor minver}";

-  # The minimum Nix version supported by Nixpkgs
+  # The minimum Nix version supported by Nixpkgs
+  # Note that some functionality *might* have been backported into this Nix version,
+  # making this package an inaccurate representation of what features are available
+  # in the actual lowest minver.nix *patch* version.

Uh oh!

Conversation

infinisil commented May 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Things done

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roberth left a comment

Choose a reason for hiding this comment

Uh oh!

vcunat commented Jun 22, 2023

Uh oh!

github-actions bot commented Jun 22, 2023

Uh oh!

vcunat commented Jun 22, 2023

Uh oh!

vcunat commented Jun 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

infinisil commented May 25, 2023 •

edited

Loading