diff options
author | Silvan Mosberger <silvan.mosberger@tweag.io> | 2023-10-19 23:19:13 +0200 |
---|---|---|
committer | Silvan Mosberger <silvan.mosberger@tweag.io> | 2023-10-24 01:15:57 +0200 |
commit | 9a0ef886236eefb13ea84422c5c8ba7865ccf6f8 (patch) | |
tree | d6f34aa4da3702d49022e2f6d0042e8e2bb7d3ae /pkgs/test | |
parent | 4897b63ae67d6f56a0d9a7f0db421467ebe8828b (diff) |
tests.nixpkgs-check-by-name: Intermediate NonDerivation error
Diffstat (limited to 'pkgs/test')
-rw-r--r-- | pkgs/test/nixpkgs-check-by-name/src/check_result.rs | 10 | ||||
-rw-r--r-- | pkgs/test/nixpkgs-check-by-name/src/eval.rs | 17 | ||||
-rw-r--r-- | pkgs/test/nixpkgs-check-by-name/src/main.rs | 3 | ||||
-rw-r--r-- | pkgs/test/nixpkgs-check-by-name/src/references.rs | 53 |
4 files changed, 43 insertions, 40 deletions
diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index 32d43edc153a1..4f1ee9f674f62 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -6,6 +6,10 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + NonDerivation { + relative_package_file: PathBuf, + package_name: String, + }, OutsideSymlink { relative_package_dir: PathBuf, subpath: PathBuf, @@ -56,6 +60,12 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::NonDerivation { relative_package_file, package_name } => + write!( + f, + "pkgs.{package_name}: This attribute defined by {} is not a derivation", + relative_package_file.display() + ), CheckError::OutsideSymlink { relative_package_dir, subpath } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index 26836501c97ad..695d52c0a8f29 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -1,3 +1,4 @@ +use crate::check_result::{pass, write_check_result, CheckError}; use crate::structure; use crate::utils::ErrorWriter; use crate::Version; @@ -144,12 +145,16 @@ pub fn check_values<W: io::Write>( continue; } - if !attribute_info.is_derivation { - error_writer.write(&format!( - "pkgs.{package_name}: This attribute defined by {} is not a derivation", - relative_package_file.display() - ))?; - } + let check_result = if !attribute_info.is_derivation { + CheckError::NonDerivation { + relative_package_file: relative_package_file.clone(), + package_name: package_name.clone(), + } + .into_result() + } else { + pass(()) + }; + write_check_result(error_writer, check_result)?; } else { error_writer.write(&format!( "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}", diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index ee98c5a1a893e..80ed29646f725 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -5,6 +5,7 @@ mod structure; mod utils; use anyhow::Context; +use check_result::write_check_result; use clap::{Parser, ValueEnum}; use colored::Colorize; use std::io; @@ -92,7 +93,7 @@ pub fn check_nixpkgs<W: io::Write>( if error_writer.empty { // Only if we could successfully parse the structure, we do the semantic checks eval::check_values(version, &mut error_writer, &nixpkgs, eval_accessible_paths)?; - references::check_references(&mut error_writer, &nixpkgs)?; + write_check_result(&mut error_writer, references::check_references(&nixpkgs))?; } } Ok(error_writer.empty) diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index f72074483b142..b932b556f54fe 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -1,20 +1,16 @@ -use crate::check_result::{ - flatten_check_results, pass, write_check_result, CheckError, CheckResult, -}; +use crate::check_result::{flatten_check_results, pass, CheckError, CheckResult}; use crate::structure::Nixpkgs; use crate::utils; -use crate::utils::{ErrorWriter, LineIndex}; +use crate::utils::LineIndex; use anyhow::Context; use rnix::{Root, SyntaxKind::NODE_PATH}; use std::ffi::OsStr; use std::fs::read_to_string; -use std::io; use std::path::{Path, PathBuf}; /// Small helper so we don't need to pass in the same arguments to all functions -struct PackageContext<'a, W: io::Write> { - error_writer: &'a mut ErrorWriter<W>, +struct PackageContext<'a> { /// The package directory relative to Nixpkgs, such as `pkgs/by-name/fo/foo` relative_package_dir: &'a PathBuf, /// The absolute package directory @@ -23,36 +19,32 @@ struct PackageContext<'a, W: io::Write> { /// Check that every package directory in pkgs/by-name doesn't link to outside that directory. /// Both symlinks and Nix path expressions are checked. -pub fn check_references<W: io::Write>( - error_writer: &mut ErrorWriter<W>, - nixpkgs: &Nixpkgs, -) -> anyhow::Result<()> { +pub fn check_references(nixpkgs: &Nixpkgs) -> CheckResult<()> { // Check the directories for each package separately - for package_name in &nixpkgs.package_names { + let check_results = nixpkgs.package_names.iter().map(|package_name| { let relative_package_dir = Nixpkgs::relative_dir_for_package(package_name); - let mut context = PackageContext { - error_writer, + let context = PackageContext { relative_package_dir: &relative_package_dir, absolute_package_dir: &nixpkgs.path.join(&relative_package_dir), }; // The empty argument here is the subpath under the package directory to check // An empty one means the package directory itself - check_path(&mut context, Path::new("")).context(format!( + check_path(&context, Path::new("")).context(format!( "While checking the references in package directory {}", relative_package_dir.display() - ))?; - } - Ok(()) + )) + }); + flatten_check_results(check_results, |_| ()) } /// Checks for a specific path to not have references outside -fn check_path<W: io::Write>(context: &mut PackageContext<W>, subpath: &Path) -> anyhow::Result<()> { +fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> { let path = context.absolute_package_dir.join(subpath); if path.is_symlink() { // Check whether the symlink resolves to outside the package directory - let check_result = match path.canonicalize() { + match path.canonicalize() { Ok(target) => { // No need to handle the case of it being inside the directory, since we scan through the // entire directory recursively anyways @@ -72,18 +64,18 @@ fn check_path<W: io::Write>(context: &mut PackageContext<W>, subpath: &Path) -> io_error, } .into_result(), - }; - write_check_result(context.error_writer, check_result)?; + } } else if path.is_dir() { // Recursively check each entry - for entry in utils::read_dir_sorted(&path)? { + let check_results = utils::read_dir_sorted(&path)?.into_iter().map(|entry| { let entry_subpath = subpath.join(entry.file_name()); check_path(context, &entry_subpath) - .context(format!("Error while recursing into {}", subpath.display()))? - } + .context(format!("Error while recursing into {}", subpath.display())) + }); + flatten_check_results(check_results, |_| ()) } else if path.is_file() { // Only check Nix files - let check_result = if let Some(ext) = path.extension() { + if let Some(ext) = path.extension() { if ext == OsStr::new("nix") { check_nix_file(context, subpath).context(format!( "Error while checking Nix file {}", @@ -94,21 +86,16 @@ fn check_path<W: io::Write>(context: &mut PackageContext<W>, subpath: &Path) -> } } else { pass(()) - }; - write_check_result(context.error_writer, check_result)?; + } } else { // This should never happen, git doesn't support other file types anyhow::bail!("Unsupported file type for path {}", subpath.display()); } - Ok(()) } /// Check whether a nix file contains path expression references pointing outside the package /// directory -fn check_nix_file<W: io::Write>( - context: &mut PackageContext<W>, - subpath: &Path, -) -> CheckResult<()> { +fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { let path = context.absolute_package_dir.join(subpath); let parent_dir = path.parent().context(format!( "Could not get parent of path {}", |