BoomMicrophone/nixpkgs-suyu
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

tests.nixpkgs-check-by-name: Improve more errors

Browse source
This commit is contained in:
Silvan Mosberger 2024-02-23 00:10:20 +01:00
parent 7258d472bc
commit 75cdccd6c4
10 changed files with 134 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
pkgs/test/nixpkgs-check-by-name/src/eval.rs
View file

@ -305,7 +305,7 @@ fn by_name(
// Figure out whether it's an attribute definition of the form `= callPackage <arg1> <arg2>`,
// returning the arguments if so.
let optional_syntactic_call_package = nix_file.call_package_argument_info_at(
let (optional_syntactic_call_package, definition) = nix_file.call_package_argument_info_at(
location.line,
location.column,
nixpkgs_path,
@ -315,7 +315,7 @@ fn by_name(
// `callPackage`
match (is_semantic_call_package, optional_syntactic_call_package) {
// Something like `<attr> = foo`
(_, Err(definition)) => NixpkgsProblem::NonSyntacticCallPackage {
(_, None) => NixpkgsProblem::NonSyntacticCallPackage {
package_name: attribute_name.to_owned(),
file: relative_file,
line: location.line,
@ -324,17 +324,16 @@ fn by_name(
}
.into(),
// Something like `<attr> = pythonPackages.callPackage ...`
(false, Ok(_)) => {
// All of these are not of the expected form, so error out
// TODO: Make error more specific, don't lump everything together
NixpkgsProblem::WrongCallPackage {
relative_package_file: relative_package_file.to_owned(),
(false, Some(_)) => NixpkgsProblem::NonToplevelCallPackage {
package_name: attribute_name.to_owned(),
file: relative_file,
line: location.line,
column: location.column,
definition,
}
.into()
}
.into(),
// Something like `<attr> = pkgs.callPackage ...`
(true, Ok(syntactic_call_package)) => {
(true, Some(syntactic_call_package)) => {
if let Some(path) = syntactic_call_package.relative_path {
if path == relative_package_file {
// Manual definitions with empty arguments are not allowed
@ -358,9 +357,12 @@ fn by_name(
}
} else {
// No path
NixpkgsProblem::WrongCallPackage {
relative_package_file: relative_package_file.to_owned(),
NixpkgsProblem::NonPath {
package_name: attribute_name.to_owned(),
file: relative_file,
line: location.line,
column: location.column,
definition,
}
.into()
}
@ -451,7 +453,7 @@ fn handle_non_by_name_attribute(
// Parse the Nix file in the location and figure out whether it's an
// attribute definition of the form `= callPackage <arg1> <arg2>`,
// returning the arguments if so.
let optional_syntactic_call_package = nix_file_store
let (optional_syntactic_call_package, _definition) = nix_file_store
.get(&location.file)?
.call_package_argument_info_at(
location.line,
@ -466,16 +468,16 @@ fn handle_non_by_name_attribute(
// `callPackage`
match (is_semantic_call_package, optional_syntactic_call_package) {
// Something like `<attr> = { }`
(false, Err(_))
(false, None)
// Something like `<attr> = pythonPackages.callPackage ...`
| (false, Ok(_))
| (false, Some(_))
// Something like `<attr> = bar` where `bar = pkgs.callPackage ...`
| (true, Err(_)) => {
| (true, None) => {
// In all of these cases, it's not possible to migrate the package to `pkgs/by-name`
NonApplicable
}
// Something like `<attr> = pkgs.callPackage ...`
(true, Ok(syntactic_call_package)) => {
(true, Some(syntactic_call_package)) => {
// It's only possible to migrate such a definitions if..
match syntactic_call_package.relative_path {
Some(ref rel_path) if rel_path.starts_with(utils::BASE_SUBPATH) => {

35
pkgs/test/nixpkgs-check-by-name/src/nix_file.rs
View file

@ -119,13 +119,14 @@ impl NixFile {
line: usize,
column: usize,
relative_to: &Path,
) -> anyhow::Result<Result<CallPackageArgumentInfo, String>> {
) -> anyhow::Result<(Option<CallPackageArgumentInfo>, String)> {
Ok(match self.attrpath_value_at(line, column)? {
Err(definition) => Err(definition),
Err(definition) => (None, definition),
Ok(attrpath_value) => {
let definition = attrpath_value.to_string();
self.attrpath_value_call_package_argument_info(attrpath_value, relative_to)?
.ok_or(definition)
let attrpath_value =
self.attrpath_value_call_package_argument_info(attrpath_value, relative_to)?;
(attrpath_value, definition)
}
})
}
@ -492,41 +493,41 @@ mod tests {
let nix_file = NixFile::new(&file)?;
let cases = [
(2, Err("a.sub = null;")),
(3, Err("b = null;")),
(4, Err("c = import ./file.nix;")),
(5, Err("d = import ./file.nix { };")),
(2, None),
(3, None),
(4, None),
(5, None),
(
6,
Ok(CallPackageArgumentInfo {
Some(CallPackageArgumentInfo {
relative_path: Some(PathBuf::from("file.nix")),
empty_arg: true,
}),
),
(
7,
Ok(CallPackageArgumentInfo {
Some(CallPackageArgumentInfo {
relative_path: Some(PathBuf::from("file.nix")),
empty_arg: true,
}),
),
(
8,
Ok(CallPackageArgumentInfo {
Some(CallPackageArgumentInfo {
relative_path: None,
empty_arg: true,
}),
),
(
9,
Ok(CallPackageArgumentInfo {
Some(CallPackageArgumentInfo {
relative_path: Some(PathBuf::from("file.nix")),
empty_arg: false,
}),
),
(
10,
Ok(CallPackageArgumentInfo {
Some(CallPackageArgumentInfo {
relative_path: None,
empty_arg: false,
}),
@ -534,12 +535,10 @@ mod tests {
];
for (line, expected_result) in cases {
let actual_result = nix_file
let (actual_result, _definition) = nix_file
.call_package_argument_info_at(line, 3, temp_dir.path())
.context(format!("line {line}"))?
.map_err(|x| x);
let owned_expected_result = expected_result.map_err(|x| x.to_string());
assert_eq!(actual_result, owned_expected_result, "line {line}");
.context(format!("line {line}"))?;
assert_eq!(actual_result, expected_result, "line {line}");
}
Ok(())

79
pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs
View file

@ -47,6 +47,20 @@ pub enum NixpkgsProblem {
relative_package_file: PathBuf,
package_name: String,
},
NonToplevelCallPackage {
package_name: String,
file: PathBuf,
line: usize,
column: usize,
definition: String,
},
NonPath {
package_name: String,
file: PathBuf,
line: usize,
column: usize,
definition: String,
},
WrongCallPackagePath {
package_name: String,
file: PathBuf,
@ -182,6 +196,42 @@ impl fmt::Display for NixpkgsProblem {
"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()
),
NixpkgsProblem::NonToplevelCallPackage { package_name, file, line, column, definition } =>
writedoc!(
f,
"
- Because {} exists, the attribute `pkgs.{package_name}` must be defined like
{package_name} = callPackage {} {{ /* ... */ }};
This is however not the case: A different `callPackage` is used.
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::NonPath { package_name, file, line, column, definition } =>
writedoc!(
f,
"
- Because {} exists, the attribute `pkgs.{package_name}` must be defined like
{package_name} = callPackage {} {{ /* ... */ }};
This is however not the case: The first callPackage argument is not right:
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::WrongCallPackagePath { package_name, file, line, column, actual_path, expected_path } =>
writedoc! {
f,
@ -200,20 +250,6 @@ impl fmt::Display for NixpkgsProblem {
create_path_expr(file, actual_path),
},
NixpkgsProblem::NonSyntacticCallPackage { package_name, file, line, column, definition } => {
// This is needed such multi-line definitions don't look odd
// A bit round-about, but it works and we might not need anything more complicated
let definition_indented =
// The entire code should be indented 4 spaces
textwrap::indent(
// But first we want to strip the code's natural indentation
&textwrap::dedent(
// The definition _doesn't_ include the leading spaces, but we can
// recover those from the column
&format!("{}{definition}",
" ".repeat(column - 1)),
),
" ",
);
writedoc!(
f,
"
@ -229,7 +265,7 @@ impl fmt::Display for NixpkgsProblem {
structure::relative_file_for_package(package_name).display(),
file.display(),
line,
definition_indented,
indent_definition(*column, definition.clone()),
)
}
NixpkgsProblem::NonDerivation { relative_package_file, package_name } =>
@ -340,6 +376,19 @@ impl fmt::Display for NixpkgsProblem {
}
}
fn indent_definition(column: usize, definition: String) -> String {
// The entire code should be indented 4 spaces
textwrap::indent(
// But first we want to strip the code's natural indentation
&textwrap::dedent(
// The definition _doesn't_ include the leading spaces, but we can
// recover those from the column
&format!("{}{definition}", " ".repeat(column - 1)),
),
" ",
)
}
/// Creates a Nix path expression that when put into Nix file `from_file`, would point to the `to_file`.
fn create_path_expr(from_file: impl AsRef<Path>, to_file: impl AsRef<Path>) -> String {
// FIXME: Clean these unwrap's up

7
pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/all-packages.nix Normal file
View file

@ -0,0 +1,7 @@
self: super: {
alt.callPackage = self.lib.callPackageWith {};
foo = self.alt.callPackage ({ }: self.someDrv) { };
}

1
pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/default.nix Normal file
View file

@ -0,0 +1 @@
import <test-nixpkgs> { root = ./.; }

8
pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/expected Normal file
View file

@ -0,0 +1,8 @@
- Because pkgs/by-name/fo/foo exists, the attribute `pkgs.foo` must be defined like
foo = callPackage pkgs/by-name/fo/foo/package.nix { /* ... */ };
This is however not the case: A different `callPackage` is used.
It is defined in all-packages.nix:5 as
foo = self.alt.callPackage ({ }: self.someDrv) { };

1
pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/pkgs/by-name/fo/foo/package.nix Normal file
View file

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

1
pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/all-packages.nix
View file

@ -2,5 +2,4 @@ self: super: {
bar = (x: x) self.callPackage ./pkgs/by-name/fo/foo/package.nix { someFlag = true; };
foo = self.bar;
}

9
pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected
View file

@ -1 +1,8 @@
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 { /* ... */ };
This is however not the case: The first callPackage argument is not right:
It is defined in all-packages.nix:2 as
nonDerivation = self.callPackage ({ someDrv }: someDrv) { };

9
pkgs/test/nixpkgs-check-by-name/tests/override-non-path/expected
View file

@ -1 +1,8 @@
pkgs.foo: 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/fo/foo/package.nix { ... }` with a non-empty second argument.
- Because pkgs/by-name/fo/foo exists, the attribute `pkgs.foo` must be defined like
foo = callPackage pkgs/by-name/fo/foo/package.nix { /* ... */ };
This is however not the case: The first callPackage argument is not right:
It is defined in all-packages.nix:3 as
foo = self.callPackage ({ someDrv, someFlag }: someDrv) { someFlag = true; };