gador/nixpkgs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

tests.nixpkgs-check-by-name: Intermediate refactor

Browse Source 
This prepares the code base for the removal of the `--version` flag, to
be replaced with a flag that can specify a base version to compare the
main Nixpkgs against, in order to have gradual transitions to stricter
checks.

This refactoring does:
- Introduce the `version` module that can house the logic to increase
  strictness, with a `version::Nixpkgs` struct that contains the
  strictness conformity of a single Nixpkgs version
- Make the check return `version::Nixpkgs`
- Handle the behavior of the still-existing `--version` flag with `version::Nixpkgs`
- Introduce an intermediate `process` function to handle the top-level
  logic, especially useful in the next commit
This commit is contained in:
Silvan Mosberger
2023-12-14 03:40:49 +01:00
parent e98d22851b
commit a6ba4cae31
3 changed files with 139 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkgs/test/nixpkgs-check-by-name/src/eval.rs
+25 -19
View File
@@ -1,7 +1,7 @@
use crate::nixpkgs_problem::NixpkgsProblem;
use crate::structure;
use crate::validation::{self, Validation::Success};
use crate::Version;
use crate::version;
use std::path::Path;
use anyhow::Context;
@@ -39,11 +39,10 @@ enum AttributeVariant {
/// of the form `callPackage <package_file> { ... }`.
/// See the `eval.nix` file for how this is achieved on the Nix side
pub fn check_values(
version: Version,
nixpkgs_path: &Path,
package_names: Vec<String>,
eval_accessible_paths: Vec<&Path>,
) -> validation::Result<()> {
) -> validation::Result<version::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().context("Failed to create a temporary file")?;
@@ -110,8 +109,8 @@ pub fn check_values(
String::from_utf8_lossy(&result.stdout)
))?;
Ok(validation::sequence_(package_names.iter().map(
|package_name| {
Ok(
validation::sequence(package_names.iter().map(|package_name| {
let relative_package_file = structure::relative_file_for_package(package_name);
let absolute_package_file = nixpkgs_path.join(&relative_package_file);
@@ -126,23 +125,25 @@ pub fn check_values(
Success(())
};
check_result.and(match &attribute_info.variant {
AttributeVariant::AutoCalled => Success(()),
let check_result = check_result.and(match &attribute_info.variant {
AttributeVariant::AutoCalled => Success(version::Attribute {
empty_non_auto_called: version::EmptyNonAutoCalled::Valid,
}),
AttributeVariant::CallPackage { path, empty_arg } => {
let correct_file = if let Some(call_package_path) = path {
absolute_package_file == *call_package_path
} else {
false
};
// Only check for the argument to be non-empty if the version is V1 or
// higher
let non_empty = if version >= Version::V1 {
!empty_arg
} else {
true
};
if correct_file && non_empty {
Success(())
if correct_file {
Success(version::Attribute {
empty_non_auto_called: if *empty_arg {
version::EmptyNonAutoCalled::Invalid
} else {
version::EmptyNonAutoCalled::Valid
},
})
} else {
NixpkgsProblem::WrongCallPackage {
relative_package_file: relative_package_file.clone(),
@@ -156,7 +157,9 @@ pub fn check_values(
package_name: package_name.clone(),
}
.into(),
})
});
check_result.map(|value| (package_name.clone(), value))
} else {
NixpkgsProblem::UndefinedAttr {
relative_package_file: relative_package_file.clone(),
@@ -164,6 +167,9 @@ pub fn check_values(
}
.into()
}
},
)))
}))
.map(|elems| version::Nixpkgs {
attributes: elems.into_iter().collect(),
}),
)
}
pkgs/test/nixpkgs-check-by-name/src/main.rs
+43 -21
View File
@@ -4,6 +4,7 @@ mod references;
mod structure;
mod utils;
mod validation;
mod version;
use crate::structure::check_structure;
use crate::validation::Validation::Failure;
@@ -39,7 +40,7 @@ pub enum Version {
fn main() -> ExitCode {
let args = Args::parse();
match check_nixpkgs(&args.nixpkgs, args.version, vec![], &mut io::stderr()) {
match process(&args.nixpkgs, args.version, vec![], &mut io::stderr()) {
Ok(true) => {
eprintln!("{}", "Validated successfully".green());
ExitCode::SUCCESS
@@ -55,7 +56,7 @@ fn main() -> ExitCode {
}
}
/// Checks whether the pkgs/by-name structure in Nixpkgs is valid.
/// Does the actual work. This is the abstraction used both by `main` and the tests.
///
/// # Arguments
/// - `nixpkgs_path`: The path to the Nixpkgs to check
@@ -68,28 +69,23 @@ fn main() -> ExitCode {
/// - `Err(e)` if an I/O-related error `e` occurred.
/// - `Ok(false)` if there are problems, all of which will be written to `error_writer`.
/// - `Ok(true)` if there are no problems
pub fn check_nixpkgs<W: io::Write>(
pub fn process<W: io::Write>(
nixpkgs_path: &Path,
version: Version,
eval_accessible_paths: Vec<&Path>,
error_writer: &mut W,
) -> anyhow::Result<bool> {
let nixpkgs_path = nixpkgs_path.canonicalize().context(format!(
"Nixpkgs path {} could not be resolved",
nixpkgs_path.display()
))?;
let check_result = if !nixpkgs_path.join(utils::BASE_SUBPATH).exists() {
eprintln!(
"Given Nixpkgs path does not contain a {} subdirectory, no check necessary.",
utils::BASE_SUBPATH
);
Success(())
} else {
check_structure(&nixpkgs_path)?.result_map(|package_names|
// Only if we could successfully parse the structure, we do the evaluation checks
eval::check_values(version, &nixpkgs_path, package_names, eval_accessible_paths))?
};
let nixpkgs_result = check_nixpkgs(nixpkgs_path, eval_accessible_paths)?;
let check_result = nixpkgs_result.result_map(|nixpkgs_version| {
let empty_non_auto_called_base = match version {
Version::V0 => version::EmptyNonAutoCalled::Invalid,
Version::V1 => version::EmptyNonAutoCalled::Valid,
};
Ok(version::Nixpkgs::compare(
&empty_non_auto_called_base,
nixpkgs_version,
))
})?;
match check_result {
Failure(errors) => {
@@ -102,9 +98,35 @@ pub fn check_nixpkgs<W: io::Write>(
}
}
/// Checks whether the pkgs/by-name structure in Nixpkgs is valid,
/// and returns to which degree it's valid for checks with increased strictness.
pub fn check_nixpkgs(
nixpkgs_path: &Path,
eval_accessible_paths: Vec<&Path>,
) -> validation::Result<version::Nixpkgs> {
Ok({
let nixpkgs_path = nixpkgs_path.canonicalize().context(format!(
"Nixpkgs path {} could not be resolved",
nixpkgs_path.display()
))?;
if !nixpkgs_path.join(utils::BASE_SUBPATH).exists() {
eprintln!(
"Given Nixpkgs path does not contain a {} subdirectory, no check necessary.",
utils::BASE_SUBPATH
);
Success(version::Nixpkgs::default())
} else {
check_structure(&nixpkgs_path)?.result_map(|package_names|
// Only if we could successfully parse the structure, we do the evaluation checks
eval::check_values(&nixpkgs_path, package_names, eval_accessible_paths))?
}
})
}
#[cfg(test)]
mod tests {
use crate::check_nixpkgs;
use crate::process;
use crate::utils;
use crate::Version;
use anyhow::Context;
@@ -195,7 +217,7 @@ mod tests {
// We don't want coloring to mess up the tests
let writer = temp_env::with_var("NO_COLOR", Some("1"), || -> anyhow::Result<_> {
let mut writer = vec![];
check_nixpkgs(&path, Version::V1, vec![&extra_nix_path], &mut writer)
process(&path, Version::V1, vec![&extra_nix_path], &mut writer)
.context(format!("Failed test case {name}"))?;
Ok(writer)
})?;
pkgs/test/nixpkgs-check-by-name/src/version.rs
+71
View File
@@ -0,0 +1,71 @@
use crate::nixpkgs_problem::NixpkgsProblem;
use crate::structure;
use crate::validation::{self, Validation, Validation::Success};
use std::collections::HashMap;
/// The check version conformity of a Nixpkgs path:
/// When the strictness of the check increases, this structure should be extended to distinguish
/// between parts that are still valid, and ones that aren't valid anymore.
#[derive(Default)]
pub struct Nixpkgs {
/// The package attributes tracked in `pkgs/by-name`
pub attributes: HashMap<String, Attribute>,
}
impl Nixpkgs {
/// Compares two Nixpkgs versions against each other, returning validation errors only if the
/// `from` version satisfied the stricter checks, while the `to` version doesn't satisfy them
/// anymore.
pub fn compare(empty_non_auto_called_from: &EmptyNonAutoCalled, to: Self) -> Validation<()> {
validation::sequence_(
// We only loop over the current attributes,
// we don't need to check ones that were removed
to.attributes.into_iter().map(|(name, attr_to)| {
Attribute::compare(&name, empty_non_auto_called_from, &attr_to)
}),
)
}
}
/// The check version conformity of an attribute defined by `pkgs/by-name`
pub struct Attribute {
pub empty_non_auto_called: EmptyNonAutoCalled,
}
impl Attribute {
pub fn compare(
name: &str,
empty_non_auto_called_from: &EmptyNonAutoCalled,
to: &Self,
) -> Validation<()> {
EmptyNonAutoCalled::compare(name, empty_non_auto_called_from, &to.empty_non_auto_called)
}
}
/// Whether an attribute conforms to the new strictness check that
/// `callPackage ... {}` is not allowed anymore in `all-package.nix`
#[derive(PartialEq, PartialOrd)]
pub enum EmptyNonAutoCalled {
/// The attribute is not valid anymore with the new check
Invalid,
/// The attribute is still valid with the new check
Valid,
}
impl EmptyNonAutoCalled {
fn compare(
name: &str,
empty_non_auto_called_from: &EmptyNonAutoCalled,
to: &Self,
) -> Validation<()> {
if to >= empty_non_auto_called_from {
Success(())
} else {
NixpkgsProblem::WrongCallPackage {
relative_package_file: structure::relative_file_for_package(name),
package_name: name.to_owned(),
}
.into()
}
}
}