diff options
author | Silvan Mosberger <silvan.mosberger@tweag.io> | 2023-10-12 01:20:56 +0200 |
---|---|---|
committer | Silvan Mosberger <silvan.mosberger@tweag.io> | 2023-10-12 02:28:34 +0200 |
commit | d3bf6133e826a6901c50d8524cb3fbbce038df28 (patch) | |
tree | a75c6fa38ef7bc0b2b42b4bdfdd83016fdb91e2c /pkgs/test | |
parent | 23541bed72c8385acddfc8e7ba0bd15d5a2c26af (diff) |
tests.nixpkgs-check-by-name: Disallow empty all-packages.nix overrides
Only enabled with `--version v1`
Diffstat (limited to 'pkgs/test')
11 files changed, 39 insertions, 13 deletions
diff --git a/pkgs/test/nixpkgs-check-by-name/README.md b/pkgs/test/nixpkgs-check-by-name/README.md index ebd1cd40aba08..2d2db9a58bc5f 100644 --- a/pkgs/test/nixpkgs-check-by-name/README.md +++ b/pkgs/test/nixpkgs-check-by-name/README.md @@ -15,6 +15,7 @@ This API may be changed over time if the CI workflow making use of it is adjuste Possible values: - `v0` (default) + - `v1` See [validation](#validity-checks) for the differences. - Exit code: @@ -41,6 +42,7 @@ These checks are performed by this tool: ### Nix evaluation checks - `pkgs.${name}` is defined as `callPackage pkgs/by-name/${shard}/${name}/package.nix args` for some `args`. + - **Only after --version v1**: If `pkgs.${name}` is not auto-called from `pkgs/by-name`, `args` must not be empty - `pkgs.lib.isDerivation pkgs.${name}` is `true`. ## Development diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.nix b/pkgs/test/nixpkgs-check-by-name/src/eval.nix index 2fa0c5a9709c1..bf9f19d8e460b 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.nix +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.nix @@ -25,6 +25,8 @@ let toString fn else null; + CallPackage.empty_arg = + args == { }; }; in if builtins.isAttrs result then diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index b1c1a44f8c080..26836501c97ad 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -1,5 +1,6 @@ use crate::structure; use crate::utils::ErrorWriter; +use crate::Version; use std::path::Path; use anyhow::Context; @@ -22,10 +23,14 @@ enum AttributeVariant { /// The attribute is auto-called as pkgs.callPackage using pkgs/by-name, /// and it is not overridden by a definition in all-packages.nix AutoCalled, - /// The attribute is defined as a pkgs.callPackage <path>, + /// The attribute is defined as a pkgs.callPackage <path> <args>, /// and overridden by all-packages.nix - /// The path is None when the <path> argument isn't a path - CallPackage { path: Option<PathBuf> }, + CallPackage { + /// The <path> argument or None if it's not a path + path: Option<PathBuf>, + /// true if <args> is { } + empty_arg: bool, + }, /// The attribute is not defined as pkgs.callPackage Other, } @@ -36,6 +41,7 @@ const EXPR: &str = include_str!("eval.nix"); /// of the form `callPackage <package_file> { ... }`. /// See the `eval.nix` file for how this is achieved on the Nix side pub fn check_values<W: io::Write>( + version: Version, error_writer: &mut ErrorWriter<W>, nixpkgs: &structure::Nixpkgs, eval_accessible_paths: Vec<&Path>, @@ -112,19 +118,27 @@ pub fn check_values<W: io::Write>( if let Some(attribute_info) = actual_files.get(package_name) { let valid = match &attribute_info.variant { AttributeVariant::AutoCalled => true, - AttributeVariant::CallPackage { path } => { - if let Some(call_package_path) = path { + 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 + }; + correct_file && non_empty } AttributeVariant::Other => false, }; if !valid { error_writer.write(&format!( - "pkgs.{package_name}: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage {} {{ ... }}`.", + "pkgs.{package_name}: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage {} {{ ... }}` with a non-empty second argument.", relative_package_file.display() ))?; continue; diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index 5c51f32b5cec9..5d28077ae4aec 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -30,6 +30,8 @@ pub struct Args { pub enum Version { /// Initial version V0, + /// Empty argument check + V1, } fn main() -> ExitCode { @@ -65,7 +67,7 @@ fn main() -> ExitCode { /// - `Ok(true)` if the structure is valid, nothing will have been written to `error_writer`. pub fn check_nixpkgs<W: io::Write>( nixpkgs_path: &Path, - _version: Version, + version: Version, eval_accessible_paths: Vec<&Path>, error_writer: &mut W, ) -> anyhow::Result<bool> { @@ -88,7 +90,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(&mut error_writer, &nixpkgs, eval_accessible_paths)?; + eval::check_values(version, &mut error_writer, &nixpkgs, eval_accessible_paths)?; references::check_references(&mut error_writer, &nixpkgs)?; } } @@ -188,7 +190,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::V0, vec![&extra_nix_path], &mut writer) + check_nixpkgs(&path, Version::V1, vec![&extra_nix_path], &mut writer) .context(format!("Failed test case {name}"))?; Ok(writer) })?; diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected b/pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected index 36fa40d18ead1..51479e22d26fe 100644 --- a/pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected @@ -1 +1 @@ -pkgs.nonDerivation: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage pkgs/by-name/no/nonDerivation/package.nix { ... }`. +pkgs.nonDerivation: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage pkgs/by-name/no/nonDerivation/package.nix { ... }` with a non-empty second argument. diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/all-packages.nix b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/all-packages.nix new file mode 100644 index 0000000000000..d369dd7228dca --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/all-packages.nix @@ -0,0 +1,3 @@ +self: super: { + nonDerivation = self.callPackage ./pkgs/by-name/no/nonDerivation/package.nix { }; +} diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/default.nix b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/default.nix new file mode 100644 index 0000000000000..af25d1450122b --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/default.nix @@ -0,0 +1 @@ +import ../mock-nixpkgs.nix { root = ./.; } diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected new file mode 100644 index 0000000000000..51479e22d26fe --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected @@ -0,0 +1 @@ +pkgs.nonDerivation: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage pkgs/by-name/no/nonDerivation/package.nix { ... }` with a non-empty second argument. diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/pkgs/by-name/no/nonDerivation/package.nix b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/pkgs/by-name/no/nonDerivation/package.nix new file mode 100644 index 0000000000000..a1b92efbbadb9 --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/pkgs/by-name/no/nonDerivation/package.nix @@ -0,0 +1 @@ +{ someDrv }: someDrv diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected b/pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected index 36fa40d18ead1..51479e22d26fe 100644 --- a/pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected @@ -1 +1 @@ -pkgs.nonDerivation: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage pkgs/by-name/no/nonDerivation/package.nix { ... }`. +pkgs.nonDerivation: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage pkgs/by-name/no/nonDerivation/package.nix { ... }` with a non-empty second argument. diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected b/pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected index 36fa40d18ead1..51479e22d26fe 100644 --- a/pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected @@ -1 +1 @@ -pkgs.nonDerivation: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage pkgs/by-name/no/nonDerivation/package.nix { ... }`. +pkgs.nonDerivation: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage pkgs/by-name/no/nonDerivation/package.nix { ... }` with a non-empty second argument. |