Skip to content

Eval fixes from release-attrpaths-superset.nix optimization (part 2) #271123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
infinisil merged 5 commits into from
Dec 1, 2023
Merged

Eval fixes from release-attrpaths-superset.nix optimization (part 2) #271123

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f8ea911
lib.customisation.callPackageWith: use throw, not abort
Nov 22, 2023
bffbf8b
test.cuda: fix broken eval
Nov 23, 2023
e30975f
darwin.iosSdkPkgs: fix broken eval
Nov 23, 2023
72985fe
cudaPackages_10.cudaFlags: fix broken eval
Nov 23, 2023
310cda5
mkAdoptopenjdk: allow missing {jre,jdk}-openj9 attribute
Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/customisation.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,9 @@ rec {
# Only show the error for the first missing argument
error = errorForArg missingArgs.${head (attrNames missingArgs)};

in if missingArgs == {} then makeOverridable f allArgs else abort error;
in if missingArgs == {}
then makeOverridable f allArgs
else throw "lib.customisation.callPackageWith: ${error}";

@lilyinstarlight lilyinstarlight Jan 4, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the motivation for this specific change? It makes it so nix-env -qa no longer fails and just silently drops the attrpath when querying pacakges that have missing arguments (and therefore ofborg and other CI relies on the previous behavior to catch missing argument errors during outpath calculation)

This still smells like a bug in nix-env regardless to me, so this bit in Nixpkgs probably doesn't need to be changed back, but I'm still trying to trace down in that code where/why this throw would be ignored rather than propagated as an eval error

@lilyinstarlight lilyinstarlight Jan 4, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, this is intended behavior in Nix:

Throw an error message s. This usually aborts Nix expression
evaluation, but in nix-env -qa and other commands that try to
evaluate a set of derivations to get information about those
derivations, a derivation that throws an error is silently skipped
(which is not the case for abort).

https://nixos.org/manual/nix/stable/language/builtins.html#builtins-throw

@infinisil infinisil Jan 4, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh damn yeah. Probably the entire reason this is in nix-env at all is because we historically haven't had anything like this PR, which would've prevented packages from not being evaluatable.

@infinisil infinisil Jan 4, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert and eval fixes in #278777



/* Like callPackage, but for a function that returns an attribute
Expand Down