diff --git a/Cargo.lock b/Cargo.lock index e3e269e..d628a13 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -258,7 +258,7 @@ dependencies = [ [[package]] name = "nixpkgs-check-by-name" -version = "0.1.2" +version = "0.1.3" dependencies = [ "anyhow", "clap", diff --git a/Cargo.toml b/Cargo.toml index 9503e4f..9717282 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "nixpkgs-check-by-name" -version = "0.1.2" +version = "0.1.3" edition = "2021" [dependencies] diff --git a/src/main.rs b/src/main.rs index 25f9c72..c1b3fe9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -176,7 +176,7 @@ mod tests { test_nixpkgs( "case_sensitive", path, - "pkgs/by-name/fo: Duplicate case-sensitive package directories \"foO\" and \"foo\".\n\ + "- pkgs/by-name/fo: Duplicate case-sensitive package directories \"foO\" and \"foo\".\n\ This PR introduces the problems listed above. Please fix them before merging, \ otherwise the base branch would break.\n", ); diff --git a/src/nixpkgs_problem.rs b/src/nixpkgs_problem.rs index bccc9fe..f5046de 100644 --- a/src/nixpkgs_problem.rs +++ b/src/nixpkgs_problem.rs @@ -158,17 +158,17 @@ impl fmt::Display for NixpkgsProblem { ShardErrorKind::ShardNonDir => write!( f, - "{relative_shard_path}: This is a file, but it should be a directory.", + "- {relative_shard_path}: This is a file, but it should be a directory.", ), ShardErrorKind::InvalidShardName => write!( f, - "{relative_shard_path}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".", + "- {relative_shard_path}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".", ), ShardErrorKind::CaseSensitiveDuplicate { first, second } => write!( f, - "{relative_shard_path}: Duplicate case-sensitive package directories {first:?} and {second:?}.", + "- {relative_shard_path}: Duplicate case-sensitive package directories {first:?} and {second:?}.", ), } } @@ -181,28 +181,28 @@ impl fmt::Display for NixpkgsProblem { let relative_package_dir = structure::relative_dir_for_package(package_name); write!( f, - "{relative_package_dir}: This path is a file, but it should be a directory.", + "- {relative_package_dir}: This path is a file, but it should be a directory.", ) } PackageErrorKind::InvalidPackageName { invalid_package_name } => write!( f, - "{relative_package_dir}: Invalid package directory name \"{invalid_package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".", + "- {relative_package_dir}: Invalid package directory name \"{invalid_package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".", ), PackageErrorKind::IncorrectShard { correct_relative_package_dir } => write!( f, - "{relative_package_dir}: Incorrect directory location, should be {correct_relative_package_dir} instead.", + "- {relative_package_dir}: Incorrect directory location, should be {correct_relative_package_dir} instead.", ), PackageErrorKind::PackageNixNonExistent => write!( f, - "{relative_package_dir}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.", + "- {relative_package_dir}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.", ), PackageErrorKind::PackageNixDir => write!( f, - "{relative_package_dir}: \"{PACKAGE_NIX_FILENAME}\" must be a file.", + "- {relative_package_dir}: \"{PACKAGE_NIX_FILENAME}\" must be a file.", ), } } @@ -215,25 +215,25 @@ impl fmt::Display for NixpkgsProblem { let relative_package_file = structure::relative_file_for_package(attribute_name); write!( f, - "pkgs.{attribute_name}: This attribute is not defined but it should be defined automatically as {relative_package_file}", + "- pkgs.{attribute_name}: This attribute is not defined but it should be defined automatically as {relative_package_file}", ) } ByNameErrorKind::NonDerivation => { let relative_package_file = structure::relative_file_for_package(attribute_name); write!( f, - "pkgs.{attribute_name}: This attribute defined by {relative_package_file} is not a derivation", + "- pkgs.{attribute_name}: This attribute defined by {relative_package_file} is not a derivation", ) } ByNameErrorKind::InternalCallPackageUsed => write!( f, - "pkgs.{attribute_name}: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use.", + "- pkgs.{attribute_name}: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use.", ), ByNameErrorKind::CannotDetermineAttributeLocation => write!( f, - "pkgs.{attribute_name}: Cannot determine the location of this attribute using `builtins.unsafeGetAttrPos`.", + "- pkgs.{attribute_name}: Cannot determine the location of this attribute using `builtins.unsafeGetAttrPos`.", ), } } @@ -333,12 +333,12 @@ impl fmt::Display for NixpkgsProblem { PathErrorKind::OutsideSymlink => write!( f, - "{relative_package_dir}: Path {subpath} is a symlink pointing to a path outside the directory of that package.", + "- {relative_package_dir}: Path {subpath} is a symlink pointing to a path outside the directory of that package.", ), PathErrorKind::UnresolvableSymlink { io_error } => write!( f, - "{relative_package_dir}: Path {subpath} is a symlink which cannot be resolved: {io_error}.", + "- {relative_package_dir}: Path {subpath} is a symlink which cannot be resolved: {io_error}.", ), } }, @@ -353,22 +353,30 @@ impl fmt::Display for NixpkgsProblem { NixFileErrorKind::PathInterpolation => write!( f, - "{relative_package_dir}: File {subpath} at line {line} contains the path expression \"{text}\", which is not yet supported and may point outside the directory of that package.", + "- {relative_package_dir}: File {subpath} at line {line} contains the path expression \"{text}\", which is not yet supported and may point outside the directory of that package.", ), NixFileErrorKind::SearchPath => write!( f, - "{relative_package_dir}: File {subpath} at line {line} contains the nix search path expression \"{text}\" which may point outside the directory of that package.", + "- {relative_package_dir}: File {subpath} at line {line} contains the nix search path expression \"{text}\" which may point outside the directory of that package.", ), NixFileErrorKind::OutsidePathReference => - write!( + writedoc!( f, - "{relative_package_dir}: File {subpath} at line {line} contains the path expression \"{text}\" which may point outside the directory of that package.", + " + - {relative_package_dir}: File {subpath} at line {line} contains the path expression \"{text}\" which may point outside the directory of that package. + This is undesirable because it creates dependencies between internal paths, making it harder to reorganise Nixpkgs in the future. + Alternatives include: + - If you are creating a new version of a package with a common file between versions, consider following the recommendation in https://github.com/NixOS/nixpkgs/tree/master/pkgs/by-name#recommendation-for-new-packages-with-multiple-versions. + - If the path being referenced could be considered a stable interface with multiple uses, consider exposing it via a `pkgs` attribute, then taking it as a attribute argument in {PACKAGE_NIX_FILENAME}. + - If the path being referenced is internal and has multiple uses, consider passing the file as an explicit `callPackage` argument in `pkgs/top-level/all-packages.nix`. + - If the path being referenced is internal and will need to be modified independently of the original, consider copying it into the {relative_package_dir} directory. + " ), NixFileErrorKind::UnresolvablePathReference { io_error } => write!( f, - "{relative_package_dir}: File {subpath} at line {line} contains the path expression \"{text}\" which cannot be resolved: {io_error}.", + "- {relative_package_dir}: File {subpath} at line {line} contains the path expression \"{text}\" which cannot be resolved: {io_error}.", ), } }, diff --git a/tests/base-still-broken/expected b/tests/base-still-broken/expected index c25f06b..de80289 100644 --- a/tests/base-still-broken/expected +++ b/tests/base-still-broken/expected @@ -1,3 +1,3 @@ -pkgs/by-name/bar: This is a file, but it should be a directory. +- pkgs/by-name/bar: This is a file, but it should be a directory. The base branch is broken and still has above problems with this PR, which need to be fixed first. Consider reverting the PR that introduced these problems in order to prevent more failures of unrelated PRs. diff --git a/tests/broken-autocall/expected b/tests/broken-autocall/expected index 15b3e3f..cff69b1 100644 --- a/tests/broken-autocall/expected +++ b/tests/broken-autocall/expected @@ -1,2 +1,2 @@ -pkgs.foo: This attribute is not defined but it should be defined automatically as pkgs/by-name/fo/foo/package.nix +- pkgs.foo: This attribute is not defined but it should be defined automatically as pkgs/by-name/fo/foo/package.nix This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/incorrect-shard/expected b/tests/incorrect-shard/expected index 3b0146c..41d1610 100644 --- a/tests/incorrect-shard/expected +++ b/tests/incorrect-shard/expected @@ -1,2 +1,2 @@ -pkgs/by-name/aa/FOO: Incorrect directory location, should be pkgs/by-name/fo/FOO instead. +- pkgs/by-name/aa/FOO: Incorrect directory location, should be pkgs/by-name/fo/FOO instead. This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/internalCallPackage/expected b/tests/internalCallPackage/expected index b3d0c6f..13b6a61 100644 --- a/tests/internalCallPackage/expected +++ b/tests/internalCallPackage/expected @@ -1,2 +1,2 @@ -pkgs.foo: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use. +- pkgs.foo: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use. This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/invalid-package-name/expected b/tests/invalid-package-name/expected index 80f6e7d..1bb3ae8 100644 --- a/tests/invalid-package-name/expected +++ b/tests/invalid-package-name/expected @@ -1,2 +1,2 @@ -pkgs/by-name/fo/fo@: Invalid package directory name "fo@", must be ASCII characters consisting of a-z, A-Z, 0-9, "-" or "_". +- pkgs/by-name/fo/fo@: Invalid package directory name "fo@", must be ASCII characters consisting of a-z, A-Z, 0-9, "-" or "_". This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/invalid-shard-name/expected b/tests/invalid-shard-name/expected index 7ca9ff8..050507e 100644 --- a/tests/invalid-shard-name/expected +++ b/tests/invalid-shard-name/expected @@ -1,2 +1,2 @@ -pkgs/by-name/A: Invalid directory name "A", must be at most 2 ASCII characters consisting of a-z, 0-9, "-" or "_". +- pkgs/by-name/A: Invalid directory name "A", must be at most 2 ASCII characters consisting of a-z, 0-9, "-" or "_". This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/missing-package-nix/expected b/tests/missing-package-nix/expected index 1b67704..81631f2 100644 --- a/tests/missing-package-nix/expected +++ b/tests/missing-package-nix/expected @@ -1,2 +1,2 @@ -pkgs/by-name/fo/foo: Missing required "package.nix" file. +- pkgs/by-name/fo/foo: Missing required "package.nix" file. This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/multiple-failures/expected b/tests/multiple-failures/expected index c8fc550..3ed7f9c 100644 --- a/tests/multiple-failures/expected +++ b/tests/multiple-failures/expected @@ -1,14 +1,21 @@ -pkgs/by-name/A: Invalid directory name "A", must be at most 2 ASCII characters consisting of a-z, 0-9, "-" or "_". -pkgs/by-name/A/fo@: Invalid package directory name "fo@", must be ASCII characters consisting of a-z, A-Z, 0-9, "-" or "_". -pkgs/by-name/A/fo@: Path foo is a symlink which cannot be resolved: No such file or directory (os error 2). -pkgs/by-name/A/fo@: Path package.nix is a symlink pointing to a path outside the directory of that package. -pkgs/by-name/aa: This is a file, but it should be a directory. -pkgs/by-name/ba/bar: This path is a file, but it should be a directory. -pkgs/by-name/ba/baz: "package.nix" must be a file. -pkgs/by-name/ba/foo: Incorrect directory location, should be pkgs/by-name/fo/foo instead. -pkgs/by-name/ba/foo: File package.nix at line 4 contains the path expression "/bar" which cannot be resolved: No such file or directory (os error 2). -pkgs/by-name/ba/foo: File package.nix at line 5 contains the path expression "../." which may point outside the directory of that package. -pkgs/by-name/ba/foo: File package.nix at line 6 contains the nix search path expression "" which may point outside the directory of that package. -pkgs/by-name/ba/foo: File package.nix at line 7 contains the path expression "./${"test"}", which is not yet supported and may point outside the directory of that package. -pkgs/by-name/fo/foo: Missing required "package.nix" file. +- pkgs/by-name/A: Invalid directory name "A", must be at most 2 ASCII characters consisting of a-z, 0-9, "-" or "_". +- pkgs/by-name/A/fo@: Invalid package directory name "fo@", must be ASCII characters consisting of a-z, A-Z, 0-9, "-" or "_". +- pkgs/by-name/A/fo@: Path foo is a symlink which cannot be resolved: No such file or directory (os error 2). +- pkgs/by-name/A/fo@: Path package.nix is a symlink pointing to a path outside the directory of that package. +- pkgs/by-name/aa: This is a file, but it should be a directory. +- pkgs/by-name/ba/bar: This path is a file, but it should be a directory. +- pkgs/by-name/ba/baz: "package.nix" must be a file. +- pkgs/by-name/ba/foo: Incorrect directory location, should be pkgs/by-name/fo/foo instead. +- pkgs/by-name/ba/foo: File package.nix at line 4 contains the path expression "/bar" which cannot be resolved: No such file or directory (os error 2). +- pkgs/by-name/ba/foo: File package.nix at line 5 contains the path expression "../." which may point outside the directory of that package. + This is undesirable because it creates dependencies between internal paths, making it harder to reorganise Nixpkgs in the future. + Alternatives include: + - If you are creating a new version of a package with a common file between versions, consider following the recommendation in https://github.com/NixOS/nixpkgs/tree/master/pkgs/by-name#recommendation-for-new-packages-with-multiple-versions. + - If the path being referenced could be considered a stable interface with multiple uses, consider exposing it via a `pkgs` attribute, then taking it as a attribute argument in package.nix. + - If the path being referenced is internal and has multiple uses, consider passing the file as an explicit `callPackage` argument in `pkgs/top-level/all-packages.nix`. + - If the path being referenced is internal and will need to be modified independently of the original, consider copying it into the pkgs/by-name/ba/foo directory. + +- pkgs/by-name/ba/foo: File package.nix at line 6 contains the nix search path expression "" which may point outside the directory of that package. +- pkgs/by-name/ba/foo: File package.nix at line 7 contains the path expression "./${"test"}", which is not yet supported and may point outside the directory of that package. +- pkgs/by-name/fo/foo: Missing required "package.nix" file. This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/non-attrs/expected b/tests/non-attrs/expected index 1705d22..5de115b 100644 --- a/tests/non-attrs/expected +++ b/tests/non-attrs/expected @@ -1,2 +1,2 @@ -pkgs.nonDerivation: This attribute defined by pkgs/by-name/no/nonDerivation/package.nix is not a derivation +- pkgs.nonDerivation: This attribute defined by pkgs/by-name/no/nonDerivation/package.nix is not a derivation This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/non-derivation/expected b/tests/non-derivation/expected index 1705d22..5de115b 100644 --- a/tests/non-derivation/expected +++ b/tests/non-derivation/expected @@ -1,2 +1,2 @@ -pkgs.nonDerivation: This attribute defined by pkgs/by-name/no/nonDerivation/package.nix is not a derivation +- pkgs.nonDerivation: This attribute defined by pkgs/by-name/no/nonDerivation/package.nix is not a derivation This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/package-dir-is-file/expected b/tests/package-dir-is-file/expected index adee1d0..17be7f7 100644 --- a/tests/package-dir-is-file/expected +++ b/tests/package-dir-is-file/expected @@ -1,2 +1,2 @@ -pkgs/by-name/fo/foo: This path is a file, but it should be a directory. +- pkgs/by-name/fo/foo: This path is a file, but it should be a directory. This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/package-nix-dir/expected b/tests/package-nix-dir/expected index d03e1ec..a2c7634 100644 --- a/tests/package-nix-dir/expected +++ b/tests/package-nix-dir/expected @@ -1,2 +1,2 @@ -pkgs/by-name/fo/foo: "package.nix" must be a file. +- pkgs/by-name/fo/foo: "package.nix" must be a file. This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/ref-absolute/expected b/tests/ref-absolute/expected index a3125b5..eaeb85f 100644 --- a/tests/ref-absolute/expected +++ b/tests/ref-absolute/expected @@ -1,2 +1,2 @@ -pkgs/by-name/aa/aa: File package.nix at line 1 contains the path expression "/foo" which cannot be resolved: No such file or directory (os error 2). +- pkgs/by-name/aa/aa: File package.nix at line 1 contains the path expression "/foo" which cannot be resolved: No such file or directory (os error 2). This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/ref-escape/expected b/tests/ref-escape/expected index cc67423..4693999 100644 --- a/tests/ref-escape/expected +++ b/tests/ref-escape/expected @@ -1,2 +1,9 @@ -pkgs/by-name/aa/aa: File package.nix at line 1 contains the path expression "../." which may point outside the directory of that package. +- pkgs/by-name/aa/aa: File package.nix at line 1 contains the path expression "../." which may point outside the directory of that package. + This is undesirable because it creates dependencies between internal paths, making it harder to reorganise Nixpkgs in the future. + Alternatives include: + - If you are creating a new version of a package with a common file between versions, consider following the recommendation in https://github.com/NixOS/nixpkgs/tree/master/pkgs/by-name#recommendation-for-new-packages-with-multiple-versions. + - If the path being referenced could be considered a stable interface with multiple uses, consider exposing it via a `pkgs` attribute, then taking it as a attribute argument in package.nix. + - If the path being referenced is internal and has multiple uses, consider passing the file as an explicit `callPackage` argument in `pkgs/top-level/all-packages.nix`. + - If the path being referenced is internal and will need to be modified independently of the original, consider copying it into the pkgs/by-name/aa/aa directory. + This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/ref-nix-path/expected b/tests/ref-nix-path/expected index 5c8021a..c298827 100644 --- a/tests/ref-nix-path/expected +++ b/tests/ref-nix-path/expected @@ -1,2 +1,2 @@ -pkgs/by-name/aa/aa: File package.nix at line 1 contains the nix search path expression "" which may point outside the directory of that package. +- pkgs/by-name/aa/aa: File package.nix at line 1 contains the nix search path expression "" which may point outside the directory of that package. This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/ref-path-subexpr/expected b/tests/ref-path-subexpr/expected index 8fec8dc..2e1887a 100644 --- a/tests/ref-path-subexpr/expected +++ b/tests/ref-path-subexpr/expected @@ -1,2 +1,2 @@ -pkgs/by-name/aa/aa: File package.nix at line 1 contains the path expression "./${"test"}", which is not yet supported and may point outside the directory of that package. +- pkgs/by-name/aa/aa: File package.nix at line 1 contains the path expression "./${"test"}", which is not yet supported and may point outside the directory of that package. This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/shard-file/expected b/tests/shard-file/expected index 689cee4..70400c8 100644 --- a/tests/shard-file/expected +++ b/tests/shard-file/expected @@ -1,2 +1,2 @@ -pkgs/by-name/fo: This is a file, but it should be a directory. +- pkgs/by-name/fo: This is a file, but it should be a directory. This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/symlink-escape/expected b/tests/symlink-escape/expected index cd555c6..1e7dbf2 100644 --- a/tests/symlink-escape/expected +++ b/tests/symlink-escape/expected @@ -1,2 +1,2 @@ -pkgs/by-name/fo/foo: Path package.nix is a symlink pointing to a path outside the directory of that package. +- pkgs/by-name/fo/foo: Path package.nix is a symlink pointing to a path outside the directory of that package. This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/symlink-invalid/expected b/tests/symlink-invalid/expected index 1b06bcf..9549d74 100644 --- a/tests/symlink-invalid/expected +++ b/tests/symlink-invalid/expected @@ -1,2 +1,2 @@ -pkgs/by-name/fo/foo: Path foo is a symlink which cannot be resolved: No such file or directory (os error 2). +- pkgs/by-name/fo/foo: Path foo is a symlink which cannot be resolved: No such file or directory (os error 2). This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/tests/unknown-location/expected b/tests/unknown-location/expected index 608843d..fb671bb 100644 --- a/tests/unknown-location/expected +++ b/tests/unknown-location/expected @@ -1,2 +1,2 @@ -pkgs.foo: Cannot determine the location of this attribute using `builtins.unsafeGetAttrPos`. +- pkgs.foo: Cannot determine the location of this attribute using `builtins.unsafeGetAttrPos`. This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.