tests.nixpkgs-check-by-name: Improve error for wrong package file
This commit is contained in:
Silvan Mosberger
parent
24069b982d
commit
a53b07e252
6 changed files with 106 additions and 31 deletions
7
pkgs/test/nixpkgs-check-by-name/Cargo.lock
generated
7
pkgs/test/nixpkgs-check-by-name/Cargo.lock
generated
|
|
@ -299,6 +299,7 @@ dependencies = [
|
|||
"itertools",
|
||||
"lazy_static",
|
||||
"regex",
|
||||
"relative-path",
|
||||
"rnix",
|
||||
"rowan",
|
||||
"serde",
|
||||
|
|
@ -392,6 +393,12 @@ version = "0.7.4"
|
|||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e5ea92a5b6195c6ef2a0295ea818b312502c6fc94dde986c5553242e18fd4ce2"
|
||||
|
||||
[[package]]
|
||||
name = "relative-path"
|
||||
version = "1.9.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e898588f33fdd5b9420719948f9f2a32c922a246964576f71ba7f24f80610fbc"
|
||||
|
||||
[[package]]
|
||||
name = "rnix"
|
||||
version = "0.11.0"
|
||||
|
|
|
|||
|
|
@ -15,7 +15,8 @@ lazy_static = "1.4.0"
|
|||
colored = "2.0.4"
|
||||
itertools = "0.11.0"
|
||||
rowan = "0.15.11"
|
||||
indoc = "2.0.4"
|
||||
relative-path = "1.9.2"
|
||||
|
||||
[dev-dependencies]
|
||||
temp-env = "0.3.5"
|
||||
indoc = "2.0.4"
|
||||
|
|
|
|||
|
|
@ -52,22 +52,19 @@ struct Location {
|
|||
}
|
||||
|
||||
impl Location {
|
||||
// Creates a [String] from a [Location], in a format like `{file}:{line}:{column}`,
|
||||
// where `file` is relative to the given [Path].
|
||||
fn to_relative_string(&self, nixpkgs_path: &Path) -> anyhow::Result<String> {
|
||||
let relative = self.file.strip_prefix(nixpkgs_path).with_context(|| {
|
||||
format!(
|
||||
"An attributes location ({}) is outside Nixpkgs ({})",
|
||||
self.file.display(),
|
||||
nixpkgs_path.display()
|
||||
)
|
||||
})?;
|
||||
Ok(format!(
|
||||
"{}:{}:{}",
|
||||
relative.display(),
|
||||
self.line,
|
||||
self.column
|
||||
))
|
||||
// Returns the [file] field, but relative to Nixpkgs
|
||||
fn relative_file(&self, nixpkgs_path: &Path) -> anyhow::Result<PathBuf> {
|
||||
Ok(self
|
||||
.file
|
||||
.strip_prefix(nixpkgs_path)
|
||||
.with_context(|| {
|
||||
format!(
|
||||
"The file ({}) is outside Nixpkgs ({})",
|
||||
self.file.display(),
|
||||
nixpkgs_path.display()
|
||||
)
|
||||
})?
|
||||
.to_path_buf())
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -295,16 +292,24 @@ fn by_name(
|
|||
// We should expect manual definitions to have a location, otherwise we can't
|
||||
// enforce the expected format
|
||||
if let Some(location) = location {
|
||||
// Parse the Nix file in the location and figure out whether it's an
|
||||
// attribute definition of the form `= callPackage <arg1> <arg2>`,
|
||||
// Parse the Nix file in the location
|
||||
let nix_file = nix_file_store.get(&location.file)?;
|
||||
|
||||
// The relative path of the Nix file, for error messages
|
||||
let relative_file =
|
||||
location.relative_file(nixpkgs_path).with_context(|| {
|
||||
format!(
|
||||
"Failed to resolve location file of attribute {attribute_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_store
|
||||
.get(&location.file)?
|
||||
.call_package_argument_info_at(
|
||||
let optional_syntactic_call_package = nix_file.call_package_argument_info_at(
|
||||
location.line,
|
||||
location.column,
|
||||
nixpkgs_path,
|
||||
)?;
|
||||
).with_context(|| format!("Failed to get the definition info for attribute {attribute_name}"))?;
|
||||
|
||||
// At this point, we completed two different checks for whether it's a
|
||||
// `callPackage`
|
||||
|
|
@ -312,7 +317,9 @@ fn by_name(
|
|||
// Something like `<attr> = foo`
|
||||
(_, Err(definition)) => NixpkgsProblem::NonSyntacticCallPackage {
|
||||
package_name: attribute_name.to_owned(),
|
||||
location: location.to_relative_string(nixpkgs_path)?,
|
||||
file: relative_file,
|
||||
line: location.line,
|
||||
column: location.column,
|
||||
definition,
|
||||
}
|
||||
.into(),
|
||||
|
|
@ -338,13 +345,19 @@ fn by_name(
|
|||
Tight
|
||||
})
|
||||
} else {
|
||||
NixpkgsProblem::WrongCallPackage {
|
||||
relative_package_file: relative_package_file.to_owned(),
|
||||
// Wrong path
|
||||
NixpkgsProblem::WrongCallPackagePath {
|
||||
package_name: attribute_name.to_owned(),
|
||||
file: relative_file,
|
||||
line: location.line,
|
||||
column: location.column,
|
||||
actual_path: path,
|
||||
expected_path: relative_package_file,
|
||||
}
|
||||
.into()
|
||||
}
|
||||
} else {
|
||||
// No path
|
||||
NixpkgsProblem::WrongCallPackage {
|
||||
relative_package_file: relative_package_file.to_owned(),
|
||||
package_name: attribute_name.to_owned(),
|
||||
|
|
|
|||
|
|
@ -249,7 +249,7 @@ mod tests {
|
|||
|
||||
if actual_errors != expected_errors {
|
||||
panic!(
|
||||
"Failed test case {name}, expected these errors:\n\n{}\n\nbut got these:\n\n{}",
|
||||
"Failed test case {name}, expected these errors:\n=======\n{}\n=======\nbut got these:\n=======\n{}\n=======",
|
||||
expected_errors, actual_errors
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,8 +1,11 @@
|
|||
use crate::structure;
|
||||
use crate::utils::PACKAGE_NIX_FILENAME;
|
||||
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
|
||||
|
|
@ -44,9 +47,19 @@ pub enum NixpkgsProblem {
|
|||
relative_package_file: PathBuf,
|
||||
package_name: String,
|
||||
},
|
||||
WrongCallPackagePath {
|
||||
package_name: String,
|
||||
file: PathBuf,
|
||||
line: usize,
|
||||
column: usize,
|
||||
actual_path: PathBuf,
|
||||
expected_path: PathBuf,
|
||||
},
|
||||
NonSyntacticCallPackage {
|
||||
package_name: String,
|
||||
location: String,
|
||||
file: PathBuf,
|
||||
line: usize,
|
||||
column: usize,
|
||||
definition: String,
|
||||
},
|
||||
NonDerivation {
|
||||
|
|
@ -169,12 +182,32 @@ 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::NonSyntacticCallPackage { package_name, location, definition } =>
|
||||
NixpkgsProblem::WrongCallPackagePath { package_name, file, line, column, actual_path, expected_path } =>
|
||||
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 the wrong path.
|
||||
It is defined in {}:{}:{} as
|
||||
|
||||
{package_name} = callPackage {} {{ /* ... */ }}",
|
||||
structure::relative_dir_for_package(package_name).display(),
|
||||
create_path_expr(file, expected_path),
|
||||
file.display(), line, column,
|
||||
create_path_expr(file, actual_path),
|
||||
},
|
||||
NixpkgsProblem::NonSyntacticCallPackage { package_name, file, line, column, definition } =>
|
||||
write!(
|
||||
f,
|
||||
"Because {} exists, the attribute `pkgs.{package_name}` must be defined as `callPackage {} {{ ... }}`. This is however not the case: The attribute is defined in {location} as\n\t{}",
|
||||
"Because {} exists, the attribute `pkgs.{package_name}` must be defined as `callPackage {} {{ ... }}`. This is however not the case: The attribute is defined in {}:{}:{} as\n\t{}",
|
||||
structure::relative_dir_for_package(package_name).display(),
|
||||
structure::relative_file_for_package(package_name).display(),
|
||||
file.display(),
|
||||
line,
|
||||
column,
|
||||
definition,
|
||||
),
|
||||
NixpkgsProblem::NonDerivation { relative_package_file, package_name } =>
|
||||
|
|
@ -284,3 +317,17 @@ impl fmt::Display for NixpkgsProblem {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// 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
|
||||
// But these should never trigger because we only call this function with relative paths, and only
|
||||
// with `from_file` being an actual file (so there's always a parent)
|
||||
let from = RelativePath::from_path(&from_file)
|
||||
.unwrap()
|
||||
.parent()
|
||||
.unwrap();
|
||||
let to = RelativePath::from_path(&to_file).unwrap();
|
||||
let rel = from.relative(to);
|
||||
format!("./{rel}")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 the wrong path.
|
||||
It is defined in all-packages.nix:2:3 as
|
||||
|
||||
nonDerivation = callPackage ./someDrv.nix { /* ... */ }
|
||||
|
|
|
|||
Loading…
Reference in a new issue