tests.nixpkgs-check-by-name: Disallow empty all-packages.nix overrides

Only enabled with `--version v1`

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

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

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;

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)
         })?;

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.

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 { };
+}

pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/default.nix

@@ -0,0 +1 @@
+import ../mock-nixpkgs.nix { root = ./.; }

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.

pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/pkgs/by-name/no/nonDerivation/package.nix

@@ -0,0 +1 @@
+{ someDrv }: someDrv

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.

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.