From 53b43ce0e322eb4fc8daaad8a5a597155d42379a Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 15 Dec 2023 00:47:34 +0100 Subject: [PATCH] tests.nixpkgs-check-by-name: Fix and document behavior without --base Previously, not passing `--base` would enforce the most strict checks. While there's currently no actual violation of these stricter checks, this does not match the previous behavior. This won't matter once CI passes `--base`, the code handling the optionality can be removed then. --- pkgs/test/nixpkgs-check-by-name/README.md | 9 +++++++-- pkgs/test/nixpkgs-check-by-name/src/main.rs | 16 +++++++++------- pkgs/test/nixpkgs-check-by-name/src/version.rs | 17 +++++++++++++++-- .../tests/override-empty-arg/base/default.nix | 1 + .../base/pkgs/by-name/README.md | 1 + 5 files changed, 33 insertions(+), 11 deletions(-) create mode 100644 pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/base/default.nix create mode 100644 pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/base/pkgs/by-name/README.md diff --git a/pkgs/test/nixpkgs-check-by-name/README.md b/pkgs/test/nixpkgs-check-by-name/README.md index c906eaffc974..8ed23204deca 100644 --- a/pkgs/test/nixpkgs-check-by-name/README.md +++ b/pkgs/test/nixpkgs-check-by-name/README.md @@ -10,8 +10,13 @@ This API may be changed over time if the CI workflow making use of it is adjuste - Command line: `nixpkgs-check-by-name [--base ] ` - Arguments: - - ``: The path to the Nixpkgs to check against - - ``: The path to the Nixpkgs to check + - ``: The path to the Nixpkgs to check. + - ``: The path to the Nixpkgs to use as the base to compare `` against. + This allows the strictness of checks to increase over time by only preventing _new_ violations from being introduced, + while allowing violations that already existed. + + If not specified, all violations of stricter checks are allowed. + However, this flag will become required once CI passes it. - Exit code: - `0`: If the [validation](#validity-checks) is successful - `1`: If the [validation](#validity-checks) is not successful diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index bf3bfb193f18..91e1992a52c9 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -75,14 +75,16 @@ pub fn process( let main_result = check_nixpkgs(main_nixpkgs, eval_accessible_paths)?; let check_result = main_result.result_map(|nixpkgs_version| { if let Some(base) = base_nixpkgs { - check_nixpkgs(base, eval_accessible_paths)?.result_map(|base_nixpkgs_version| { - Ok(Nixpkgs::compare(base_nixpkgs_version, nixpkgs_version)) - }) + check_nixpkgs(base, eval_accessible_paths, error_writer)?.result_map( + |base_nixpkgs_version| { + Ok(Nixpkgs::compare( + Some(base_nixpkgs_version), + nixpkgs_version, + )) + }, + ) } else { - Ok(Nixpkgs::compare( - version::Nixpkgs::default(), - nixpkgs_version, - )) + Ok(Nixpkgs::compare(None, nixpkgs_version)) } })?; diff --git a/pkgs/test/nixpkgs-check-by-name/src/version.rs b/pkgs/test/nixpkgs-check-by-name/src/version.rs index 7f83bdf3ff67..c5cee95e0d53 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/version.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/version.rs @@ -16,12 +16,25 @@ 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(from: Self, to: Self) -> Validation<()> { + pub fn compare(optional_from: Option, 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, from.attributes.get(&name), &attr_to) + let attr_from = if let Some(from) = &optional_from { + from.attributes.get(&name) + } else { + // This pretends that if there's no base version to compare against, all + // attributes existed without conforming to the new strictness check for + // backwards compatibility. + // TODO: Remove this case. This is only needed because the `--base` + // argument is still optional, which doesn't need to be once CI is updated + // to pass it. + Some(&Attribute { + empty_non_auto_called: EmptyNonAutoCalled::Invalid, + }) + }; + Attribute::compare(&name, attr_from, &attr_to) }), ) } diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/base/default.nix b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/base/default.nix new file mode 100644 index 000000000000..2875ea6327ef --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/base/default.nix @@ -0,0 +1 @@ +import ../../mock-nixpkgs.nix { root = ./.; } diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/base/pkgs/by-name/README.md b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/base/pkgs/by-name/README.md new file mode 100644 index 000000000000..b0d2b34e338a --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/base/pkgs/by-name/README.md @@ -0,0 +1 @@ +(this is just here so the directory can get tracked by git)