From 47848487b2511345b8dfabe24a7b000e3cd9ba8a Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Sat, 20 Jul 2024 10:34:55 -0700 Subject: [PATCH] eval: use a work directory instead of an environment variable to control the Nix expression No test used the `NIX_CHECK_BY_NAME_EXPR_PATH` to change the Nix file that was read, and it's simple to rebuild in order to test out changes to the `src/eval.nix`. --- default.nix | 3 --- package.nix | 3 --- src/eval.rs | 50 ++++++++++++++++++++++++++++---------------------- src/main.rs | 27 ++++++++++++--------------- 4 files changed, 40 insertions(+), 43 deletions(-) diff --git a/default.nix b/default.nix index 404a707..8bb7bac 100644 --- a/default.nix +++ b/default.nix @@ -14,7 +14,6 @@ let }; inherit (pkgs) lib; - runtimeExprPath = ./src/eval.nix; testNixpkgsPath = ./tests/mock-nixpkgs.nix; nixpkgsLibPath = nixpkgs + "/lib"; @@ -53,7 +52,6 @@ let inherit nixpkgsLibPath initNix - runtimeExprPath testNixpkgsPath version ; @@ -61,7 +59,6 @@ let }; 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}"; diff --git a/package.nix b/package.nix index b37d2f2..a2e1da5 100644 --- a/package.nix +++ b/package.nix @@ -14,7 +14,6 @@ nixpkgsLibPath, initNix, - runtimeExprPath, testNixpkgsPath, version, }: @@ -38,7 +37,6 @@ rustPlatform.buildRustPackage { 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}"; checkPhase = '' @@ -62,7 +60,6 @@ rustPlatform.buildRustPackage { ''; 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_NIX_PACKAGE ${lib.getBin nix} ''; } diff --git a/src/eval.rs b/src/eval.rs index c72546b..b9dfeb9 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -11,13 +11,16 @@ use crate::validation::ResultIteratorExt as _; use crate::validation::{self, Validation::Success}; use crate::NixFileStore; use relative_path::RelativePathBuf; +use std::fs; use std::path::Path; use anyhow::Context; use serde::Deserialize; use std::path::PathBuf; use std::process; -use tempfile::NamedTempFile; +use tempfile::Builder; + +const EVAL_NIX: &[u8] = include_bytes!("eval.nix"); /// Attribute set of this structure is returned by `./eval.nix` #[derive(Deserialize)] @@ -106,26 +109,29 @@ pub fn check_values( package_names: Vec, is_test: bool, ) -> validation::Result { - // Write the list of packages we need to check into a temporary JSON file. - // This can then get read by the Nix evaluation. - let attrs_file = NamedTempFile::new().with_context(|| "Failed to create a temporary file")?; + let work_dir = Builder::new() + .prefix("nixpkgs-check-by-name") + .tempdir() + .with_context(|| "Failed to create a working directory")?; - // We need to canonicalise this path. If it's a symlink (which can be the case on Darwin), - // Nix would need to read both the symlink and the target path, and therefore need two NIX_PATH - // entries for restrict-eval. If we resolve the symlinks, we only need one predictable entry. - let attrs_file_path = attrs_file.path().canonicalize()?; + // Canonicalize the path so that if a symlink were returned, we wouldn't ask Nix to follow it. + let work_dir_path = work_dir.path().canonicalize()?; - serde_json::to_writer(&attrs_file, &package_names).with_context(|| { + // Write the list of packages we need to check into a temporary JSON file. + let package_names_path = work_dir_path.join("package-names.json"); + let package_names_file = fs::File::create(&package_names_path)?; + serde_json::to_writer(&package_names_file, &package_names).with_context(|| { format!( - "Failed to serialise the package names to the temporary path {}", - attrs_file_path.display() + "Failed to serialise the package names to the work dir {}", + work_dir_path.display() ) })?; - 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")?; + // Write the Nix file into the work directory. + let eval_nix_path = work_dir_path.join("eval.nix"); + fs::write(&eval_nix_path, EVAL_NIX)?; - // Pinning nix in this way makes the tool more reproducible + // 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")?; @@ -141,13 +147,13 @@ pub fn check_values( "--readonly-mode", "--restrict-eval", ]) - // Pass the path to the attrs_file as an argument and add it to the NIX_PATH so it can be - // accessed in restrict-eval mode. - .args(["--arg", "attrsPath"]) - .arg(&attrs_file_path) + // Add the work directory to the NIX_PATH so that it can be accessed in restrict-eval mode. .arg("-I") - .arg(&attrs_file_path) - // Same for the nixpkgs to test. + .arg(&work_dir_path) + .args(["--arg", "attrsPath"]) + .arg(&package_names_path) + // Same for the nixpkgs to test, adding it to the NIX_PATH so it can be accessed in + // restrict-eval mode. .args(["--arg", "nixpkgsPath"]) .arg(nixpkgs_path) .arg("-I") @@ -162,8 +168,7 @@ pub fn check_values( command.arg("--show-trace"); } - command.args(["-I", &expr_path]); - command.arg(expr_path); + command.arg(eval_nix_path); let result = command .output() @@ -174,6 +179,7 @@ pub fn check_values( let stderr = String::from_utf8_lossy(&result.stderr).to_string(); return Ok(NixpkgsProblem::NixEval(NixEvalError { stderr }).into()); } + // Parse the resulting JSON value let attributes: Vec<(String, Attribute)> = serde_json::from_slice(&result.stdout) .with_context(|| { diff --git a/src/main.rs b/src/main.rs index 33111f9..d71057e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -272,12 +272,23 @@ mod tests { } fn test_nixpkgs(name: &str, path: &Path, expected_errors: &str) -> anyhow::Result<()> { + // Match the expected errors almost verbatim -- `@REDACTED@` turns into `.*`. + let pattern = format!( + "^{}$", + regex::escape(expected_errors).replace("@REDACTED@", ".*") + ); + + let expected_errors_regex = regex::RegexBuilder::new(&pattern) + .dot_matches_new_line(true) + .build()?; + let base_path = path.join("base"); let base_nixpkgs = if base_path.exists() { base_path.as_path() } 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()?; @@ -297,21 +308,7 @@ mod tests { }, )?; - 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")?; - - // We end up with a small Nix trace that includes the absolute path to src/eval.nix - // 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"); - - let pattern = format!( - "^{}$", - regex::escape(expected_errors).replace("@REDACTED@", ".*") - ); - - let expected_errors_regex = regex::RegexBuilder::new(&pattern) - .dot_matches_new_line(true) - .build()?; + let actual_errors = String::from_utf8_lossy(&writer); if !expected_errors_regex.is_match(&actual_errors) { panic!(