Skip to content
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.

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

cudaPackages.cuda_nvcc: don't inspect the cc wrapper #282382

Merged
merged 2 commits into from
Jan 21, 2024

Conversation

SomeoneSerge
Copy link
Contributor

@SomeoneSerge SomeoneSerge commented Jan 20, 2024

Adjust the evaluation after the refactoring in 86c28ee (#282220).

The way I explicitly extended nvcc.profile's LIBRARIES may have been redundant (building something mildly cursed like jaxlib may be necessary to verify). We should also strive to avoid looking inside the wrapper (e.g. the useLibsFrom interface should be mostly sufficient for our purposes). Basic things like saxpy do work.

Previously:

❯ NIXPKGS_ALLOW_UNFREE=1 nix-instantiate '.' -A tests.cuda
...
       error: necessary to fix CI
❯ nix build -f '.' --arg config '{ allowUnfree = true; }' -L python3Packages.torchWithCuda
...
       error: necessary to fix CI

Now:

❯ NIXPKGS_ALLOW_UNFREE=1 nix-instantiate '.' -A tests.cuda
/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-cuda-samples-12.2.drv
/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-saxpy-unstable-2023-07-11.drv
...

Description of changes

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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

CC @NixOS/cuda-maintainers


Add a 👍 reaction to pull requests you find important.

Unbreaks evaluation after the refactoring in
86c28ee
(NixOS#282220).

Explicitly extending nvcc's LIBRARIES may have been redundant (building
something mildly cursed like jaxlib may be necessary to verify)

Previously:
```
❯ NIXPKGS_ALLOW_UNFREE=1 nix-instantiate '.' -A tests.cuda
...
       error: necessary to fix CI
❯ nix build -f '.' --arg config '{ allowUnfree = true; }' -L python3Packages.torchWithCuda
...
       error: necessary to fix CI
```

Now:
```
❯ NIXPKGS_ALLOW_UNFREE=1 nix-instantiate '.' -A tests.cuda
/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-cuda-samples-12.2.drv
/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-saxpy-unstable-2023-07-11.drv
...
```
@SomeoneSerge SomeoneSerge added the 6.topic: cuda Parallel computing platform and API label Jan 20, 2024
@SomeoneSerge SomeoneSerge changed the title cudaPackages.cuda_nvcc: back-end cc already exposes the c++ stdlib cudaPackages.cuda_nvcc: don't inspect the cc wrapper Jan 20, 2024
Copy link
Member

@samuela samuela left a comment

Choose a reason for hiding this comment

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

Thanks @SomeoneSerge ! I just got hit with a bunch of these error: necessary to fix CI errors in https://github.com/samuela/nixpkgs-upkeep/actions/runs/7597660003

@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 21, 2024
@SomeoneSerge SomeoneSerge merged commit 21788cb into NixOS:master Jan 21, 2024
30 checks passed
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 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants