Skip to content

cudaPackages_11_0.autoAddCudaCompatRunpath.libcudaPath: fix the eval#406668

Merged
ConnorBaker merged 1 commit intoNixOS:masterfrom
trofi:cudaPackages_11_0.autoAddCudaCompatRunpath.libcudaPath-fix-eval
Jun 9, 2025
Merged

cudaPackages_11_0.autoAddCudaCompatRunpath.libcudaPath: fix the eval#406668
ConnorBaker merged 1 commit intoNixOS:masterfrom
trofi:cudaPackages_11_0.autoAddCudaCompatRunpath.libcudaPath-fix-eval

Conversation

@trofi
Copy link
Contributor

@trofi trofi commented May 13, 2025

Addresses #405031 (comment). Without the change direct access to the attribute fails to eval as:

$ nix eval --impure --expr 'with import ./. {}; cudaPackages_11_0.autoAddCudaCompatRunpath.libcudaPath'
evaluation warning: CUDA versions older than 12.0 will be removed in Nixpkgs 25.05; see the 24.11 release notes for more information
error:
   … while evaluating the attribute 'autoAddCudaCompatRunpath.libcudaPath'
     at pkgs/development/cuda-modules/packages/autoAddCudaCompatRunpath/package.nix:16:5:
       15|   substitutions = {
       16|     libcudaPath = if cuda_compat then "${cuda_compat}/compat" else null;
         |     ^
       17|   };

   … while evaluating a branch condition
     at pkgs/development/cuda-modules/packages/autoAddCudaCompatRunpath/package.nix:16:19:
       15|   substitutions = {
       16|     libcudaPath = if cuda_compat then "${cuda_compat}/compat" else null;
         |                   ^
       17|   };

   error: expected a Boolean but found null: null
   at pkgs/development/cuda-modules/packages/autoAddCudaCompatRunpath/package.nix:16:19:
       15|   substitutions = {
       16|     libcudaPath = if cuda_compat then "${cuda_compat}/compat" else null;
         |                   ^
       17|   };

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: cuda Parallel computing platform and API label May 13, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels May 13, 2025
@trofi
Copy link
Contributor Author

trofi commented May 13, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 406668


x86_64-linux

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Tldr: we've had this exact patch before, I'm hoping to eliminate this hacks by setting the platforms right

@SomeoneSerge SomeoneSerge mentioned this pull request May 13, 2025
13 tasks
@ConnorBaker
Copy link
Contributor

If a package is marked as broken or is on an unsupported platform, should we expect arbitrary attributes of that package to evaluate?

@SomeoneSerge
Copy link
Contributor

As I mention in #405031, my understanding is we only ever enforced that CI must pass (tautology intended), and CI only ever cared about eval errors in "supported" packages.

I'm all for adopting a stricter convention at least at the cudaPackages level, we'd just have to first ensure it's universally applied (notoriously violated by e.g. our handling of cudnn), and then ensure it stays that way (need CI).

We've nominally adopted the rule that namesets of cudaPackages should be stable (if one instance has a cudnn_8_9 attribute, all instnaces must have one, albeit marked broken or unsupported), we failed to enforce it. Specifically libcudaPath a fringe case because nobody actually ever references it outside the definition, but it would be sure nice not to have it throwing.

For now I propose we do an ugly fix along the lines of #406207 (comment)

More generally, one goal of #406740 is to fix a static set of names guaranteed to exist across all cudaPackages instances

@winterqt
Copy link
Member

If a package is marked as broken or is on an unsupported platform, should we expect arbitrary attributes of that package to evaluate?

I don't think so -- there are plenty of packages in-tree that fail to eval under those two cases.

FWIW: my CI work (which I expect to PR some time today) catches the issue that #406207 fixed, but not this one, because it has no supported platforms.

@winterqt
Copy link
Member

@trofi If I may ask, how did you even find this if you're not just blanket evaling everything while allowing broken and unsupported platforms?

@SomeoneSerge
Copy link
Contributor

SomeoneSerge commented May 13, 2025 via email

@trofi
Copy link
Contributor Author

trofi commented May 13, 2025

@trofi If I may ask, how did you even find this if you're not just blanket evaling everything while allowing broken and unsupported platforms?

I blanket-eval everything :) https://trofi.github.io/posts/309-listing-all-nixpkgs-packages.html

I usually run ./all-attrs-iter.bash -I nixpkgs=~/n --arg maxDepth 4 --arg verbose 3 --arg ignoreDrvAttrs false --arg stepLimit 10000 daily. Current script: https://github.com/trofi/nixpkgs-overlays/blob/main/lib/all-attrs-iter.bash

@trofi
Copy link
Contributor Author

trofi commented May 13, 2025

Note that normally packages on unsupported platform generate the exceptions that you can catch (and ignore) with builtins.tryEval. I ignore those. The problem here is that it's an uncatchable null-dereference error not guarded by any platform check.

@SomeoneSerge
Copy link
Contributor

builtins.tryEval

That's a good distinction! Anyway, if you're ok with the alternative "fix" I pushed feel free to squash & merge

@trofi trofi force-pushed the cudaPackages_11_0.autoAddCudaCompatRunpath.libcudaPath-fix-eval branch from 0e1df15 to 6a84a15 Compare May 26, 2025 16:31
@trofi
Copy link
Contributor Author

trofi commented May 26, 2025

You change fixes eval for me as well. Dropped my patch.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/5561

@ConnorBaker ConnorBaker merged commit 7ad6768 into NixOS:master Jun 9, 2025
16 of 17 checks passed
@github-project-automation github-project-automation bot moved this from New to ✅ Done in CUDA Team Jun 9, 2025
@trofi trofi deleted the cudaPackages_11_0.autoAddCudaCompatRunpath.libcudaPath-fix-eval branch June 10, 2025 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: cuda Parallel computing platform and API 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

5 participants