lib.callPackageWith: Use abort again instead of throw and fix evaluation errors caused by it
#278777
Conversation
github-actions
bot
added
6.topic: python
6.topic: mate
| @@ -53,7 +53,7 @@ final: _: { | |||
| autoAddCudaCompatRunpathHook = | |||
| final.callPackage | |||
| ( | |||
| {makeSetupHook, cuda_compat}: | |||
| {makeSetupHook, cuda_compat ? throw "autoAddCudaCompatRunpathHook: No cuda_compat for CUDA ${final.cudaMajorMinorVersion}" }: | |||
There was a problem hiding this comment.
So to make sure I understand correctly, it's ok to use throw here because we do want to prevent Ofborg from evaluating the hook on platforms that don't support cuda_compat? Or should this be an abort as well?
delroth
added
the
12.approvals: 1
Was broken in 9fddd79 but not caught due to NixOS#271123 (comment)
This was not caught when the package was introduced because of NixOS#271123 (comment)
Was broken by feb114f And not caught due to NixOS#271123 (comment)
Was broken since 7dd3469 but not caught due to NixOS#271123 (comment)
Was broken since introduced, but not caught due to NixOS#271123 (comment)
|
I just rebased and pushed another commit to fix another failure that's since been introduced on master. I'll merge this PR as soon as ofborg passes so we fix this for PRs going forward. Note that even after merging, new failures on master may be introduced from PRs that already passed CI. |
… false Was broken since initialised in 9b2d4d2 but not noticed due to NixOS#271123 (comment)
This reverts f8ea911, see also NixOS#271123 (comment)
delroth
removed
the
12.approvals: 1
|
This merge itself caused breakage and blocks channels now: |
|
@vcunat Yeah this is what I was anticipating with my previous message:
In the future we should apply the ratchet check idea I recently introduced for For now we can only fix forward as these problems happen, I'll do so. |
|
Oh actually no this is a different problem, I think this is something that wasn't anticipated in #269356, I'll investigate.. |
|
To be clear, this is broken on the merge commit itself already, not by any subsequent merges. |
This very weirdly broke the channel evaluation: https://hydra.nixos.org/build/245871962/nixlog/1 It appears that this attribute is only evaluated by Hydra, _not_ by ofborg. So this wouldn't have been detected by CI anyways in the PR that introduced the problem: #276800. However, due to #271123 (comment), the channel only broke once that was fixed with #278777 Whether the fix is good, I don't know, but the failing-on-darwin attribute doesn't exist anymore with this commit, making the tarball build succeed again: nix-build pkgs/top-level/release.nix -A tarball
Description of changes
In #269356 by @amjoseph-nixpkgs (but really the split-off #271123 by @Mic92),
lib.callPackageWithwas switched from anabortto athrow, which can be caught withbuiltins.tryEval. Because the ofborg evaluation gracefully handles evaluation failures that can be caught, this meant that alllib.callPackageWitherrors weren't being guarded against by CI anymore, making CI succeed when it really shouldn't have.This especially affected evaluation with
allowAliases = false, which triggers thecallPackageWitherror when aliases are used.Affected merged PRs are:
allowAliases = false#276112There may be more PRs in the pipeline that would fail on master.
@jtojnar already made a fix for one evaluation failure in #278744. This PR fixes the remaining errors and uses
abortagain to prevent against future breakages.In particular
nix-build pkgs/test/releasecontinues succeeding with this PR.Ping @lilyinstarlight
Add a 👍 reaction to pull requests you find important.