Merge pull request #281399 from tweag/by-name-minor-improvements

tests.nixpkgs-check-by-name: Minor improvements

pkgs/test/nixpkgs-check-by-name/default.nix

@@ -11,6 +11,7 @@
 let
   runtimeExprPath = ./src/eval.nix;
   nixpkgsLibPath = ../../../lib;
+  testNixpkgsPath = ./tests/mock-nixpkgs.nix;
 
   # Needed to make Nix evaluation work inside nix builds
   initNix = ''
@@ -26,10 +27,20 @@ let
     nix-store --init
   '';
 
+  fs = lib.fileset;
+
   package =
     rustPlatform.buildRustPackage {
       name = "nixpkgs-check-by-name";
-      src = lib.cleanSource ./.;
+      src = fs.toSource {
+        root = ./.;
+        fileset = fs.unions [
+          ./Cargo.lock
+          ./Cargo.toml
+          ./src
+          ./tests
+        ];
+      };
       cargoLock.lockFile = ./Cargo.lock;
       nativeBuildInputs = [
         nix
@@ -38,7 +49,7 @@ let
         makeWrapper
       ];
       env.NIX_CHECK_BY_NAME_EXPR_PATH = "${runtimeExprPath}";
-      env.NIXPKGS_LIB_PATH = "${nixpkgsLibPath}";
+      env.NIX_PATH = "test-nixpkgs=${testNixpkgsPath}:test-nixpkgs/lib=${nixpkgsLibPath}";
       preCheck = initNix;
       postCheck = ''
         cargo fmt --check
@@ -50,7 +61,7 @@ let
       '';
       passthru.shell = mkShell {
         env.NIX_CHECK_BY_NAME_EXPR_PATH = toString runtimeExprPath;
-        env.NIXPKGS_LIB_PATH = toString nixpkgsLibPath;
+        env.NIX_PATH = "test-nixpkgs=${toString testNixpkgsPath}:test-nixpkgs/lib=${toString nixpkgsLibPath}";
         inputsFrom = [ package ];
       };
 

pkgs/test/nixpkgs-check-by-name/src/eval.rs

@@ -2,8 +2,6 @@ use crate::nixpkgs_problem::NixpkgsProblem;
 use crate::ratchet;
 use crate::structure;
 use crate::validation::{self, Validation::Success};
-use std::collections::HashMap;
-use std::ffi::OsString;
 use std::path::Path;
 
 use anyhow::Context;
@@ -73,7 +71,7 @@ enum CallPackageVariant {
 pub fn check_values(
     nixpkgs_path: &Path,
     package_names: Vec<String>,
-    eval_nix_path: &HashMap<String, PathBuf>,
+    keep_nix_path: bool,
 ) -> validation::Result<ratchet::Nixpkgs> {
     // Write the list of packages we need to check into a temporary JSON file.
     // This can then get read by the Nix evaluation.
@@ -99,8 +97,6 @@ pub fn check_values(
     command
         // Inherit stderr so that error messages always get shown
         .stderr(process::Stdio::inherit())
-        // Clear NIX_PATH to be sure it doesn't influence the result
-        .env_remove("NIX_PATH")
         .args([
             "--eval",
             "--json",
@@ -121,15 +117,12 @@ pub fn check_values(
         .arg("-I")
         .arg(nixpkgs_path);
 
-    // Also add extra paths that need to be accessible
-    for (name, path) in eval_nix_path {
-        command.arg("-I");
-        let mut name_value = OsString::new();
-        name_value.push(name);
-        name_value.push("=");
-        name_value.push(path);
-        command.arg(name_value);
+    // Clear NIX_PATH to be sure it doesn't influence the result
+    // But not when requested to keep it, used so that the tests can pass extra Nix files
+    if !keep_nix_path {
+        command.env_remove("NIX_PATH");
     }
+
     command.args(["-I", &expr_path]);
     command.arg(expr_path);
 

pkgs/test/nixpkgs-check-by-name/src/main.rs

@@ -12,7 +12,6 @@ use crate::validation::Validation::Success;
 use anyhow::Context;
 use clap::Parser;
 use colored::Colorize;
-use std::collections::HashMap;
 use std::io;
 use std::path::{Path, PathBuf};
 use std::process::ExitCode;
@@ -45,12 +44,7 @@ pub struct Args {
 
 fn main() -> ExitCode {
     let args = Args::parse();
-    match process(
-        &args.base,
-        &args.nixpkgs,
-        &HashMap::new(),
-        &mut io::stderr(),
-    ) {
+    match process(&args.base, &args.nixpkgs, false, &mut io::stderr()) {
         Ok(true) => {
             eprintln!("{}", "Validated successfully".green());
             ExitCode::SUCCESS
@@ -71,9 +65,9 @@ fn main() -> ExitCode {
 /// # Arguments
 /// - `base_nixpkgs`: Path to the base Nixpkgs to run ratchet checks against.
 /// - `main_nixpkgs`: Path to the main Nixpkgs to check.
-/// - `eval_accessible_paths`:
-///   Extra paths that need to be accessible to evaluate Nixpkgs using `restrict-eval`.
-///   This is used to allow the tests to access the mock-nixpkgs.nix file
+/// - `keep_nix_path`: Whether the value of the NIX_PATH environment variable should be kept for
+/// the evaluation stage, allowing its contents to be accessed.
+///   This is used to allow the tests to access e.g. the mock-nixpkgs.nix file
 /// - `error_writer`: An `io::Write` value to write validation errors to, if any.
 ///
 /// # Return value
@@ -83,15 +77,15 @@ fn main() -> ExitCode {
 pub fn process<W: io::Write>(
     base_nixpkgs: &Path,
     main_nixpkgs: &Path,
-    eval_nix_path: &HashMap<String, PathBuf>,
+    keep_nix_path: bool,
     error_writer: &mut W,
 ) -> anyhow::Result<bool> {
     // Check the main Nixpkgs first
-    let main_result = check_nixpkgs(main_nixpkgs, eval_nix_path, error_writer)?;
+    let main_result = check_nixpkgs(main_nixpkgs, keep_nix_path, error_writer)?;
     let check_result = main_result.result_map(|nixpkgs_version| {
         // If the main Nixpkgs doesn't have any problems, run the ratchet checks against the base
         // Nixpkgs
-        check_nixpkgs(base_nixpkgs, eval_nix_path, error_writer)?.result_map(
+        check_nixpkgs(base_nixpkgs, keep_nix_path, error_writer)?.result_map(
             |base_nixpkgs_version| {
                 Ok(ratchet::Nixpkgs::compare(
                     base_nixpkgs_version,
@@ -119,7 +113,7 @@ pub fn process<W: io::Write>(
 /// ratchet check against another result.
 pub fn check_nixpkgs<W: io::Write>(
     nixpkgs_path: &Path,
-    eval_nix_path: &HashMap<String, PathBuf>,
+    keep_nix_path: bool,
     error_writer: &mut W,
 ) -> validation::Result<ratchet::Nixpkgs> {
     Ok({
@@ -140,7 +134,7 @@ pub fn check_nixpkgs<W: io::Write>(
         } else {
             check_structure(&nixpkgs_path)?.result_map(|package_names|
                 // Only if we could successfully parse the structure, we do the evaluation checks
-                eval::check_values(&nixpkgs_path, package_names, eval_nix_path))?
+                eval::check_values(&nixpkgs_path, package_names, keep_nix_path))?
         }
     })
 }
@@ -150,10 +144,8 @@ mod tests {
     use crate::process;
     use crate::utils;
     use anyhow::Context;
-    use std::collections::HashMap;
     use std::fs;
     use std::path::Path;
-    use std::path::PathBuf;
     use tempfile::{tempdir_in, TempDir};
 
     #[test]
@@ -234,20 +226,6 @@ mod tests {
     }
 
     fn test_nixpkgs(name: &str, path: &Path, expected_errors: &str) -> anyhow::Result<()> {
-        let eval_nix_path = HashMap::from([
-            (
-                "test-nixpkgs".to_string(),
-                PathBuf::from("tests/mock-nixpkgs.nix"),
-            ),
-            (
-                "test-nixpkgs/lib".to_string(),
-                PathBuf::from(
-                    std::env::var("NIXPKGS_LIB_PATH")
-                        .with_context(|| "Could not get environment variable NIXPKGS_LIB_PATH")?,
-                ),
-            ),
-        ]);
-
         let base_path = path.join("base");
         let base_nixpkgs = if base_path.exists() {
             base_path.as_path()
@@ -258,7 +236,7 @@ mod tests {
         // 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, &path, &eval_nix_path, &mut writer)
+            process(base_nixpkgs, &path, true, &mut writer)
                 .with_context(|| format!("Failed test case {name}"))?;
             Ok(writer)
         })?;