Respect global config.cudaSupport #224068
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ofborg
bot
added
10.rebuild-darwin: 0
This comment was marked as outdated.
This comment was marked as outdated.
|
Woohoo! Yeah there's really no excuse for packages not to be using the |
SomeoneSerge
force-pushed
the
respect-config-cudaSupport
branch
from
862e963
to
4dd5676
Compare
|
@samuela @ConnorBaker here goes the more controversial one: should we make a |
Maybe let's save that for a future PR? Is there not a situation in which someone might want |
Then they'd explicitly set |
|
Now, this time I don't understand what ofborg is saying. How do I reproduce this error? |
3 tasks
SomeoneSerge
force-pushed
the
respect-config-cudaSupport
branch
4 times, most recently
from
f9701be
to
941117e
Compare
|
Result of |
|
misclick |
|
Result of 188 packages failed to build:
284 packages built:
|
…espect global defaults
'cudaSupport ? false' -> 'cudaSupport ? config.cudaSupport or false' to respect global defaults Packages expressions that take `cudaSupport ? false` are likely to ignore `config.cudaSupport`. Instead, we want them to make `cudaSupport` a required argument, or to explicitly refer to `config`
SomeoneSerge
force-pushed
the
respect-config-cudaSupport
branch
from
941117e
to
a17baa5
Compare
Eliminate uses of `config.cudaSupport or false` and alike, since the
option is now declared in config.nix with a default value
fd .nix -t f -x sed 's/config\.cudaSupport or false, cudaPackages [?] [{][}]/config.cudaSupport, cudaPackages ? { }/' '{}' -i
fd .nix -t f -x sed 's/config\.cudaSupport or false/config.cudaSupport/' '{}' -i
fd .nix -t f -x sed 's/cudaSupport = pkgs.config.cudaSupport/inherit (pkgs.config) cudaSupport/' '{}' -i
fd .nix -t f -x sed 's/cudaSupport = config.cudaSupport/inherit (config) cudaSupport/' '{}' -i
|
Revisiting this PR after a while:
Only a few changes in this PR were manual. These include the option declaration, and the ucx fix (#239182) which wasn't required until we made ucx respect This PR does not introduce anything like |
I'm not sure I understand... we already have |
An option definition in |
|
Result of |
ConnorBaker
approved these changes
Jul 25, 2023
|
Great work @SomeoneSerge! Merging. |
|
Result of 85 packages marked as broken and skipped:
160 packages failed to build:
430 packages built:
|
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
Make nixpkgs respect
config.cudaSupportto reduce divergence in outPaths caused by out-of-tree overlays.Also proposing a new practice: require derivations to consume
configand writecudaSupport ? config.cudaSupport, instead of checkingconfigoutside inall-packages.nixor such.Rationale: we had quite a few packages not respecting
config.cudaSupport, resulting in people (including me) adopting overlays that "fix" them. Overlays mean diverging hashes, which means we can't share cache. The easiest way to check if there are packages that don't respectconfig.cudaSupportis to grep:❯ ag 'enableCuda \? false'. Checkingconfiginall-packages.nixmeans grep produces false-positivesTBD: add links to example overlays, but some are SomeoneSerge/nixpkgs-unfree, and there are more in NixOS-QChem, and
numtide/nixpkgs-unfreeThings done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)CC @NixOS/cuda-maintainers