From a1db0cdf9bb5e90727eb1c455da448fb21bab91e Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 9 Jan 2024 19:39:50 +0100 Subject: [PATCH] tests.nixpkgs-check-by-name: .context -> .with_context Avoids allocation in the non-error case --- pkgs/test/nixpkgs-check-by-name/src/eval.rs | 26 ++++++++++------- pkgs/test/nixpkgs-check-by-name/src/main.rs | 12 ++++---- .../nixpkgs-check-by-name/src/references.rs | 29 ++++++++++--------- pkgs/test/nixpkgs-check-by-name/src/utils.rs | 4 +-- 4 files changed, 40 insertions(+), 31 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index 7680af185bdb..65f71ccafc6f 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -60,20 +60,22 @@ pub fn check_values( ) -> 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().context("Failed to create a temporary file")?; + let attrs_file = NamedTempFile::new().with_context(|| "Failed to create a temporary file")?; // We need to canonicalise this path because if it's a symlink (which can be the case on // Darwin), Nix would need to read both the symlink and the target path, therefore need 2 // NIX_PATH entries for restrict-eval. But if we resolve the symlinks then only one predictable // entry is needed. let attrs_file_path = attrs_file.path().canonicalize()?; - serde_json::to_writer(&attrs_file, &package_names).context(format!( - "Failed to serialise the package names to the temporary path {}", - attrs_file_path.display() - ))?; + serde_json::to_writer(&attrs_file, &package_names).with_context(|| { + format!( + "Failed to serialise the package names to the temporary path {}", + attrs_file_path.display() + ) + })?; let expr_path = std::env::var("NIX_CHECK_BY_NAME_EXPR_PATH") - .context("Could not get environment variable NIX_CHECK_BY_NAME_EXPR_PATH")?; + .with_context(|| "Could not get environment variable NIX_CHECK_BY_NAME_EXPR_PATH")?; // With restrict-eval, only paths in NIX_PATH can be accessed, so we explicitly specify the // ones needed needed let mut command = process::Command::new("nix-instantiate"); @@ -112,17 +114,19 @@ pub fn check_values( let result = command .output() - .context(format!("Failed to run command {command:?}"))?; + .with_context(|| format!("Failed to run command {command:?}"))?; if !result.status.success() { anyhow::bail!("Failed to run command {command:?}"); } // Parse the resulting JSON value let attributes: Vec<(String, ByNameAttribute)> = serde_json::from_slice(&result.stdout) - .context(format!( - "Failed to deserialise {}", - String::from_utf8_lossy(&result.stdout) - ))?; + .with_context(|| { + format!( + "Failed to deserialise {}", + String::from_utf8_lossy(&result.stdout) + ) + })?; let check_result = validation::sequence(attributes.into_iter().map( |(attribute_name, attribute_value)| { diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index 6e78f6f5ae2a..d7627acb5fee 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -117,10 +117,12 @@ pub fn check_nixpkgs( error_writer: &mut W, ) -> validation::Result { Ok({ - let nixpkgs_path = nixpkgs_path.canonicalize().context(format!( - "Nixpkgs path {} could not be resolved", - nixpkgs_path.display() - ))?; + let nixpkgs_path = nixpkgs_path.canonicalize().with_context(|| { + format!( + "Nixpkgs path {} could not be resolved", + nixpkgs_path.display() + ) + })?; if !nixpkgs_path.join(utils::BASE_SUBPATH).exists() { writeln!( @@ -237,7 +239,7 @@ mod tests { let writer = temp_env::with_var("NO_COLOR", Some("1"), || -> anyhow::Result<_> { let mut writer = vec![]; process(base_nixpkgs, &path, &[&extra_nix_path], &mut writer) - .context(format!("Failed test case {name}"))?; + .with_context(|| format!("Failed test case {name}"))?; Ok(writer) })?; diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index 0561a9b22e85..3b3b05419780 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -17,10 +17,12 @@ pub fn check_references( ) -> validation::Result<()> { // The empty argument here is the subpath under the package directory to check // An empty one means the package directory itself - check_path(relative_package_dir, absolute_package_dir, Path::new("")).context(format!( - "While checking the references in package directory {}", - relative_package_dir.display() - )) + check_path(relative_package_dir, absolute_package_dir, Path::new("")).with_context(|| { + format!( + "While checking the references in package directory {}", + relative_package_dir.display() + ) + }) } /// Checks for a specific path to not have references outside @@ -62,7 +64,9 @@ fn check_path( .map(|entry| { let entry_subpath = subpath.join(entry.file_name()); check_path(relative_package_dir, absolute_package_dir, &entry_subpath) - .context(format!("Error while recursing into {}", subpath.display())) + .with_context(|| { + format!("Error while recursing into {}", subpath.display()) + }) }) .collect_vec()?, ) @@ -70,8 +74,8 @@ fn check_path( // Only check Nix files if let Some(ext) = path.extension() { if ext == OsStr::new("nix") { - check_nix_file(relative_package_dir, absolute_package_dir, subpath).context( - format!("Error while checking Nix file {}", subpath.display()), + check_nix_file(relative_package_dir, absolute_package_dir, subpath).with_context( + || format!("Error while checking Nix file {}", subpath.display()), )? } else { Success(()) @@ -93,13 +97,12 @@ fn check_nix_file( subpath: &Path, ) -> validation::Result<()> { let path = absolute_package_dir.join(subpath); - let parent_dir = path.parent().context(format!( - "Could not get parent of path {}", - subpath.display() - ))?; + let parent_dir = path + .parent() + .with_context(|| format!("Could not get parent of path {}", subpath.display()))?; - let contents = - read_to_string(&path).context(format!("Could not read file {}", subpath.display()))?; + let contents = read_to_string(&path) + .with_context(|| format!("Could not read file {}", subpath.display()))?; let root = Root::parse(&contents); if let Some(error) = root.errors().first() { diff --git a/pkgs/test/nixpkgs-check-by-name/src/utils.rs b/pkgs/test/nixpkgs-check-by-name/src/utils.rs index 5cc4a0863ba8..7e0198dede42 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/utils.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/utils.rs @@ -10,10 +10,10 @@ pub const PACKAGE_NIX_FILENAME: &str = "package.nix"; pub fn read_dir_sorted(base_dir: &Path) -> anyhow::Result> { let listing = base_dir .read_dir() - .context(format!("Could not list directory {}", base_dir.display()))?; + .with_context(|| format!("Could not list directory {}", base_dir.display()))?; let mut shard_entries = listing .collect::>>() - .context(format!("Could not list directory {}", base_dir.display()))?; + .with_context(|| format!("Could not list directory {}", base_dir.display()))?; shard_entries.sort_by_key(|entry| entry.file_name()); Ok(shard_entries) }