Merge pull request #83 from NixOS/nix-versions

Fixed Nix version and testing of multiple versions (and fix them for nixVersions.minimum)

CONTRIBUTING.md

@@ -61,6 +61,10 @@ Integration tests are declared in [`./tests`](./tests) as subdirectories imitati
   A file containing the expected standard output.
   The default is expecting an empty standard output.
 
+  This file is matched against the error almost literally,
+  with the only exception being that the `@REDACTED@` string can match anything,
+  which is useful for non-deterministic errors.
+
 ## Automation
 
 Pinned dependencies are [regularly updated automatically](./.github/workflows/update.yml).

default.nix

@@ -45,6 +45,9 @@ let
     settings.formatter.shfmt.options = [ "--space-redirects" ];
   };
 
+  # The resulting package is built to always use this Nix version, such that the result is reproducible
+  defaultNixPackage = pkgs.nix;
+
   packages = {
     build = pkgs.callPackage ./package.nix {
       inherit
@@ -54,10 +57,12 @@ let
         testNixpkgsPath
         version
         ;
+      nix = defaultNixPackage;
     };
 
     shell = pkgs.mkShell {
       env.NIX_CHECK_BY_NAME_EXPR_PATH = toString runtimeExprPath;
+      env.NIX_CHECK_BY_NAME_NIX_PACKAGE = lib.getBin defaultNixPackage;
       env.NIX_PATH = "test-nixpkgs=${toString testNixpkgsPath}:test-nixpkgs/lib=${toString nixpkgsLibPath}";
       env.RUST_SRC_PATH = "${pkgs.rustPlatform.rustLibSrc}";
       inputsFrom = [ packages.build ];
@@ -69,6 +74,7 @@ let
         rust-analyzer
         rustfmt
         treefmtEval.config.build.wrapper
+        defaultNixPackage
       ];
     };
 

package.nix

@@ -2,9 +2,16 @@
   lib,
   rustPlatform,
   nix,
+  nixVersions,
   clippy,
   makeWrapper,
 
+  nixVersionsToTest ? [
+    nix
+    nixVersions.stable
+    nixVersions.minimum
+  ],
+
   nixpkgsLibPath,
   initNix,
   runtimeExprPath,
@@ -28,19 +35,34 @@ rustPlatform.buildRustPackage {
   };
   cargoLock.lockFile = ./Cargo.lock;
   nativeBuildInputs = [
-    nix
     clippy
     makeWrapper
   ];
   env.NIX_CHECK_BY_NAME_EXPR_PATH = "${runtimeExprPath}";
+  env.NIX_CHECK_BY_NAME_NIX_PACKAGE = lib.getBin nix;
   env.NIX_PATH = "test-nixpkgs=${testNixpkgsPath}:test-nixpkgs/lib=${nixpkgsLibPath}";
-  preCheck = initNix;
-  postCheck = ''
+  checkPhase = ''
+    # This path will be symlinked to the current version that is being tested
+    nixPackage=$(mktemp -d)/nix
+    # For initNix
+    export PATH=$nixPackage/bin:$PATH
+    # This is what nixpkgs-check-by-name uses
+    export NIX_CHECK_BY_NAME_NIX_PACKAGE=$nixPackage
+
+    ${lib.concatMapStringsSep "\n" (nix: ''
+      ln -s ${lib.getBin nix} "$nixPackage"
+      echo "Testing with $(nix --version)"
+      ${initNix}
+      runHook cargoCheckHook
+      rm "$nixPackage"
+    '') (lib.unique nixVersionsToTest)}
+
     # --tests or --all-targets include tests for linting
     cargo clippy --all-targets -- -D warnings
   '';
   postInstall = ''
     wrapProgram $out/bin/nixpkgs-check-by-name \
-      --set NIX_CHECK_BY_NAME_EXPR_PATH "$NIX_CHECK_BY_NAME_EXPR_PATH"
+      --set NIX_CHECK_BY_NAME_EXPR_PATH "$NIX_CHECK_BY_NAME_EXPR_PATH" \
+      --set NIX_CHECK_BY_NAME_NIX_PACKAGE ${lib.getBin nix}
   '';
 }

