Skip to content

cudaPackages: move __attrsFailEvaluation to allow deeper inspection #324685

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
ConnorBaker merged 2 commits into from
Jul 18, 2024
Merged

cudaPackages: move __attrsFailEvaluation to allow deeper inspection #324685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45a663d
cudaPackages: move __attrsFailEvaluation to allow deeper inspection
philiptaron Jul 4, 2024
31e3a94
tests.cuda.cudaPackages: remove __attrsFailEvaluation since the attrs…
philiptaron Jul 4, 2024
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
1 change: 0 additions & 1 deletion pkgs/test/cuda/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ in
_: ps:
lib.pipe ps [
(lib.filterAttrs isTest)
(as: as // { __attrsFailEvaluation = true; })
recurseIntoAttrs
]
))
Expand Down
19 changes: 11 additions & 8 deletions pkgs/top-level/cuda-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
# - Attribute names should be computable without relying on `final`.
# - Extensions should take arguments to build attribute names before relying on `final`.
#
# Silvan's recommendation then is to explicitly use `callPackage` to provide everything our extensions need
# to compute the attribute names, without relying on `final`.
# Silvan's recommendation then is to explicitly use `callPackage` to provide everything our
# extensions need to compute the attribute names, without relying on `final`.
#
# I've (@connorbaker) attempted to do that, though I'm unsure of how this will interact with overrides.
{
Expand All @@ -27,7 +27,6 @@
newScope,
pkgs,
config,
__attrsFailEvaluation ? true,
}:
let
inherit (lib)
Expand All @@ -50,10 +49,14 @@ let
cudaAtLeast = strings.versionAtLeast cudaVersion;

# Maintain a reference to the final cudaPackages.
# Without this, if we use `final.callPackage` and a package accepts `cudaPackages` as an argument,
# it's provided with `cudaPackages` from the top-level scope, which is not what we want. We want to
# provide the `cudaPackages` from the final scope -- that is, the *current* scope.
cudaPackages = final;
# Without this, if we use `final.callPackage` and a package accepts `cudaPackages` as an
# argument, it's provided with `cudaPackages` from the top-level scope, which is not what we
# want. We want to provide the `cudaPackages` from the final scope -- that is, the *current*
Comment on lines +52 to +54
Copy link
Contributor

@SomeoneSerge SomeoneSerge Jul 19, 2024

Choose a reason for hiding this comment

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

The change is fine within the scope of the PR, but the comment is misleading because packages under cudaPackages should not ask for the entire scope as an argment, they should ask for individual components CC @ConnorBaker

# scope. However, we also want to prevent `pkgs/top-level/release-attrpaths-superset.nix` from
# recursing more than one level here.
cudaPackages = final // {
__attrsFailEvaluation = true;
};

# TODO(@connorbaker): `cudaFlags` is an alias for `flags` which should be removed in the future.
cudaFlags = flags;
Expand Down Expand Up @@ -120,4 +123,4 @@ let
fixedPoints.extends composedExtension passthruFunction
);
in
cudaPackages // { inherit __attrsFailEvaluation; }
cudaPackages