tests.nixpkgs-check-by-name: Fix absolute path in errors

This commit is contained in:
Silvan Mosberger 2024-03-01 01:42:28 +01:00
parent 2e8d778994
commit f056449c46
5 changed files with 43 additions and 23 deletions

View file

@ -183,6 +183,7 @@ pub fn check_values(
Attribute::NonByName(non_by_name_attribute) => handle_non_by_name_attribute( Attribute::NonByName(non_by_name_attribute) => handle_non_by_name_attribute(
nixpkgs_path, nixpkgs_path,
nix_file_store, nix_file_store,
&attribute_name,
non_by_name_attribute, non_by_name_attribute,
)?, )?,
Attribute::ByName(by_name_attribute) => by_name( Attribute::ByName(by_name_attribute) => by_name(
@ -431,6 +432,7 @@ fn by_name_override(
fn handle_non_by_name_attribute( fn handle_non_by_name_attribute(
nixpkgs_path: &Path, nixpkgs_path: &Path,
nix_file_store: &mut NixFileStore, nix_file_store: &mut NixFileStore,
attribute_name: &str,
non_by_name_attribute: NonByNameAttribute, non_by_name_attribute: NonByNameAttribute,
) -> validation::Result<ratchet::Package> { ) -> validation::Result<ratchet::Package> {
use ratchet::RatchetState::*; use ratchet::RatchetState::*;
@ -481,11 +483,17 @@ fn handle_non_by_name_attribute(
location: Some(location), location: Some(location),
}) = non_by_name_attribute { }) = non_by_name_attribute {
// Parse the Nix file in the location and figure out whether it's an // Parse the Nix file in the location
// attribute definition of the form `= callPackage <arg1> <arg2>`, let nix_file = nix_file_store.get(&location.file)?;
// The relative path of the Nix file, for error messages
let relative_location_file = location.relative_file(nixpkgs_path).with_context(|| {
format!("Failed to resolve the file where attribute {attribute_name} is defined")
})?;
// Figure out whether it's an attribute definition of the form `= callPackage <arg1> <arg2>`,
// returning the arguments if so. // returning the arguments if so.
let (optional_syntactic_call_package, _definition) = nix_file_store let (optional_syntactic_call_package, _definition) = nix_file
.get(&location.file)?
.call_package_argument_info_at( .call_package_argument_info_at(
location.line, location.line,
location.column, location.column,
@ -493,7 +501,10 @@ fn handle_non_by_name_attribute(
// strips the absolute Nixpkgs path from it, such that // strips the absolute Nixpkgs path from it, such that
// syntactic_call_package.relative_path is relative to Nixpkgs // syntactic_call_package.relative_path is relative to Nixpkgs
nixpkgs_path 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 // At this point, we completed two different checks for whether it's a
// `callPackage` // `callPackage`
@ -529,7 +540,7 @@ fn handle_non_by_name_attribute(
_ => { _ => {
// Otherwise, the path is outside `pkgs/by-name`, which means it can be // Otherwise, the path is outside `pkgs/by-name`, which means it can be
// migrated // migrated
Loose((syntactic_call_package, location.file)) Loose((syntactic_call_package, relative_location_file))
} }
} }
} }

View file

@ -28,13 +28,22 @@ let
lib = import <test-nixpkgs/lib>; lib = import <test-nixpkgs/lib>;
# The base fixed-point function to populate the resulting attribute set # The base fixed-point function to populate the resulting attribute set
pkgsFun = self: { pkgsFun =
inherit lib; self: {
newScope = extra: lib.callPackageWith (self // extra); inherit lib;
callPackage = self.newScope { }; newScope = extra: lib.callPackageWith (self // extra);
callPackages = lib.callPackagesWith self; callPackage = self.newScope { };
someDrv = { type = "derivation"; }; callPackages = lib.callPackagesWith self;
}; }
# This mapAttrs is a very hacky workaround necessary because for all attributes defined in Nixpkgs,
# the files that define them are verified to be within Nixpkgs.
# This is usually a very safe assumption, but it fails in these tests for someDrv,
# because it's technically defined outside the Nixpkgs directories of each test case.
# By using `mapAttrs`, `builtins.unsafeGetAttrPos` just returns `null`,
# which then doesn't trigger this check
// lib.mapAttrs (name: value: value) {
someDrv = { type = "derivation"; };
};
baseDirectory = root + "/pkgs/by-name"; baseDirectory = root + "/pkgs/by-name";

View file

@ -1,13 +1,13 @@
- Attribute `pkgs.foo1` was previously defined in pkgs/by-name/fo/foo1/package.nix, but is now manually defined as `callPackage ... { /* ... */ }` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/all-packages.nix. - Attribute `pkgs.foo1` was previously defined in pkgs/by-name/fo/foo1/package.nix, but is now manually defined as `callPackage ... { /* ... */ }` in all-packages.nix.
Please move the package back and remove the manual `callPackage`. Please move the package back and remove the manual `callPackage`.
- Attribute `pkgs.foo2` was previously defined in pkgs/by-name/fo/foo2/package.nix, but is now manually defined as `callPackage ./without-config.nix { /* ... */ }` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/all-packages.nix. - Attribute `pkgs.foo2` was previously defined in pkgs/by-name/fo/foo2/package.nix, but is now manually defined as `callPackage ./without-config.nix { /* ... */ }` in all-packages.nix.
Please move the package back and remove the manual `callPackage`. Please move the package back and remove the manual `callPackage`.
- Attribute `pkgs.foo3` was previously defined in pkgs/by-name/fo/foo3/package.nix, but is now manually defined as `callPackage ... { ... }` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/all-packages.nix. - Attribute `pkgs.foo3` was previously defined in pkgs/by-name/fo/foo3/package.nix, but is now manually defined as `callPackage ... { ... }` in all-packages.nix.
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.
- Attribute `pkgs.foo4` was previously defined in pkgs/by-name/fo/foo4/package.nix, but is now manually defined as `callPackage ./with-config.nix { ... }` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/all-packages.nix. - Attribute `pkgs.foo4` was previously defined in pkgs/by-name/fo/foo4/package.nix, but is now manually defined as `callPackage ./with-config.nix { ... }` in all-packages.nix.
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.
This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch. This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch.

View file

@ -1,21 +1,21 @@
- Attribute `pkgs.new1` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`. - Attribute `pkgs.new1` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`.
Please define it in pkgs/by-name/ne/new1/package.nix instead. Please define it in pkgs/by-name/ne/new1/package.nix instead.
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 /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/all-packages.nix is needed anymore. Since the second `callPackage` argument is `{ }`, no manual `callPackage` in all-packages.nix is needed anymore.
- Attribute `pkgs.new2` is a new top-level package using `pkgs.callPackage ./without-config.nix { /* ... */ }`. - Attribute `pkgs.new2` is a new top-level package using `pkgs.callPackage ./without-config.nix { /* ... */ }`.
Please define it in pkgs/by-name/ne/new2/package.nix instead. Please define it in pkgs/by-name/ne/new2/package.nix instead.
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 /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/all-packages.nix is needed anymore. Since the second `callPackage` argument is `{ }`, no manual `callPackage` in all-packages.nix is needed anymore.
- Attribute `pkgs.new3` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`. - Attribute `pkgs.new3` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`.
Please define it in pkgs/by-name/ne/new3/package.nix instead. Please define it in pkgs/by-name/ne/new3/package.nix instead.
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 /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/all-packages.nix is still needed. Since the second `callPackage` argument is not `{ }`, the manual `callPackage` in all-packages.nix is still needed.
- Attribute `pkgs.new4` is a new top-level package using `pkgs.callPackage ./with-config.nix { /* ... */ }`. - Attribute `pkgs.new4` is a new top-level package using `pkgs.callPackage ./with-config.nix { /* ... */ }`.
Please define it in pkgs/by-name/ne/new4/package.nix instead. Please define it in pkgs/by-name/ne/new4/package.nix instead.
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 /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/all-packages.nix is still needed. Since the second `callPackage` argument is not `{ }`, the manual `callPackage` in all-packages.nix is still needed.
This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch. This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch.

View file

@ -9,7 +9,7 @@
- Attribute `pkgs.b` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`. - Attribute `pkgs.b` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`.
Please define it in pkgs/by-name/b/b/package.nix instead. Please define it in pkgs/by-name/b/b/package.nix instead.
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 /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/all-packages.nix is needed anymore. Since the second `callPackage` argument is `{ }`, no manual `callPackage` in all-packages.nix is needed anymore.
- Because pkgs/by-name/c/c exists, the attribute `pkgs.c` must be defined like - Because pkgs/by-name/c/c exists, the attribute `pkgs.c` must be defined like
@ -22,6 +22,6 @@
- Attribute `pkgs.d` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`. - Attribute `pkgs.d` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`.
Please define it in pkgs/by-name/d/d/package.nix instead. Please define it in pkgs/by-name/d/d/package.nix instead.
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 /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/all-packages.nix is needed anymore. Since the second `callPackage` argument is `{ }`, no manual `callPackage` in all-packages.nix is needed anymore.
This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch. This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch.