tests.nixpkgs-check-by-name: Intermediate NonDerivation error

This commit is contained in:
Silvan Mosberger 2023-10-19 23:19:13 +02:00
parent 4897b63ae6
commit 9a0ef88623
4 changed files with 43 additions and 40 deletions

View file

@ -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,

View file

@ -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 {}",

View file

@ -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)

View file

@ -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 {}",