Skip to content

Commit

Permalink
eval: use a work directory instead of an environment variable to cont…
Browse files Browse the repository at this point in the history
…rol 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`.
  • Loading branch information
philiptaron committed Jul 20, 2024
1 parent 6f7a610 commit 4784848
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 43 deletions.
3 changes: 0 additions & 3 deletions default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ let
};
inherit (pkgs) lib;

runtimeExprPath = ./src/eval.nix;
testNixpkgsPath = ./tests/mock-nixpkgs.nix;
nixpkgsLibPath = nixpkgs + "/lib";

Expand Down Expand Up @@ -53,15 +52,13 @@ let
inherit
nixpkgsLibPath
initNix
runtimeExprPath
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}";
Expand Down
3 changes: 0 additions & 3 deletions package.nix
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

nixpkgsLibPath,
initNix,
runtimeExprPath,
testNixpkgsPath,
version,
}:
Expand All @@ -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 = ''
Expand All @@ -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}
'';
}
50 changes: 28 additions & 22 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -106,26 +109,29 @@ pub fn check_values(
package_names: Vec<String>,
is_test: 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.
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")?;

Expand All @@ -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")
Expand All @@ -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()
Expand All @@ -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(|| {
Expand Down
27 changes: 12 additions & 15 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand All @@ -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!(
Expand Down

0 comments on commit 4784848

Please sign in to comment.