diff options
author | github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> | 2023-10-13 00:02:24 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-10-13 00:02:24 +0000 |
commit | 58485aaddb4a6531ac7790bf3aefd7a9d22ae1a9 (patch) | |
tree | d8c96a8143cb487631e456177a5caf63fb1df395 /pkgs/test | |
parent | 75db88ef460b461a806925106562c7d0c0d859cf (diff) | |
parent | c6770338187931189435b0da8f68d4a4a36ce8e1 (diff) |
Merge master into staging-next
Diffstat (limited to 'pkgs/test')
15 files changed, 136 insertions, 32 deletions
diff --git a/pkgs/test/nixpkgs-check-by-name/README.md b/pkgs/test/nixpkgs-check-by-name/README.md index 4dd694acd2f0a..2d2db9a58bc5f 100644 --- a/pkgs/test/nixpkgs-check-by-name/README.md +++ b/pkgs/test/nixpkgs-check-by-name/README.md @@ -11,6 +11,13 @@ This API may be changed over time if the CI workflow making use of it is adjuste - Command line: `nixpkgs-check-by-name <NIXPKGS>` - Arguments: - `<NIXPKGS>`: The path to the Nixpkgs to check + - `--version <VERSION>`: The version of the checks to perform. + + Possible values: + - `v0` (default) + - `v1` + + See [validation](#validity-checks) for the differences. - Exit code: - `0`: If the [validation](#validity-checks) is successful - `1`: If the [validation](#validity-checks) is not successful @@ -35,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 7c0ae755215a0..bf9f19d8e460b 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.nix +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.nix @@ -18,19 +18,42 @@ let callPackage = fn: args: let result = super.callPackage fn args; + variantInfo._attributeVariant = { + # These names are used by the deserializer on the Rust side + CallPackage.path = + if builtins.isPath fn then + toString fn + else + null; + CallPackage.empty_arg = + args == { }; + }; in if builtins.isAttrs result then # If this was the last overlay to be applied, we could just only return the `_callPackagePath`, # but that's not the case because stdenv has another overlays on top of user-provided ones. # So to not break the stdenv build we need to return the mostly proper result here - result // { - _callPackagePath = fn; - } + result // variantInfo else # It's very rare that callPackage doesn't return an attribute set, but it can occur. - { - _callPackagePath = fn; + variantInfo; + + _internalCallByNamePackageFile = file: + let + result = super._internalCallByNamePackageFile file; + variantInfo._attributeVariant = { + # This name is used by the deserializer on the Rust side + AutoCalled = null; }; + in + if builtins.isAttrs result then + # If this was the last overlay to be applied, we could just only return the `_callPackagePath`, + # but that's not the case because stdenv has another overlays on top of user-provided ones. + # So to not break the stdenv build we need to return the mostly proper result here + result // variantInfo + else + # It's very rare that callPackage doesn't return an attribute set, but it can occur. + variantInfo; }; pkgs = import nixpkgsPath { @@ -39,14 +62,14 @@ let overlays = [ callPackageOverlay ]; }; - attrInfo = attr: { + attrInfo = attr: + let + value = pkgs.${attr}; + in + { # These names are used by the deserializer on the Rust side - call_package_path = - if pkgs.${attr} ? _callPackagePath && builtins.isPath pkgs.${attr}._callPackagePath then - toString pkgs.${attr}._callPackagePath - else - null; - is_derivation = pkgs.lib.isDerivation pkgs.${attr}; + variant = value._attributeVariant or { Other = null; }; + is_derivation = pkgs.lib.isDerivation value; }; attrInfos = builtins.listToAttrs (map (name: { diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index 17e22495b22ad..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; @@ -13,16 +14,34 @@ use tempfile::NamedTempFile; /// Attribute set of this structure is returned by eval.nix #[derive(Deserialize)] struct AttributeInfo { - call_package_path: Option<PathBuf>, + variant: AttributeVariant, is_derivation: bool, } +#[derive(Deserialize)] +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> <args>, + /// and overridden by all-packages.nix + 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, +} + const EXPR: &str = include_str!("eval.nix"); /// Check that the Nixpkgs attribute values corresponding to the packages in pkgs/by-name are /// 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>, @@ -97,16 +116,29 @@ pub fn check_values<W: io::Write>( let absolute_package_file = nixpkgs.path.join(&relative_package_file); if let Some(attribute_info) = actual_files.get(package_name) { - let is_expected_file = - if let Some(call_package_path) = &attribute_info.call_package_path { - absolute_package_file == *call_package_path - } else { - false - }; + let valid = match &attribute_info.variant { + AttributeVariant::AutoCalled => true, + 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 !is_expected_file { + if !valid { error_writer.write(&format!( - "pkgs.{package_name}: This attribute is not defined as `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 8c663061bb097..5d28077ae4aec 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -4,7 +4,7 @@ mod structure; mod utils; use anyhow::Context; -use clap::Parser; +use clap::{Parser, ValueEnum}; use colored::Colorize; use std::io; use std::path::{Path, PathBuf}; @@ -15,14 +15,28 @@ use utils::ErrorWriter; /// Program to check the validity of pkgs/by-name #[derive(Parser, Debug)] #[command(about)] -struct Args { +pub struct Args { /// Path to nixpkgs nixpkgs: PathBuf, + /// The version of the checks + /// Increasing this may cause failures for a Nixpkgs that succeeded before + /// TODO: Remove default once Nixpkgs CI passes this argument + #[arg(long, value_enum, default_value_t = Version::V0)] + version: Version, +} + +/// The version of the checks +#[derive(Debug, Clone, PartialEq, PartialOrd, ValueEnum)] +pub enum Version { + /// Initial version + V0, + /// Empty argument check + V1, } fn main() -> ExitCode { let args = Args::parse(); - match check_nixpkgs(&args.nixpkgs, vec![], &mut io::stderr()) { + match check_nixpkgs(&args.nixpkgs, args.version, vec![], &mut io::stderr()) { Ok(true) => { eprintln!("{}", "Validated successfully".green()); ExitCode::SUCCESS @@ -53,6 +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, eval_accessible_paths: Vec<&Path>, error_writer: &mut W, ) -> anyhow::Result<bool> { @@ -75,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)?; } } @@ -86,6 +101,7 @@ pub fn check_nixpkgs<W: io::Write>( mod tests { use crate::check_nixpkgs; use crate::structure; + use crate::Version; use anyhow::Context; use std::fs; use std::path::Path; @@ -174,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, 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/mock-nixpkgs.nix b/pkgs/test/nixpkgs-check-by-name/tests/mock-nixpkgs.nix index 50ad67617542d..01bb27a480388 100644 --- a/pkgs/test/nixpkgs-check-by-name/tests/mock-nixpkgs.nix +++ b/pkgs/test/nixpkgs-check-by-name/tests/mock-nixpkgs.nix @@ -75,9 +75,14 @@ let # Turns autoCalledPackageFiles into an overlay that `callPackage`'s all of them autoCalledPackages = self: super: - builtins.mapAttrs (name: file: - self.callPackage file { } - ) autoCalledPackageFiles; + { + # Needed to be able to detect empty arguments in all-packages.nix + # See a more detailed description in pkgs/top-level/by-name-overlay.nix + _internalCallByNamePackageFile = file: self.callPackage file { }; + } + // builtins.mapAttrs + (name: self._internalCallByNamePackageFile) + autoCalledPackageFiles; # A list optionally containing the `all-packages.nix` file from the test case as an overlay optionalAllPackagesOverlay = 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 1c6377d8aef00..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 not defined as `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 1c6377d8aef00..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 not defined as `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 1c6377d8aef00..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 not defined as `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-success/all-packages.nix b/pkgs/test/nixpkgs-check-by-name/tests/override-success/all-packages.nix new file mode 100644 index 0000000000000..6b323756ae412 --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-success/all-packages.nix @@ -0,0 +1,5 @@ +self: super: { + foo = self.callPackage ./pkgs/by-name/fo/foo/package.nix { + enableBar = true; + }; +} diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-success/default.nix b/pkgs/test/nixpkgs-check-by-name/tests/override-success/default.nix new file mode 100644 index 0000000000000..af25d1450122b --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-success/default.nix @@ -0,0 +1 @@ +import ../mock-nixpkgs.nix { root = ./.; } diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-success/pkgs/by-name/fo/foo/package.nix b/pkgs/test/nixpkgs-check-by-name/tests/override-success/pkgs/by-name/fo/foo/package.nix new file mode 100644 index 0000000000000..c811a7215a286 --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-success/pkgs/by-name/fo/foo/package.nix @@ -0,0 +1,8 @@ +{ + someDrv, + enableBar ? false, +}: +if enableBar then + someDrv +else + {} |