cafkafk/nixpkgs
cafkafk/nixpkgs
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

tests.nixpkgs-check-by-name: Use RelativePath for relative paths

Browse source 
Makes the code easier to understand and less error-prone
This commit is contained in:
Silvan Mosberger 2024-03-01 02:29:24 +01:00
parent 7858f06288
commit 5981aff212
6 changed files with 158 additions and 153 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -8,6 +8,7 @@ use crate::utils;
use crate::validation::ResultIteratorExt as _; use crate::validation::ResultIteratorExt as _;
use crate::validation::{self, Validation::Success}; use crate::validation::{self, Validation::Success};
use crate::NixFileStore; use crate::NixFileStore;
use relative_path::RelativePathBuf;
use std::path::Path; use std::path::Path;
use anyhow::Context; use anyhow::Context;
@ -56,18 +57,15 @@ struct Location {
impl Location { impl Location {
// Returns the [file] field, but relative to Nixpkgs // Returns the [file] field, but relative to Nixpkgs
fn relative_file(&self, nixpkgs_path: &Path) -> anyhow::Result<PathBuf> { fn relative_file(&self, nixpkgs_path: &Path) -> anyhow::Result<RelativePathBuf> {
Ok(self let path = self.file.strip_prefix(nixpkgs_path).with_context(|| {
.file
.strip_prefix(nixpkgs_path)
.with_context(|| {
format!( format!(
"The file ({}) is outside Nixpkgs ({})", "The file ({}) is outside Nixpkgs ({})",
self.file.display(), self.file.display(),
nixpkgs_path.display() nixpkgs_path.display()
) )
})? })?;
.to_path_buf()) Ok(RelativePathBuf::from_path(path).expect("relative path"))
} }
} }
@ -352,12 +350,12 @@ fn by_name(
/// all-packages.nix /// all-packages.nix
fn by_name_override( fn by_name_override(
attribute_name: &str, attribute_name: &str,
expected_package_file: PathBuf, expected_package_file: RelativePathBuf,
is_semantic_call_package: bool, is_semantic_call_package: bool,
optional_syntactic_call_package: Option<CallPackageArgumentInfo>, optional_syntactic_call_package: Option<CallPackageArgumentInfo>,
definition: String, definition: String,
location: Location, location: Location,
relative_location_file: PathBuf, relative_location_file: RelativePathBuf,
) -> validation::Validation<ratchet::RatchetState<ratchet::ManualDefinition>> { ) -> validation::Validation<ratchet::RatchetState<ratchet::ManualDefinition>> {
// At this point, we completed two different checks for whether it's a // At this point, we completed two different checks for whether it's a
// `callPackage` // `callPackage`

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

@ -3,6 +3,7 @@
use crate::utils::LineIndex; use crate::utils::LineIndex;
use anyhow::Context; use anyhow::Context;
use itertools::Either::{self, Left, Right}; use itertools::Either::{self, Left, Right};
use relative_path::RelativePathBuf;
use rnix::ast; use rnix::ast;
use rnix::ast::Expr; use rnix::ast::Expr;
use rnix::ast::HasEntry; use rnix::ast::HasEntry;
@ -79,7 +80,7 @@ impl NixFile {
#[derive(Debug, PartialEq)] #[derive(Debug, PartialEq)]
pub struct CallPackageArgumentInfo { pub struct CallPackageArgumentInfo {
/// The relative path of the first argument, or `None` if it's not a path. /// The relative path of the first argument, or `None` if it's not a path.
pub relative_path: Option<PathBuf>, pub relative_path: Option<RelativePathBuf>,
/// Whether the second argument is an empty attribute set /// Whether the second argument is an empty attribute set
pub empty_arg: bool, pub empty_arg: bool,
} }
@ -369,8 +370,8 @@ pub enum ResolvedPath {
/// The path is outside the given absolute path /// The path is outside the given absolute path
Outside, Outside,
/// The path is within the given absolute path. /// The path is within the given absolute path.
/// The `PathBuf` is the relative path under the given absolute path. /// The `RelativePathBuf` is the relative path under the given absolute path.
Within(PathBuf), Within(RelativePathBuf),
} }
impl NixFile { impl NixFile {
@ -402,7 +403,9 @@ impl NixFile {
// Check if it's within relative_to // Check if it's within relative_to
match resolved.strip_prefix(relative_to) { match resolved.strip_prefix(relative_to) {
Err(_prefix_error) => ResolvedPath::Outside, Err(_prefix_error) => ResolvedPath::Outside,
Ok(suffix) => ResolvedPath::Within(suffix.to_path_buf()), Ok(suffix) => ResolvedPath::Within(
RelativePathBuf::from_path(suffix).expect("a relative path"),
),
} }
} }
} }
@ -506,14 +509,14 @@ mod tests {
( (
6, 6,
Some(CallPackageArgumentInfo { Some(CallPackageArgumentInfo {
relative_path: Some(PathBuf::from("file.nix")), relative_path: Some(RelativePathBuf::from("file.nix")),
empty_arg: true, empty_arg: true,
}), }),
), ),
( (
7, 7,
Some(CallPackageArgumentInfo { Some(CallPackageArgumentInfo {
relative_path: Some(PathBuf::from("file.nix")), relative_path: Some(RelativePathBuf::from("file.nix")),
empty_arg: true, empty_arg: true,
}), }),
), ),
@ -527,7 +530,7 @@ mod tests {
( (
9, 9,
Some(CallPackageArgumentInfo { Some(CallPackageArgumentInfo {
relative_path: Some(PathBuf::from("file.nix")), relative_path: Some(RelativePathBuf::from("file.nix")),
empty_arg: false, empty_arg: false,
}), }),
), ),

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

@ -2,139 +2,140 @@ use crate::structure;
use crate::utils::PACKAGE_NIX_FILENAME; use crate::utils::PACKAGE_NIX_FILENAME;
use indoc::writedoc; use indoc::writedoc;
use relative_path::RelativePath; use relative_path::RelativePath;
use relative_path::RelativePathBuf;
use std::ffi::OsString; use std::ffi::OsString;
use std::fmt; use std::fmt;
use std::path::Path;
use std::path::PathBuf;
/// Any problem that can occur when checking Nixpkgs /// Any problem that can occur when checking Nixpkgs
/// All paths are relative to Nixpkgs such that the error messages can't be influenced by Nixpkgs absolute
/// location
#[derive(Clone)] #[derive(Clone)]
pub enum NixpkgsProblem { pub enum NixpkgsProblem {
ShardNonDir { ShardNonDir {
relative_shard_path: PathBuf, relative_shard_path: RelativePathBuf,
}, },
InvalidShardName { InvalidShardName {
relative_shard_path: PathBuf, relative_shard_path: RelativePathBuf,
shard_name: String, shard_name: String,
}, },
PackageNonDir { PackageNonDir {
relative_package_dir: PathBuf, relative_package_dir: RelativePathBuf,
}, },
CaseSensitiveDuplicate { CaseSensitiveDuplicate {
relative_shard_path: PathBuf, relative_shard_path: RelativePathBuf,
first: OsString, first: OsString,
second: OsString, second: OsString,
}, },
InvalidPackageName { InvalidPackageName {
relative_package_dir: PathBuf, relative_package_dir: RelativePathBuf,
package_name: String, package_name: String,
}, },
IncorrectShard { IncorrectShard {
relative_package_dir: PathBuf, relative_package_dir: RelativePathBuf,
correct_relative_package_dir: PathBuf, correct_relative_package_dir: RelativePathBuf,
}, },
PackageNixNonExistent { PackageNixNonExistent {
relative_package_dir: PathBuf, relative_package_dir: RelativePathBuf,
}, },
PackageNixDir { PackageNixDir {
relative_package_dir: PathBuf, relative_package_dir: RelativePathBuf,
}, },
UndefinedAttr { UndefinedAttr {
relative_package_file: PathBuf, relative_package_file: RelativePathBuf,
package_name: String, package_name: String,
}, },
EmptyArgument { EmptyArgument {
package_name: String, package_name: String,
file: PathBuf, file: RelativePathBuf,
line: usize, line: usize,
column: usize, column: usize,
definition: String, definition: String,
}, },
NonToplevelCallPackage { NonToplevelCallPackage {
package_name: String, package_name: String,
file: PathBuf, file: RelativePathBuf,
line: usize, line: usize,
column: usize, column: usize,
definition: String, definition: String,
}, },
NonPath { NonPath {
package_name: String, package_name: String,
file: PathBuf, file: RelativePathBuf,
line: usize, line: usize,
column: usize, column: usize,
definition: String, definition: String,
}, },
WrongCallPackagePath { WrongCallPackagePath {
package_name: String, package_name: String,
file: PathBuf, file: RelativePathBuf,
line: usize, line: usize,
actual_path: PathBuf, actual_path: RelativePathBuf,
expected_path: PathBuf, expected_path: RelativePathBuf,
}, },
NonSyntacticCallPackage { NonSyntacticCallPackage {
package_name: String, package_name: String,
file: PathBuf, file: RelativePathBuf,
line: usize, line: usize,
column: usize, column: usize,
definition: String, definition: String,
}, },
NonDerivation { NonDerivation {
relative_package_file: PathBuf, relative_package_file: RelativePathBuf,
package_name: String, package_name: String,
}, },
OutsideSymlink { OutsideSymlink {
relative_package_dir: PathBuf, relative_package_dir: RelativePathBuf,
subpath: PathBuf, subpath: RelativePathBuf,
}, },
UnresolvableSymlink { UnresolvableSymlink {
relative_package_dir: PathBuf, relative_package_dir: RelativePathBuf,
subpath: PathBuf, subpath: RelativePathBuf,
io_error: String, io_error: String,
}, },
PathInterpolation { PathInterpolation {
relative_package_dir: PathBuf, relative_package_dir: RelativePathBuf,
subpath: PathBuf, subpath: RelativePathBuf,
line: usize, line: usize,
text: String, text: String,
}, },
SearchPath { SearchPath {
relative_package_dir: PathBuf, relative_package_dir: RelativePathBuf,
subpath: PathBuf, subpath: RelativePathBuf,
line: usize, line: usize,
text: String, text: String,
}, },
OutsidePathReference { OutsidePathReference {
relative_package_dir: PathBuf, relative_package_dir: RelativePathBuf,
subpath: PathBuf, subpath: RelativePathBuf,
line: usize, line: usize,
text: String, text: String,
}, },
UnresolvablePathReference { UnresolvablePathReference {
relative_package_dir: PathBuf, relative_package_dir: RelativePathBuf,
subpath: PathBuf, subpath: RelativePathBuf,
line: usize, line: usize,
text: String, text: String,
io_error: String, io_error: String,
}, },
MovedOutOfByNameEmptyArg { MovedOutOfByNameEmptyArg {
package_name: String, package_name: String,
call_package_path: Option<PathBuf>, call_package_path: Option<RelativePathBuf>,
file: PathBuf, file: RelativePathBuf,
}, },
MovedOutOfByNameNonEmptyArg { MovedOutOfByNameNonEmptyArg {
package_name: String, package_name: String,
call_package_path: Option<PathBuf>, call_package_path: Option<RelativePathBuf>,
file: PathBuf, file: RelativePathBuf,
}, },
NewPackageNotUsingByNameEmptyArg { NewPackageNotUsingByNameEmptyArg {
package_name: String, package_name: String,
call_package_path: Option<PathBuf>, call_package_path: Option<RelativePathBuf>,
file: PathBuf, file: RelativePathBuf,
}, },
NewPackageNotUsingByNameNonEmptyArg { NewPackageNotUsingByNameNonEmptyArg {
package_name: String, package_name: String,
call_package_path: Option<PathBuf>, call_package_path: Option<RelativePathBuf>,
file: PathBuf, file: RelativePathBuf,
}, },
InternalCallPackageUsed { InternalCallPackageUsed {
attr_name: String, attr_name: String,
@ -151,56 +152,56 @@ impl fmt::Display for NixpkgsProblem {
write!( write!(
f, f,
"{}: This is a file, but it should be a directory.", "{}: This is a file, but it should be a directory.",
relative_shard_path.display(), relative_shard_path,
), ),
NixpkgsProblem::InvalidShardName { relative_shard_path, shard_name } => NixpkgsProblem::InvalidShardName { relative_shard_path, shard_name } =>
write!( write!(
f, f,
"{}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".", "{}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".",
relative_shard_path.display() relative_shard_path,
), ),
NixpkgsProblem::PackageNonDir { relative_package_dir } => NixpkgsProblem::PackageNonDir { relative_package_dir } =>
write!( write!(
f, f,
"{}: This path is a file, but it should be a directory.", "{}: This path is a file, but it should be a directory.",
relative_package_dir.display(), relative_package_dir,
), ),
NixpkgsProblem::CaseSensitiveDuplicate { relative_shard_path, first, second } => NixpkgsProblem::CaseSensitiveDuplicate { relative_shard_path, first, second } =>
write!( write!(
f, f,
"{}: Duplicate case-sensitive package directories {first:?} and {second:?}.", "{}: Duplicate case-sensitive package directories {first:?} and {second:?}.",
relative_shard_path.display(), relative_shard_path,
), ),
NixpkgsProblem::InvalidPackageName { relative_package_dir, package_name } => NixpkgsProblem::InvalidPackageName { relative_package_dir, package_name } =>
write!( write!(
f, f,
"{}: Invalid package directory name \"{package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".", "{}: Invalid package directory name \"{package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".",
relative_package_dir.display(), relative_package_dir,
), ),
NixpkgsProblem::IncorrectShard { relative_package_dir, correct_relative_package_dir } => NixpkgsProblem::IncorrectShard { relative_package_dir, correct_relative_package_dir } =>
write!( write!(
f, f,
"{}: Incorrect directory location, should be {} instead.", "{}: Incorrect directory location, should be {} instead.",
relative_package_dir.display(), relative_package_dir,
correct_relative_package_dir.display(), correct_relative_package_dir,
), ),
NixpkgsProblem::PackageNixNonExistent { relative_package_dir } => NixpkgsProblem::PackageNixNonExistent { relative_package_dir } =>
write!( write!(
f, f,
"{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.", "{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.",
relative_package_dir.display(), relative_package_dir,
), ),
NixpkgsProblem::PackageNixDir { relative_package_dir } => NixpkgsProblem::PackageNixDir { relative_package_dir } =>
write!( write!(
f, f,
"{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.", "{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.",
relative_package_dir.display(), relative_package_dir,
), ),
NixpkgsProblem::UndefinedAttr { relative_package_file, package_name } => NixpkgsProblem::UndefinedAttr { relative_package_file, package_name } =>
write!( write!(
f, f,
"pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}", "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}",
relative_package_file.display() relative_package_file,
), ),
NixpkgsProblem::EmptyArgument { package_name, file, line, column, definition } => NixpkgsProblem::EmptyArgument { package_name, file, line, column, definition } =>
writedoc!( writedoc!(
@ -216,9 +217,9 @@ impl fmt::Display for NixpkgsProblem {
Such a definition is provided automatically and therefore not necessary. Please remove it. Such a definition is provided automatically and therefore not necessary. Please remove it.
", ",
structure::relative_dir_for_package(package_name).display(), structure::relative_dir_for_package(package_name),
structure::relative_file_for_package(package_name).display(), structure::relative_file_for_package(package_name),
file.display(), file,
line, line,
indent_definition(*column, definition.clone()), indent_definition(*column, definition.clone()),
), ),
@ -234,9 +235,9 @@ impl fmt::Display for NixpkgsProblem {
{} {}
", ",
structure::relative_dir_for_package(package_name).display(), structure::relative_dir_for_package(package_name),
structure::relative_file_for_package(package_name).display(), structure::relative_file_for_package(package_name),
file.display(), file,
line, line,
indent_definition(*column, definition.clone()), indent_definition(*column, definition.clone()),
), ),
@ -252,9 +253,9 @@ impl fmt::Display for NixpkgsProblem {
{} {}
", ",
structure::relative_dir_for_package(package_name).display(), structure::relative_dir_for_package(package_name),
structure::relative_file_for_package(package_name).display(), structure::relative_file_for_package(package_name),
file.display(), file,
line, line,
indent_definition(*column, definition.clone()), indent_definition(*column, definition.clone()),
), ),
@ -270,9 +271,9 @@ impl fmt::Display for NixpkgsProblem {
{package_name} = callPackage {} {{ /* ... */ }}; {package_name} = callPackage {} {{ /* ... */ }};
", ",
structure::relative_dir_for_package(package_name).display(), structure::relative_dir_for_package(package_name),
create_path_expr(file, expected_path), create_path_expr(file, expected_path),
file.display(), line, file, line,
create_path_expr(file, actual_path), create_path_expr(file, actual_path),
}, },
NixpkgsProblem::NonSyntacticCallPackage { package_name, file, line, column, definition } => { NixpkgsProblem::NonSyntacticCallPackage { package_name, file, line, column, definition } => {
@ -287,9 +288,9 @@ impl fmt::Display for NixpkgsProblem {
{} {}
", ",
structure::relative_dir_for_package(package_name).display(), structure::relative_dir_for_package(package_name),
structure::relative_file_for_package(package_name).display(), structure::relative_file_for_package(package_name),
file.display(), file,
line, line,
indent_definition(*column, definition.clone()), indent_definition(*column, definition.clone()),
) )
@ -298,58 +299,58 @@ impl fmt::Display for NixpkgsProblem {
write!( write!(
f, f,
"pkgs.{package_name}: This attribute defined by {} is not a derivation", "pkgs.{package_name}: This attribute defined by {} is not a derivation",
relative_package_file.display() relative_package_file,
), ),
NixpkgsProblem::OutsideSymlink { relative_package_dir, subpath } => NixpkgsProblem::OutsideSymlink { relative_package_dir, subpath } =>
write!( write!(
f, f,
"{}: Path {} is a symlink pointing to a path outside the directory of that package.", "{}: Path {} is a symlink pointing to a path outside the directory of that package.",
relative_package_dir.display(), relative_package_dir,
subpath.display(), subpath,
), ),
NixpkgsProblem::UnresolvableSymlink { relative_package_dir, subpath, io_error } => NixpkgsProblem::UnresolvableSymlink { relative_package_dir, subpath, io_error } =>
write!( write!(
f, f,
"{}: Path {} is a symlink which cannot be resolved: {io_error}.", "{}: Path {} is a symlink which cannot be resolved: {io_error}.",
relative_package_dir.display(), relative_package_dir,
subpath.display(), subpath,
), ),
NixpkgsProblem::PathInterpolation { relative_package_dir, subpath, line, text } => NixpkgsProblem::PathInterpolation { relative_package_dir, subpath, line, text } =>
write!( write!(
f, f,
"{}: File {} at line {line} contains the path expression \"{}\", which is not yet supported and may point outside the directory of that package.", "{}: File {} at line {line} contains the path expression \"{}\", which is not yet supported and may point outside the directory of that package.",
relative_package_dir.display(), relative_package_dir,
subpath.display(), subpath,
text text
), ),
NixpkgsProblem::SearchPath { relative_package_dir, subpath, line, text } => NixpkgsProblem::SearchPath { relative_package_dir, subpath, line, text } =>
write!( write!(
f, f,
"{}: File {} at line {line} contains the nix search path expression \"{}\" which may point outside the directory of that package.", "{}: File {} at line {line} contains the nix search path expression \"{}\" which may point outside the directory of that package.",
relative_package_dir.display(), relative_package_dir,
subpath.display(), subpath,
text text
), ),
NixpkgsProblem::OutsidePathReference { relative_package_dir, subpath, line, text } => NixpkgsProblem::OutsidePathReference { relative_package_dir, subpath, line, text } =>
write!( write!(
f, f,
"{}: File {} at line {line} contains the path expression \"{}\" which may point outside the directory of that package.", "{}: File {} at line {line} contains the path expression \"{}\" which may point outside the directory of that package.",
relative_package_dir.display(), relative_package_dir,
subpath.display(), subpath,
text, text,
), ),
NixpkgsProblem::UnresolvablePathReference { relative_package_dir, subpath, line, text, io_error } => NixpkgsProblem::UnresolvablePathReference { relative_package_dir, subpath, line, text, io_error } =>
write!( write!(
f, f,
"{}: File {} at line {line} contains the path expression \"{}\" which cannot be resolved: {io_error}.", "{}: File {} at line {line} contains the path expression \"{}\" which cannot be resolved: {io_error}.",
relative_package_dir.display(), relative_package_dir,
subpath.display(), subpath,
text, text,
), ),
NixpkgsProblem::MovedOutOfByNameEmptyArg { package_name, call_package_path, file } => { NixpkgsProblem::MovedOutOfByNameEmptyArg { package_name, call_package_path, file } => {
let call_package_arg = let call_package_arg =
if let Some(path) = &call_package_path { if let Some(path) = &call_package_path {
format!("./{}", path.display()) format!("./{}", path)
} else { } else {
"...".into() "...".into()
}; };
@ -359,14 +360,14 @@ impl fmt::Display for NixpkgsProblem {
- Attribute `pkgs.{package_name}` was previously defined in {}, but is now manually defined as `callPackage {call_package_arg} {{ /* ... */ }}` in {}. - Attribute `pkgs.{package_name}` was previously defined in {}, but is now manually defined as `callPackage {call_package_arg} {{ /* ... */ }}` in {}.
Please move the package back and remove the manual `callPackage`. Please move the package back and remove the manual `callPackage`.
", ",
structure::relative_file_for_package(package_name).display(), structure::relative_file_for_package(package_name),
file.display(), file,
) )
}, },
NixpkgsProblem::MovedOutOfByNameNonEmptyArg { package_name, call_package_path, file } => { NixpkgsProblem::MovedOutOfByNameNonEmptyArg { package_name, call_package_path, file } => {
let call_package_arg = let call_package_arg =
if let Some(path) = &call_package_path { if let Some(path) = &call_package_path {
format!("./{}", path.display()) format!("./{}", path)
} else { } else {
"...".into() "...".into()
}; };
@ -378,14 +379,14 @@ impl fmt::Display for NixpkgsProblem {
- Attribute `pkgs.{package_name}` was previously defined in {}, but is now manually defined as `callPackage {call_package_arg} {{ ... }}` in {}. - Attribute `pkgs.{package_name}` was previously defined in {}, but is now manually defined as `callPackage {call_package_arg} {{ ... }}` in {}.
While the manual `callPackage` is still needed, it's not necessary to move the package files. While the manual `callPackage` is still needed, it's not necessary to move the package files.
", ",
structure::relative_file_for_package(package_name).display(), structure::relative_file_for_package(package_name),
file.display(), file,
) )
}, },
NixpkgsProblem::NewPackageNotUsingByNameEmptyArg { package_name, call_package_path, file } => { NixpkgsProblem::NewPackageNotUsingByNameEmptyArg { package_name, call_package_path, file } => {
let call_package_arg = let call_package_arg =
if let Some(path) = &call_package_path { if let Some(path) = &call_package_path {
format!("./{}", path.display()) format!("./{}", path)
} else { } else {
"...".into() "...".into()
}; };
@ -397,14 +398,14 @@ impl fmt::Display for NixpkgsProblem {
See `pkgs/by-name/README.md` for more details. See `pkgs/by-name/README.md` for more details.
Since the second `callPackage` argument is `{{ }}`, no manual `callPackage` in {} is needed anymore. Since the second `callPackage` argument is `{{ }}`, no manual `callPackage` in {} is needed anymore.
", ",
structure::relative_file_for_package(package_name).display(), structure::relative_file_for_package(package_name),
file.display(), file,
) )
}, },
NixpkgsProblem::NewPackageNotUsingByNameNonEmptyArg { package_name, call_package_path, file } => { NixpkgsProblem::NewPackageNotUsingByNameNonEmptyArg { package_name, call_package_path, file } => {
let call_package_arg = let call_package_arg =
if let Some(path) = &call_package_path { if let Some(path) = &call_package_path {
format!("./{}", path.display()) format!("./{}", path)
} else { } else {
"...".into() "...".into()
}; };
@ -416,8 +417,8 @@ impl fmt::Display for NixpkgsProblem {
See `pkgs/by-name/README.md` for more details. See `pkgs/by-name/README.md` for more details.
Since the second `callPackage` argument is not `{{ }}`, the manual `callPackage` in {} is still needed. Since the second `callPackage` argument is not `{{ }}`, the manual `callPackage` in {} is still needed.
", ",
structure::relative_file_for_package(package_name).display(), structure::relative_file_for_package(package_name),
file.display(), file,
) )
}, },
NixpkgsProblem::InternalCallPackageUsed { attr_name } => NixpkgsProblem::InternalCallPackageUsed { attr_name } =>
@ -448,14 +449,13 @@ fn indent_definition(column: usize, definition: String) -> String {
} }
/// Creates a Nix path expression that when put into Nix file `from_file`, would point to the `to_file`. /// 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 { fn create_path_expr(
// These `expect` calls should never trigger because we only call this function with from_file: impl AsRef<RelativePath>,
// relative paths that have a parent. That's why we `expect` them! to_file: impl AsRef<RelativePath>,
let from = RelativePath::from_path(&from_file) ) -> String {
.expect("a relative path") // This `expect` calls should never trigger because we only call this function with files.
.parent() // That's why we `expect` them!
.expect("a parent for this path"); let from = from_file.as_ref().parent().expect("a parent for this path");
let to = RelativePath::from_path(&to_file).expect("a path"); let rel = from.relative(to_file);
let rel = from.relative(to);
format!("./{rel}") format!("./{rel}")
} }

4
pkgs/test/nixpkgs-check-by-name/src/ratchet.rs
View file

@ -5,8 +5,8 @@
use crate::nix_file::CallPackageArgumentInfo; use crate::nix_file::CallPackageArgumentInfo;
use crate::nixpkgs_problem::NixpkgsProblem; use crate::nixpkgs_problem::NixpkgsProblem;
use crate::validation::{self, Validation, Validation::Success}; use crate::validation::{self, Validation, Validation::Success};
use relative_path::RelativePathBuf;
use std::collections::HashMap; use std::collections::HashMap;
use std::path::PathBuf;
/// The ratchet value for the entirety of Nixpkgs. /// The ratchet value for the entirety of Nixpkgs.
#[derive(Default)] #[derive(Default)]
@ -146,7 +146,7 @@ impl ToNixpkgsProblem for ManualDefinition {
pub enum UsesByName {} pub enum UsesByName {}
impl ToNixpkgsProblem for UsesByName { impl ToNixpkgsProblem for UsesByName {
type ToContext = (CallPackageArgumentInfo, PathBuf); type ToContext = (CallPackageArgumentInfo, RelativePathBuf);
fn to_nixpkgs_problem( fn to_nixpkgs_problem(
name: &str, name: &str,

52
pkgs/test/nixpkgs-check-by-name/src/references.rs
View file

@ -2,6 +2,7 @@ use crate::nixpkgs_problem::NixpkgsProblem;
use crate::utils; use crate::utils;
use crate::validation::{self, ResultIteratorExt, Validation::Success}; use crate::validation::{self, ResultIteratorExt, Validation::Success};
use crate::NixFileStore; use crate::NixFileStore;
use relative_path::RelativePath;
use anyhow::Context; use anyhow::Context;
use rowan::ast::AstNode; use rowan::ast::AstNode;
@ -12,14 +13,14 @@ use std::path::Path;
/// Both symlinks and Nix path expressions are checked. /// Both symlinks and Nix path expressions are checked.
pub fn check_references( pub fn check_references(
nix_file_store: &mut NixFileStore, nix_file_store: &mut NixFileStore,
relative_package_dir: &Path, relative_package_dir: &RelativePath,
absolute_package_dir: &Path, absolute_package_dir: &Path,
) -> validation::Result<()> { ) -> validation::Result<()> {
// The first subpath to check is the package directory itself, which we can represent as an // The first subpath to check is the package directory itself, which we can represent as an
// empty path, since the absolute package directory gets prepended to this. // empty path, since the absolute package directory gets prepended to this.
// We don't use `./.` to keep the error messages cleaner // We don't use `./.` to keep the error messages cleaner
// (there's no canonicalisation going on underneath) // (there's no canonicalisation going on underneath)
let subpath = Path::new(""); let subpath = RelativePath::new("");
check_path( check_path(
nix_file_store, nix_file_store,
relative_package_dir, relative_package_dir,
@ -29,7 +30,7 @@ pub fn check_references(
.with_context(|| { .with_context(|| {
format!( format!(
"While checking the references in package directory {}", "While checking the references in package directory {}",
relative_package_dir.display() relative_package_dir
) )
}) })
} }
@ -41,11 +42,11 @@ pub fn check_references(
/// The absolute package directory gets prepended before doing anything with it though. /// The absolute package directory gets prepended before doing anything with it though.
fn check_path( fn check_path(
nix_file_store: &mut NixFileStore, nix_file_store: &mut NixFileStore,
relative_package_dir: &Path, relative_package_dir: &RelativePath,
absolute_package_dir: &Path, absolute_package_dir: &Path,
subpath: &Path, subpath: &RelativePath,
) -> validation::Result<()> { ) -> validation::Result<()> {
let path = absolute_package_dir.join(subpath); let path = subpath.to_path(absolute_package_dir);
Ok(if path.is_symlink() { Ok(if path.is_symlink() {
// Check whether the symlink resolves to outside the package directory // Check whether the symlink resolves to outside the package directory
@ -55,8 +56,8 @@ fn check_path(
// entire directory recursively anyways // entire directory recursively anyways
if let Err(_prefix_error) = target.strip_prefix(absolute_package_dir) { if let Err(_prefix_error) = target.strip_prefix(absolute_package_dir) {
NixpkgsProblem::OutsideSymlink { NixpkgsProblem::OutsideSymlink {
relative_package_dir: relative_package_dir.to_path_buf(), relative_package_dir: relative_package_dir.to_owned(),
subpath: subpath.to_path_buf(), subpath: subpath.to_owned(),
} }
.into() .into()
} else { } else {
@ -64,8 +65,8 @@ fn check_path(
} }
} }
Err(io_error) => NixpkgsProblem::UnresolvableSymlink { Err(io_error) => NixpkgsProblem::UnresolvableSymlink {
relative_package_dir: relative_package_dir.to_path_buf(), relative_package_dir: relative_package_dir.to_owned(),
subpath: subpath.to_path_buf(), subpath: subpath.to_owned(),
io_error: io_error.to_string(), io_error: io_error.to_string(),
} }
.into(), .into(),
@ -80,11 +81,12 @@ fn check_path(
nix_file_store, nix_file_store,
relative_package_dir, relative_package_dir,
absolute_package_dir, absolute_package_dir,
&subpath.join(entry.file_name()), // TODO: The relative_path crate doesn't seem to support OsStr
&subpath.join(entry.file_name().to_string_lossy().to_string()),
) )
}) })
.collect_vec() .collect_vec()
.with_context(|| format!("Error while recursing into {}", subpath.display()))?, .with_context(|| format!("Error while recursing into {}", subpath))?,
) )
} else if path.is_file() { } else if path.is_file() {
// Only check Nix files // Only check Nix files
@ -96,7 +98,7 @@ fn check_path(
absolute_package_dir, absolute_package_dir,
subpath, subpath,
) )
.with_context(|| format!("Error while checking Nix file {}", subpath.display()))? .with_context(|| format!("Error while checking Nix file {}", subpath))?
} else { } else {
Success(()) Success(())
} }
@ -105,7 +107,7 @@ fn check_path(
} }
} else { } else {
// This should never happen, git doesn't support other file types // This should never happen, git doesn't support other file types
anyhow::bail!("Unsupported file type for path {}", subpath.display()); anyhow::bail!("Unsupported file type for path {}", subpath);
}) })
} }
@ -113,11 +115,11 @@ fn check_path(
/// directory /// directory
fn check_nix_file( fn check_nix_file(
nix_file_store: &mut NixFileStore, nix_file_store: &mut NixFileStore,
relative_package_dir: &Path, relative_package_dir: &RelativePath,
absolute_package_dir: &Path, absolute_package_dir: &Path,
subpath: &Path, subpath: &RelativePath,
) -> validation::Result<()> { ) -> validation::Result<()> {
let path = absolute_package_dir.join(subpath); let path = subpath.to_path(absolute_package_dir);
let nix_file = nix_file_store.get(&path)?; let nix_file = nix_file_store.get(&path)?;
@ -135,29 +137,29 @@ fn check_nix_file(
match nix_file.static_resolve_path(path, absolute_package_dir) { match nix_file.static_resolve_path(path, absolute_package_dir) {
ResolvedPath::Interpolated => NixpkgsProblem::PathInterpolation { ResolvedPath::Interpolated => NixpkgsProblem::PathInterpolation {
relative_package_dir: relative_package_dir.to_path_buf(), relative_package_dir: relative_package_dir.to_owned(),
subpath: subpath.to_path_buf(), subpath: subpath.to_owned(),
line, line,
text, text,
} }
.into(), .into(),
ResolvedPath::SearchPath => NixpkgsProblem::SearchPath { ResolvedPath::SearchPath => NixpkgsProblem::SearchPath {
relative_package_dir: relative_package_dir.to_path_buf(), relative_package_dir: relative_package_dir.to_owned(),
subpath: subpath.to_path_buf(), subpath: subpath.to_owned(),
line, line,
text, text,
} }
.into(), .into(),
ResolvedPath::Outside => NixpkgsProblem::OutsidePathReference { ResolvedPath::Outside => NixpkgsProblem::OutsidePathReference {
relative_package_dir: relative_package_dir.to_path_buf(), relative_package_dir: relative_package_dir.to_owned(),
subpath: subpath.to_path_buf(), subpath: subpath.to_owned(),
line, line,
text, text,
} }
.into(), .into(),
ResolvedPath::Unresolvable(e) => NixpkgsProblem::UnresolvablePathReference { ResolvedPath::Unresolvable(e) => NixpkgsProblem::UnresolvablePathReference {
relative_package_dir: relative_package_dir.to_path_buf(), relative_package_dir: relative_package_dir.to_owned(),
subpath: subpath.to_path_buf(), subpath: subpath.to_owned(),
line, line,
text, text,
io_error: e.to_string(), io_error: e.to_string(),

16
pkgs/test/nixpkgs-check-by-name/src/structure.rs
View file

@ -7,8 +7,9 @@ use crate::NixFileStore;
use itertools::concat; use itertools::concat;
use lazy_static::lazy_static; use lazy_static::lazy_static;
use regex::Regex; use regex::Regex;
use relative_path::RelativePathBuf;
use std::fs::DirEntry; use std::fs::DirEntry;
use std::path::{Path, PathBuf}; use std::path::Path;
lazy_static! { lazy_static! {
static ref SHARD_NAME_REGEX: Regex = Regex::new(r"^[a-z0-9_-]{1,2}$").unwrap(); static ref SHARD_NAME_REGEX: Regex = Regex::new(r"^[a-z0-9_-]{1,2}$").unwrap();
@ -21,15 +22,15 @@ pub fn shard_for_package(package_name: &str) -> String {
package_name.to_lowercase().chars().take(2).collect() package_name.to_lowercase().chars().take(2).collect()
} }
pub fn relative_dir_for_shard(shard_name: &str) -> PathBuf { pub fn relative_dir_for_shard(shard_name: &str) -> RelativePathBuf {
PathBuf::from(format!("{BASE_SUBPATH}/{shard_name}")) RelativePathBuf::from(format!("{BASE_SUBPATH}/{shard_name}"))
} }
pub fn relative_dir_for_package(package_name: &str) -> PathBuf { pub fn relative_dir_for_package(package_name: &str) -> RelativePathBuf {
relative_dir_for_shard(&shard_for_package(package_name)).join(package_name) relative_dir_for_shard(&shard_for_package(package_name)).join(package_name)
} }
pub fn relative_file_for_package(package_name: &str) -> PathBuf { pub fn relative_file_for_package(package_name: &str) -> RelativePathBuf {
relative_dir_for_package(package_name).join(PACKAGE_NIX_FILENAME) relative_dir_for_package(package_name).join(PACKAGE_NIX_FILENAME)
} }
@ -120,7 +121,8 @@ fn check_package(
) -> validation::Result<String> { ) -> validation::Result<String> {
let package_path = package_entry.path(); let package_path = package_entry.path();
let package_name = package_entry.file_name().to_string_lossy().into_owned(); let package_name = package_entry.file_name().to_string_lossy().into_owned();
let relative_package_dir = PathBuf::from(format!("{BASE_SUBPATH}/{shard_name}/{package_name}")); let relative_package_dir =
RelativePathBuf::from(format!("{BASE_SUBPATH}/{shard_name}/{package_name}"));
Ok(if !package_path.is_dir() { Ok(if !package_path.is_dir() {
NixpkgsProblem::PackageNonDir { NixpkgsProblem::PackageNonDir {
@ -174,7 +176,7 @@ fn check_package(
let result = result.and(references::check_references( let result = result.and(references::check_references(
nix_file_store, nix_file_store,
&relative_package_dir, &relative_package_dir,
&path.join(&relative_package_dir), &relative_package_dir.to_path(path),
)?); )?);
result.map(|_| package_name.clone()) result.map(|_| package_name.clone())