src/eval.rs

@@ -125,8 +125,12 @@ pub fn check_values(
     let expr_path = std::env::var("NIX_CHECK_BY_NAME_EXPR_PATH")
         .with_context(|| "Could not get environment variable NIX_CHECK_BY_NAME_EXPR_PATH")?;
 
+    // Pinning nix in this way makes the tool more reproducible
+    let nix_package = std::env::var("NIX_CHECK_BY_NAME_NIX_PACKAGE")
+        .with_context(|| "Could not get environment variable NIX_CHECK_BY_NAME_NIX_PACKAGE")?;
+
     // With restrict-eval, only paths in NIX_PATH can be accessed. We explicitly specify them here.
-    let mut command = process::Command::new("nix-instantiate");
+    let mut command = process::Command::new(format!("{nix_package}/bin/nix-instantiate"));
     command
         // Capture stderr so that it can be printed later in case of failure
         .stderr(process::Stdio::piped())

src/main.rs

@@ -181,6 +181,7 @@ mod tests {
     use crate::process;
     use crate::utils;
     use anyhow::Context;
+    use std::ffi::OsStr;
     use std::fs;
     use std::path::Path;
     use tempfile::{tempdir_in, TempDir};
@@ -277,14 +278,24 @@ mod tests {
         } else {
             Path::new("tests/empty-base")
         };
+        // 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()?;
 
         // We don't want coloring to mess up the tests
-        let writer = temp_env::with_var("NO_COLOR", Some("1"), || -> anyhow::Result<_> {
-            let mut writer = vec![];
-            process(base_nixpkgs.to_owned(), path.to_owned(), true, &mut writer)
-                .with_context(|| format!("Failed test case {name}"))?;
-            Ok(writer)
-        })?;
+        let writer = temp_env::with_vars(
+            [
+                ("NO_COLOR", Some(OsStr::new("1"))),
+                // See above comment on nix_conf_dir
+                ("NIX_CONF_DIR", Some(nix_conf_dir.path().as_os_str())),
+            ],
+            || -> anyhow::Result<_> {
+                let mut writer = vec![];
+                process(base_nixpkgs.to_owned(), path.to_owned(), true, &mut writer)
+                    .with_context(|| format!("Failed test case {name}"))?;
+                Ok(writer)
+            },
+        )?;
 
         let expr_path = std::env::var("NIX_CHECK_BY_NAME_EXPR_PATH")
             .with_context(|| "Could not get environment variable NIX_CHECK_BY_NAME_EXPR_PATH")?;
@@ -293,7 +304,16 @@ mod tests {
         // on the output, which we need to relativise for the tests to succeed everywhere
         let actual_errors = String::from_utf8_lossy(&writer).replace(&expr_path, "src/eval.nix");
 
-        if actual_errors != expected_errors {
+        let pattern = format!(
+            "^{}$",
+            regex::escape(expected_errors).replace("@REDACTED@", ".*")
+        );
+
+        let expected_errors_regex = regex::RegexBuilder::new(&pattern)
+            .dot_matches_new_line(true)
+            .build()?;
+
+        if !expected_errors_regex.is_match(&actual_errors) {
             panic!(
                 "Failed test case {name}, expected these errors:\n=======\n{}\n=======\n\
                 but got these:\n=======\n{}\n=======",

tests/by-name-failure/expected

@@ -1,20 +1,4 @@
 trace: This should be on stderr!
-error:
-       … while evaluating list element at index 3
-
-       … while evaluating list element at index 1
-
-       … while evaluating attribute 'ByName'
-
-         at src/eval.nix:76:7:
-
-           75|       inherit name;
-           76|       value.ByName =
-             |       ^
-           77|         if !pkgs ? ${name} then
-
-       (stack trace truncated; use '--show-trace' to show the full trace)
-
-       error: This is an error!
+@REDACTED@error: This is an error!@REDACTED@
 - Nix evaluation failed for some package in `pkgs/by-name`, see error above
 This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.