-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
cudatoolkit: reintroduce version 11.7.0 to master #185557
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for resubmitting this @dguibert and apologies that this has been more involved than expected!
I think these changes look good, but I'd actually like us to go a bit further and de-couple TensorRT and cudatoolkit evaluation! It's totally fine for an upgrade to one of the packages to end up marking other derivations as broken, but breaking evaluation is a huge problem. Having to constantly keep the two in sync is an unnecessary burden on both packages and really came back to bite us in #179912.
Could we refactor pkgs/development/libraries/science/math/tensorrt/extension.nix
a bit such that upgrading cudatoolkit can be done without breaking the evaluation of tensorrt?
@samuela Sorry, I only just saw this PR. Since I introduced TensorRT to nixpkgs, I feel I should take this on. Could you elaborate a bit on what exactly you're proposing? |
@dguibert If you rebase off of #192958, then I think this should be good to go. Note that adding 11.7 to the supported versions of cudnn and TensorRT is still necessary in order to prevent those packages from being marked |
1a9b818
to
3cb66cf
Compare
Hi @samuela and @SuperSandro2000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I think this looks good overall. Running nixpkgs-review now...
EDIT: this run is a little wonky since NIXPKGS_ALLOW_BROKEN was not set, running a new nixpkgs-review now. Result of 4 packages marked as broken and skipped:
9 packages failed to build:
59 packages built:
|
Result of 4 packages marked as broken and skipped:
9 packages failed to build:
59 packages built:
|
1 similar comment
This comment was marked as duplicate.
This comment was marked as duplicate.
Here are the failure logs:
|
TensorRT and mathematica require special downloads.
So this change LGTM modulo the formatting changes mentioned above. |
3cb66cf
to
a0e9973
Compare
I've amending the formatting changes. (it's time to package version 11.8 released few days ago 😉 ) |
Thanks so much for your PR @dguibert, and thank you for your patience getting it merged! Can't believe that 11.8 is already here haha |
11.8 update has been proposed in #194705 |
Description of changes
This patch fixes #179912 which has been reverted on master due to https://gist.github.com/GrahamcOfBorg/45ac7f5bc9e02a74cb1e4264f365417f
Things done
sandbox = true
set 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/
)nixos/doc/manual/md-to-db.sh
to update generated release notes