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

tests.nixpkgs-check-by-name: Improve errors relating to the base branch

Browse source
This commit is contained in:
Silvan Mosberger 2024-02-23 01:01:57 +01:00
parent 75cdccd6c4
commit db7562e886
55 changed files with 110 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

94
pkgs/test/nixpkgs-check-by-name/src/main.rs
View file

@ -47,14 +47,8 @@ pub struct Args {
fn main() -> ExitCode {
let args = Args::parse();
match process(&args.base, &args.nixpkgs, false, &mut io::stderr()) {
Ok(true) => {
eprintln!("{}", "Validated successfully".green());
ExitCode::SUCCESS
}
Ok(false) => {
eprintln!("{}", "Validation failed, see above errors".yellow());
ExitCode::from(1)
}
Ok(true) => ExitCode::SUCCESS,
Ok(false) => ExitCode::from(1),
Err(e) => {
eprintln!("{} {:#}", "I/O error: ".yellow(), e);
ExitCode::from(2)
@ -82,29 +76,58 @@ pub fn process<W: io::Write>(
keep_nix_path: bool,
error_writer: &mut W,
) -> anyhow::Result<bool> {
// Check the main Nixpkgs first
let main_result = check_nixpkgs(main_nixpkgs, keep_nix_path, error_writer)?;
let check_result = main_result.result_map(|nixpkgs_version| {
// If the main Nixpkgs doesn't have any problems, run the ratchet checks against the base
// Nixpkgs
check_nixpkgs(base_nixpkgs, keep_nix_path, error_writer)?.result_map(
|base_nixpkgs_version| {
Ok(ratchet::Nixpkgs::compare(
base_nixpkgs_version,
nixpkgs_version,
))
},
)
})?;
match check_result {
Failure(errors) => {
// TODO: Run in parallel
let base_result = check_nixpkgs(base_nixpkgs, keep_nix_path)?;
let main_result = check_nixpkgs(main_nixpkgs, keep_nix_path)?;
match (base_result, main_result) {
(Failure(_), Failure(errors)) => {
// Base branch fails and the PR doesn't fix it and may also introduce additional problems
for error in errors {
writeln!(error_writer, "{}", error.to_string().red())?
}
writeln!(error_writer, "{}", "The base branch is broken and still has above problems with this PR, these need to be fixed first.\nConsider reverting the PR that introduced these problems in order to prevent more failures of unrelated PRs.".yellow())?;
Ok(false)
}
Success(()) => Ok(true),
(Failure(_), Success(_)) => {
// Base branch fails, but the PR fixes it
writeln!(
error_writer,
"{}",
"The base branch was broken, but this PR fixes it, nice job!".green()
)?;
Ok(true)
}
(Success(_), Failure(errors)) => {
// Base branch succeeds, the PR breaks it
for error in errors {
writeln!(error_writer, "{}", error.to_string().red())?
}
writeln!(
error_writer,
"{}",
"This PR introduces the above problems, merging would break the base branch"
.yellow()
)?;
Ok(false)
}
(Success(base), Success(main)) => {
// Both base and main branch succeed, check ratchet state
match ratchet::Nixpkgs::compare(base, main) {
Failure(errors) => {
for error in errors {
writeln!(error_writer, "{}", error.to_string().red())?
}
writeln!(error_writer, "{}", "This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch".yellow())?;
Ok(false)
}
Success(()) => {
writeln!(error_writer, "{}", "Validated successfully".green())?;
Ok(true)
}
}
}
}
}
@ -113,10 +136,9 @@ pub fn process<W: io::Write>(
/// This does not include ratchet checks, see ../README.md#ratchet-checks
/// Instead a `ratchet::Nixpkgs` value is returned, whose `compare` method allows performing the
/// ratchet check against another result.
pub fn check_nixpkgs<W: io::Write>(
pub fn check_nixpkgs(
nixpkgs_path: &Path,
keep_nix_path: bool,
error_writer: &mut W,
) -> validation::Result<ratchet::Nixpkgs> {
let mut nix_file_store = NixFileStore::default();
@ -129,11 +151,7 @@ pub fn check_nixpkgs<W: io::Write>(
})?;
if !nixpkgs_path.join(utils::BASE_SUBPATH).exists() {
writeln!(
error_writer,
"Given Nixpkgs path does not contain a {} subdirectory, no check necessary.",
utils::BASE_SUBPATH
)?;
// No pkgs/by-name directory, always valid
Success(ratchet::Nixpkgs::default())
} else {
check_structure(&nixpkgs_path, &mut nix_file_store)?.result_map(|package_names|
@ -163,8 +181,8 @@ mod tests {
continue;
}
let expected_errors =
fs::read_to_string(path.join("expected")).unwrap_or(String::new());
let expected_errors = fs::read_to_string(path.join("expected"))
.expect("No expected file for test {name}");
test_nixpkgs(&name, &path, &expected_errors)?;
}
@ -201,7 +219,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\".\nThis PR introduces the above problems, merging would break the base branch\n",
)?;
Ok(())
@ -225,7 +243,11 @@ mod tests {
let tmpdir = temp_root.path().join("symlinked");
temp_env::with_var("TMPDIR", Some(&tmpdir), || {
test_nixpkgs("symlinked_tmpdir", Path::new("tests/success"), "")
test_nixpkgs(
"symlinked_tmpdir",
Path::new("tests/success"),
"Validated successfully\n",
)
})
}

1
pkgs/test/nixpkgs-check-by-name/tests/aliases/expected Normal file
View file

@ -0,0 +1 @@
Validated successfully

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

@ -6,3 +6,4 @@
It is defined in all-packages.nix:5 as
foo = self.alt.callPackage ({ }: self.someDrv) { };
This PR introduces the above problems, merging would break the base branch

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

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

0
pkgs/test/nixpkgs-check-by-name/tests/base-fixed/base/pkgs/by-name/foo Normal file
View file

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

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

1
pkgs/test/nixpkgs-check-by-name/tests/base-fixed/expected Normal file
View file

@ -0,0 +1 @@
The base branch was broken, but this PR fixes it, nice job!

0
pkgs/test/nixpkgs-check-by-name/tests/base-fixed/pkgs/by-name/README.md Normal file
View file

1
pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/base/default.nix Normal file
View file

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

0
pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/base/pkgs/by-name/foo Normal file
View file

1
pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/default.nix Normal file
View file

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

3
pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/expected Normal file
View file

@ -0,0 +1,3 @@
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, these need to be fixed first.
Consider reverting the PR that introduced these problems in order to prevent more failures of unrelated PRs.

0
pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/pkgs/by-name/bar Normal file
View file

1
pkgs/test/nixpkgs-check-by-name/tests/broken-autocall/expected
View file

@ -1 +1,2 @@
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 above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/expected Normal file
View file

@ -0,0 +1 @@
Validated successfully

1
pkgs/test/nixpkgs-check-by-name/tests/empty-base/expected Normal file
View file

@ -0,0 +1 @@
Validated successfully

1
pkgs/test/nixpkgs-check-by-name/tests/incorrect-shard/expected
View file

@ -1 +1,2 @@
pkgs/by-name/aa/FOO: Incorrect directory location, should be pkgs/by-name/fo/FOO instead.
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/internalCallPackage/expected
View file

@ -1 +1,2 @@
pkgs.foo: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use.
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/invalid-package-name/expected
View file

@ -1 +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 "_".
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/invalid-shard-name/expected
View file

@ -1 +1,2 @@
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 above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected
View file

@ -1,2 +1,3 @@
pkgs.noEval: 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/noEval/package.nix { ... }` with a non-empty second argument.
pkgs.onlyMove: 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/on/onlyMove/package.nix { ... }` with a non-empty second argument.
This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/missing-package-nix/expected
View file

@ -1 +1,2 @@
pkgs/by-name/fo/foo: Missing required "package.nix" file.
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/expected
View file

@ -2,3 +2,4 @@ pkgs.foo1: This top-level package was previously defined in pkgs/by-name/fo/foo1
pkgs.foo2: This top-level package was previously defined in pkgs/by-name/fo/foo2/package.nix, but is now manually defined as `callPackage ./without-config.nix { }` (e.g. in `pkgs/top-level/all-packages.nix`). Please move the package back and remove the manual `callPackage`.
pkgs.foo3: This top-level package was previously defined in pkgs/by-name/fo/foo3/package.nix, but is now manually defined as `callPackage ... { ... }` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files.
pkgs.foo4: This top-level package was previously defined in pkgs/by-name/fo/foo4/package.nix, but is now manually defined as `callPackage ./with-config.nix { ... }` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files.
This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/multiple-failures/expected
View file

@ -11,3 +11,4 @@ pkgs/by-name/ba/foo: File package.nix at line 3 contains the path expression "..
pkgs/by-name/ba/foo: File package.nix at line 4 contains the nix search path expression "<nixpkgs>" which may point outside the directory of that package.
pkgs/by-name/ba/foo: File package.nix at line 5 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 above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/expected
View file

@ -2,3 +2,4 @@ pkgs.new1: This is a new top-level package of the form `callPackage ... { }`. Pl
pkgs.new2: This is a new top-level package of the form `callPackage ./without-config.nix { }`. Please define it in pkgs/by-name/ne/new2/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore.
pkgs.new3: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/ne/new3/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is not `{ }`, the manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is still needed.
pkgs.new4: This is a new top-level package of the form `callPackage ./with-config.nix { }`. Please define it in pkgs/by-name/ne/new4/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is not `{ }`, the manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is still needed.
This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch

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

@ -1 +1 @@
Given Nixpkgs path does not contain a pkgs/by-name subdirectory, no check necessary.
Validated successfully

1
pkgs/test/nixpkgs-check-by-name/tests/no-eval/expected Normal file
View file

@ -0,0 +1 @@
Validated successfully

1
pkgs/test/nixpkgs-check-by-name/tests/non-attrs/expected
View file

@ -1 +1,2 @@
pkgs.nonDerivation: This attribute defined by pkgs/by-name/no/nonDerivation/package.nix is not a derivation
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/non-derivation/expected
View file

@ -1 +1,2 @@
pkgs.nonDerivation: This attribute defined by pkgs/by-name/no/nonDerivation/package.nix is not a derivation
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/expected
View file

@ -6,3 +6,4 @@
It is defined in all-packages.nix:4 as
foo = self.bar;
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/one-letter/expected Normal file
View file

@ -0,0 +1 @@
Validated successfully

1
pkgs/test/nixpkgs-check-by-name/tests/only-callPackage-derivations/expected Normal file
View file

@ -0,0 +1 @@
Validated successfully

1
pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected
View file

@ -6,3 +6,4 @@
It is defined in all-packages.nix:2:3 as
nonDerivation = callPackage ./someDrv.nix { /* ... */ };
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg-gradual/expected
View file

@ -0,0 +1 @@
Validated successfully

1
pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected
View file

@ -1 +1,2 @@
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.
This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected
View file

@ -6,3 +6,4 @@
It is defined in all-packages.nix:2 as
nonDerivation = self.someDrv;
This PR introduces the above problems, merging would break the base branch

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

@ -6,3 +6,4 @@
It is defined in all-packages.nix:2 as
nonDerivation = self.callPackage ({ someDrv }: someDrv) { };
This PR introduces the above problems, merging would break the base branch

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

@ -6,3 +6,4 @@
It is defined in all-packages.nix:3 as
foo = self.callPackage ({ someDrv, someFlag }: someDrv) { someFlag = true; };
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/override-success/expected Normal file
View file

@ -0,0 +1 @@
Validated successfully

1
pkgs/test/nixpkgs-check-by-name/tests/package-dir-is-file/expected
View file

@ -1 +1,2 @@
pkgs/by-name/fo/foo: This path is a file, but it should be a directory.
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/package-nix-dir/expected
View file

@ -1 +1,2 @@
pkgs/by-name/fo/foo: "package.nix" must be a file.
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/package-variants/expected Normal file
View file

@ -0,0 +1 @@
Validated successfully

1
pkgs/test/nixpkgs-check-by-name/tests/ref-absolute/expected
View file

@ -1 +1,2 @@
pkgs/by-name/aa/aa: File package.nix at line 2 contains the path expression "/foo" which cannot be resolved: No such file or directory (os error 2).
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/ref-escape/expected
View file

@ -1 +1,2 @@
pkgs/by-name/aa/aa: File package.nix at line 2 contains the path expression "../." which may point outside the directory of that package.
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/ref-nix-path/expected
View file

@ -1 +1,2 @@
pkgs/by-name/aa/aa: File package.nix at line 2 contains the nix search path expression "<nixpkgs>" which may point outside the directory of that package.
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/ref-path-subexpr/expected
View file

@ -1 +1,2 @@
pkgs/by-name/aa/aa: File package.nix at line 2 contains the path expression "./${"test"}", which is not yet supported and may point outside the directory of that package.
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/ref-success/expected Normal file
View file

@ -0,0 +1 @@
Validated successfully

1
pkgs/test/nixpkgs-check-by-name/tests/shard-file/expected
View file

@ -1 +1,2 @@
pkgs/by-name/fo: This is a file, but it should be a directory.
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected
View file

@ -2,3 +2,4 @@ pkgs.a: This attribute is manually defined (most likely in pkgs/top-level/all-pa
pkgs.b: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/b/b/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore.
pkgs.c: 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/c/c/package.nix { ... }` with a non-empty second argument.
pkgs.d: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/d/d/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore.
This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/success/expected Normal file
View file

@ -0,0 +1 @@
Validated successfully

1
pkgs/test/nixpkgs-check-by-name/tests/symlink-escape/expected
View file

@ -1 +1,2 @@
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 above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/symlink-invalid/expected
View file

@ -1 +1,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 above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/unknown-location/expected
View file

@ -1 +1,2 @@
pkgs.foo: Cannot determine the location of this attribute using `builtins.unsafeGetAttrPos`.
This PR introduces the above problems, merging would break the base branch

1
pkgs/test/nixpkgs-check-by-name/tests/uppercase/expected Normal file
View file

@ -0,0 +1 @@
Validated successfully

1
pkgs/test/nixpkgs-check-by-name/tests/with-readme/expected Normal file
View file

@ -0,0 +1 @@
Validated successfully