Skip to content

Commit

Permalink
tests.nixpkgs-check-by-name: Don't enforce for fixed evals
Browse files Browse the repository at this point in the history
Stops enforcing that packages whose evaluation gets fixed have to be
moved to `pkgs/by-name`.

This didn't really cause problems before, but I don't think it's great
behavior, and now that NonApplicable is a thing, we can easily make this
work, whereas before it would've been a larger change.
  • Loading branch information
infinisil committed Jan 22, 2024
1 parent 034ec01 commit fdccad8
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 11 deletions.
27 changes: 16 additions & 11 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,19 +202,24 @@ pub fn check_values(
})
}
NonByName(EvalFailure) => {
// This is a bit of an odd case: We don't even _know_ whether this attribute
// would qualify for using pkgs/by-name. We can either:
// - Assume it's not using pkgs/by-name, which has the problem that if a
// package evaluation gets broken temporarily, the fix can remove it from
// pkgs/by-name again
// - Assume it's using pkgs/by-name already, which has the problem that if a
// package evaluation gets broken temporarily, fixing it requires a move to
// pkgs/by-name
// We choose the latter, since we want to move towards pkgs/by-name, not away
// from it
// We don't know anything about this attribute really
Success(ratchet::Package {
// We'll assume that we can't remove any manual definitions, which has the
// minimal drawback that if there was a manual definition that could've
// been removed, fixing the package requires removing the definition, no
// big deal, that's a minor edit.
manual_definition: Tight,
uses_by_name: Tight,

// Regarding whether this attribute could `pkgs/by-name`, we don't really
// know, so return NonApplicable, which has the effect that if a
// package evaluation gets broken temporarily, the fix can remove it from
// pkgs/by-name again. For now this isn't our problem, but in the future we
// might have another check to enforce that evaluation must not be broken.
// The alternative of assuming that it's using `pkgs/by-name` already
// has the problem that if a package evaluation gets broken temporarily,
// fixing it requires a move to pkgs/by-name, which could happen more
// often and isn't really justified.
uses_by_name: NonApplicable,
})
}
ByName(Missing) => NixpkgsProblem::UndefinedAttr {
Expand Down
2 changes: 2 additions & 0 deletions tests/no-eval/all-packages.nix
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
self: super: {
iDontEval = throw "I don't eval";

futureEval = self.callPackage ({ someDrv }: someDrv) { };
}
3 changes: 3 additions & 0 deletions tests/no-eval/base/all-packages.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
self: super: {
futureEval = throw "foo";
}
1 change: 1 addition & 0 deletions tests/no-eval/base/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import <test-nixpkgs> { root = ./.; }
Empty file.

0 comments on commit fdccad8

Please sign in to comment.