diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index ddeac2a445b3..4748e907db72 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -339,7 +339,13 @@ fn by_name( // Manual definitions with empty arguments are not allowed // anymore Success(if syntactic_call_package.empty_arg { - Loose(()) + Loose(NixpkgsProblem::EmptyArgument { + package_name: attribute_name.to_owned(), + file: relative_file, + line: location.line, + column: location.column, + definition, + }) } else { Tight }) diff --git a/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs b/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs index 808cda47cc1a..cf40238f9cb6 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs @@ -4,11 +4,11 @@ use indoc::writedoc; use relative_path::RelativePath; use std::ffi::OsString; use std::fmt; -use std::io; use std::path::Path; use std::path::PathBuf; /// Any problem that can occur when checking Nixpkgs +#[derive(Clone)] pub enum NixpkgsProblem { ShardNonDir { relative_shard_path: PathBuf, @@ -43,9 +43,12 @@ pub enum NixpkgsProblem { relative_package_file: PathBuf, package_name: String, }, - WrongCallPackage { - relative_package_file: PathBuf, + EmptyArgument { package_name: String, + file: PathBuf, + line: usize, + column: usize, + definition: String, }, NonToplevelCallPackage { package_name: String, @@ -87,7 +90,7 @@ pub enum NixpkgsProblem { UnresolvableSymlink { relative_package_dir: PathBuf, subpath: PathBuf, - io_error: io::Error, + io_error: String, }, PathInterpolation { relative_package_dir: PathBuf, @@ -112,7 +115,7 @@ pub enum NixpkgsProblem { subpath: PathBuf, line: usize, text: String, - io_error: io::Error, + io_error: String, }, MovedOutOfByName { package_name: String, @@ -190,11 +193,23 @@ impl fmt::Display for NixpkgsProblem { "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}", relative_package_file.display() ), - NixpkgsProblem::WrongCallPackage { relative_package_file, package_name } => - write!( + NixpkgsProblem::EmptyArgument { package_name, file, line, column, definition } => + writedoc!( f, - "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() + " + - Because {} exists, the attribute `pkgs.{package_name}` must be defined like + + {package_name} = callPackage ./{} {{ /* ... */ }}; + + Notably the second argument must not be empty, which is not the case. + It is defined in {}:{} as + + {}", + structure::relative_dir_for_package(package_name).display(), + structure::relative_file_for_package(package_name).display(), + file.display(), + line, + indent_definition(*column, definition.clone()), ), NixpkgsProblem::NonToplevelCallPackage { package_name, file, line, column, definition } => writedoc!( diff --git a/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs b/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs index 200bf92c516a..bdc1fbec4a27 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs @@ -4,7 +4,6 @@ use crate::nix_file::CallPackageArgumentInfo; use crate::nixpkgs_problem::NixpkgsProblem; -use crate::structure; use crate::validation::{self, Validation, Validation::Success}; use std::collections::HashMap; @@ -127,17 +126,14 @@ impl RatchetState { pub enum ManualDefinition {} impl ToNixpkgsProblem for ManualDefinition { - type ToContext = (); + type ToContext = NixpkgsProblem; fn to_nixpkgs_problem( - name: &str, + _name: &str, _optional_from: Option<()>, - _to: &Self::ToContext, + to: &Self::ToContext, ) -> NixpkgsProblem { - NixpkgsProblem::WrongCallPackage { - relative_package_file: structure::relative_file_for_package(name), - package_name: name.to_owned(), - } + (*to).clone() } } diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index 169e996300ba..b716b99099c4 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -66,7 +66,7 @@ fn check_path( Err(io_error) => NixpkgsProblem::UnresolvableSymlink { relative_package_dir: relative_package_dir.to_path_buf(), subpath: subpath.to_path_buf(), - io_error, + io_error: io_error.to_string(), } .into(), } @@ -160,7 +160,7 @@ fn check_nix_file( subpath: subpath.to_path_buf(), line, text, - io_error: e, + io_error: e.to_string(), } .into(), ResolvedPath::Within(..) => { diff --git a/pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected b/pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected index 1cc970b7fd72..e619887012d0 100644 --- a/pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected +++ b/pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected @@ -1,3 +1,17 @@ -pkgs.noEval: 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/noEval/package.nix { ... }` with a non-empty second argument. -pkgs.onlyMove: 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/on/onlyMove/package.nix { ... }` with a non-empty second argument. +- Because pkgs/by-name/no/noEval exists, the attribute `pkgs.noEval` must be defined like + + noEval = callPackage ./pkgs/by-name/no/noEval/package.nix { /* ... */ }; + + Notably the second argument must not be empty, which is not the case. + It is defined in all-packages.nix:9 as + + noEval = self.callPackage ./pkgs/by-name/no/noEval/package.nix { }; +- Because pkgs/by-name/on/onlyMove exists, the attribute `pkgs.onlyMove` must be defined like + + onlyMove = callPackage ./pkgs/by-name/on/onlyMove/package.nix { /* ... */ }; + + Notably the second argument must not be empty, which is not the case. + It is defined in all-packages.nix:7 as + + onlyMove = self.callPackage ./pkgs/by-name/on/onlyMove/package.nix { }; This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch 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 index bf96b181d7fe..dc64698f4bfa 100644 --- a/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected @@ -1,2 +1,9 @@ -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. +- Because pkgs/by-name/no/nonDerivation exists, the attribute `pkgs.nonDerivation` must be defined like + + nonDerivation = callPackage ./pkgs/by-name/no/nonDerivation/package.nix { /* ... */ }; + + Notably the second argument must not be empty, which is not the case. + It is defined in all-packages.nix:2 as + + nonDerivation = self.callPackage ./pkgs/by-name/no/nonDerivation/package.nix { }; This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch diff --git a/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected b/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected index b835348cc3cf..d96d0d5638e5 100644 --- a/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected +++ b/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected @@ -1,5 +1,19 @@ -pkgs.a: 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/a/a/package.nix { ... }` with a non-empty second argument. +- Because pkgs/by-name/a/a exists, the attribute `pkgs.a` must be defined like + + a = callPackage ./pkgs/by-name/a/a/package.nix { /* ... */ }; + + Notably the second argument must not be empty, which is not the case. + It is defined in all-packages.nix:2 as + + a = self.callPackage ./pkgs/by-name/a/a/package.nix { }; pkgs.b: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/b/b/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore. -pkgs.c: 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/c/c/package.nix { ... }` with a non-empty second argument. +- Because pkgs/by-name/c/c exists, the attribute `pkgs.c` must be defined like + + c = callPackage ./pkgs/by-name/c/c/package.nix { /* ... */ }; + + Notably the second argument must not be empty, which is not the case. + It is defined in all-packages.nix:4 as + + c = self.callPackage ./pkgs/by-name/c/c/package.nix { }; pkgs.d: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/d/d/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore. This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch