nixos: hardware.nvidia.package option for selecting nvidia package #49703

FRidh · 2018-11-03T18:52:44Z

Make it possible to select a derivation to use. This also moves some
logic that belongs to the derivation back into it.

Motivation for this change

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

FRidh · 2018-11-03T19:01:18Z

nixos/modules/hardware/video/nvidia.nix

+    hardware.nvidia.package = lib.mkOption {
+      type = lib.types.package;
+      default = config.boot.kernelPackages.nvidia_x11;
+      defaultText = "config.boot.kernelPackages.nvidia_x11";


I'm thinking we should also get rid of the aliases and use instead

config.boot.kernelPackages.nvidiaPackages.stable

FRidh · 2018-11-04T10:19:49Z

Note this breaks the config for people using nvidiaXX in videoDrivers.

coreyoconnor · 2018-11-06T07:57:24Z

AFAICT overriding the linuxPackages derivation can achieve the same. EG:

  nixpkgs.config =
  {
    packageOverrides = super: let self = super.pkgs; in
    {
      linuxPackages = super.linuxPackages_4_18.extend (self: super: {
        nvidiaPackages = super.nvidiaPackages // {
          stable = self.nvidiaPackages.stable_390;
        };
      });
    };
};

FRidh · 2018-11-11T13:21:03Z

@coreyoconnor yep, there are typically multiple ways to achieve the same output. This is about user interface and having a consistent way of selecting a package in a module.

infinisil

I think it would make sense to add an assertion that checks for any nvidiaxx values in the driver list and error out with the corresponding equivalent config after this change.

infinisil · 2018-11-16T23:55:00Z

nixos/modules/hardware/video/nvidia.nix

@@ -96,6 +78,16 @@ in
        shows the Intel GPU at "00:02.0", set this option to "PCI:0:2:0".
      '';
    };
+
+    hardware.nvidia.package = lib.mkOption {
+      type = lib.types.package;


lib is already with'd above.

This was not addressed, but it's just a nitpick. What I meant is that you can replace type = lib.types.package with type = types.package

I know, I just think we should keep the lib prefix.

Huh why? I know some people don't like with, but either we use it for the whole file or not at all. I wouldn't mind either, but this mixed style is odd.

Any resolution to this?

nixos/modules/hardware/video/nvidia.nix

eadwu · 2018-12-22T19:45:45Z

In case this ever gets merged, it might be a good idea to change https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/hardware/video/bumblebee.nix#L80 as well for those who still use Bumblebee.

FRidh · 2018-12-30T12:44:52Z

The module will now check for driver names with versions. It's ugly but it functions.

Regarding bumblebee, I am not sure how to solve that. In that case, one would ideally also be able to point to a specific nvidia package. Clearly, I don't want to duplicate this logic in that module. I suppose the nvidia module could be extended to operate differently in case bumblebee is activated. E.g., all the options are offered, but actually loading the kernel module would depend on whether bumblebee is activated. I'll keep it out of this PR.

FRidh · 2019-01-03T16:48:52Z

@vcunat @infinisil could you give this (one more) look? I intend to otherwise merge in a couple of days.

infinisil · 2019-01-08T02:58:35Z

nixos/modules/hardware/video/nvidia.nix

+    hasDeprecated = drivers: any isDeprecated drivers;
+  in if (hasDeprecated drivers) then
+    throw ''
+      Selecting an nvidia driver has been modified for NixOS 19.03. The version is now set using `hardware.nvidia.package`.


I think we should show in the error message already what package the user should choose based on their previous setting. We have all the information we need to make a better error message and ease the transition for users.

Yeah, I guess it should be fine to simply add one "legacy" example and maybe also one "beta" example.

vcunat

Appears to be OK to me.

BTW, note: #50861 (comment)

nixos-discourse · 2019-01-31T01:24:36Z

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/use-nvidia-410-drivers-on-nixos/1364/16

baracoder · 2019-06-21T11:48:41Z

This PR is stale for half a year, with only two minor changes required.
ping @FRidh @infinisil @vcunat
Can we move it forward somehow?

felschr · 2020-03-14T11:46:18Z

ping

stale · 2020-09-10T18:54:01Z

Hello, I'm a bot and I thank you in the name of the community for your contributions.

Nixpkgs is a busy repository, and unfortunately sometimes PRs get left behind for too long. Nevertheless, we'd like to help committers reach the PRs that are still important. This PR has had no activity for 180 days, and so I marked it as stale, but you can rest assured it will never be closed by a non-human.

If this is still important to you and you'd like to remove the stale label, we ask that you leave a comment. Your comment can be as simple as "still important to me". But there's a bit more you can do:

If you received an approval by an unprivileged maintainer and you are just waiting for a merge, you can @ mention someone with merge permissions and ask them to help. You might be able to find someone relevant by using Git blame on the relevant files, or via GitHub's web interface. You can see if someone's a member of the nixpkgs-committers team, by hovering with the mouse over their username on the web interface, or by searching them directly on the list.

If your PR wasn't reviewed at all, it might help to find someone who's perhaps a user of the package or module you are changing, or alternatively, ask once more for a review by the maintainer of the package/module this is about. If you don't know any, you can use Git blame on the relevant files, or GitHub's web interface to find someone who touched the relevant files in the past.

If your PR has had reviews and nevertheless got stale, make sure you've responded to all of the reviewer's requests / questions. Usually when PR authors show responsibility and dedication, reviewers (privileged or not) show dedication as well. If you've pushed a change, it's possible the reviewer wasn't notified about your push via email, so you can always officially request them for a review, or just @ mention them and say you've addressed their comments.

Lastly, you can always ask for help at our Discourse Forum, or more specifically, at this thread or at #nixos' IRC channel.

baracoder · 2020-09-10T19:22:06Z

I would still like it 👍

ftrojahn · 2020-09-17T14:59:29Z

+1 Like to have this, too.

eadwu · 2020-09-17T15:12:00Z

I can probably maintain this PR until it's merged and if @FRidh doesn't have the time?

Not sure if ownership can be switched but I'll probably create a new PR if needed.

Make it possible to select a derivation to use. This also moves some logic that belongs to the derivation back into it.

FRidh · 2020-09-17T16:20:48Z

I've rebased this PR (and it builds again) but I don't intend to go further with it. Feel free to take it over! Creating a new PR is fine.

FRidh · 2021-01-31T09:02:14Z

#99323

FRidh added the 9.needs: changelog label Nov 3, 2018

FRidh requested a review from vcunat November 3, 2018 18:52

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) labels Nov 3, 2018

FRidh mentioned this pull request Nov 3, 2018

nvidiaLegacy390 Needed #49679

Closed

FRidh commented Nov 3, 2018

View reviewed changes

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Nov 3, 2018

FRidh force-pushed the nvidia branch from 622a3a7 to d3180f5 Compare November 11, 2018 13:20

FRidh requested a review from infinisil as a code owner November 11, 2018 13:20

ryanfitzsimon mentioned this pull request Nov 14, 2018

Use nvidia_x11 from different channel on 1809 #49139

Closed

infinisil requested changes Nov 17, 2018

View reviewed changes

eadwu mentioned this pull request Dec 9, 2018

NixOS 18.09: Stable nvidia drivers not compatible with RTX 2070 #51768

Closed

ben0x539 mentioned this pull request Dec 9, 2018

nvidia_x11: 390.87 -> 410.66 #48661

Merged

11 tasks

eadwu mentioned this pull request Dec 22, 2018

bumblebee: allow the use of different driver versions #44921

Closed

9 tasks

FRidh force-pushed the nvidia branch from d3180f5 to d126479 Compare December 30, 2018 11:52

GrahamcOfBorg added 10.rebuild-linux: 1-10 and removed 10.rebuild-linux: 0 labels Dec 30, 2018

infinisil reviewed Jan 8, 2019

View reviewed changes

vcunat approved these changes Jan 9, 2019

View reviewed changes

peterhoeg mentioned this pull request Jul 29, 2019

[WIP] nvidia-firmware: init at 325.15 - calling all nvidia users #25075

Closed

7 tasks

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 10, 2020

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 10, 2020

nixos: hardware.nvidia.package option for selecting nvidia package

170ce15

Make it possible to select a derivation to use. This also moves some logic that belongs to the derivation back into it.

FRidh force-pushed the nvidia branch from a60f7a6 to 170ce15 Compare September 17, 2020 16:19

ofborg bot added the 10.rebuild-linux: 1 label Sep 17, 2020

eadwu mentioned this pull request Oct 1, 2020

nixos: hardware.nvidia.package option for selecting nvidia package #99323

Merged

10 tasks

ryantm added 2.status: merge conflict and removed 2.status: merge conflict labels Oct 3, 2020

FRidh closed this Jan 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos: hardware.nvidia.package option for selecting nvidia package #49703

nixos: hardware.nvidia.package option for selecting nvidia package #49703

FRidh commented Nov 3, 2018 •

edited

FRidh Nov 3, 2018

FRidh commented Nov 4, 2018 •

edited

coreyoconnor commented Nov 6, 2018

FRidh commented Nov 11, 2018

infinisil left a comment

infinisil Nov 16, 2018

infinisil Jan 8, 2019

FRidh Jan 8, 2019

infinisil Jan 8, 2019

billksun Jan 16, 2019

eadwu commented Dec 22, 2018

FRidh commented Dec 30, 2018

FRidh commented Jan 3, 2019

infinisil Jan 8, 2019

vcunat Jan 9, 2019

vcunat left a comment

nixos-discourse commented Jan 31, 2019

baracoder commented Jun 21, 2019

felschr commented Mar 14, 2020

stale bot commented Sep 10, 2020

baracoder commented Sep 10, 2020

ftrojahn commented Sep 17, 2020

eadwu commented Sep 17, 2020 •

edited

FRidh commented Sep 17, 2020

FRidh commented Jan 31, 2021

nixos: hardware.nvidia.package option for selecting nvidia package #49703

nixos: hardware.nvidia.package option for selecting nvidia package #49703

Conversation

FRidh commented Nov 3, 2018 • edited

Motivation for this change

Things done

Choose a reason for hiding this comment

FRidh commented Nov 4, 2018 • edited

coreyoconnor commented Nov 6, 2018

FRidh commented Nov 11, 2018

infinisil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eadwu commented Dec 22, 2018

FRidh commented Dec 30, 2018

FRidh commented Jan 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vcunat left a comment

Choose a reason for hiding this comment

nixos-discourse commented Jan 31, 2019

baracoder commented Jun 21, 2019

felschr commented Mar 14, 2020

stale bot commented Sep 10, 2020

baracoder commented Sep 10, 2020

ftrojahn commented Sep 17, 2020

eadwu commented Sep 17, 2020 • edited

FRidh commented Sep 17, 2020

FRidh commented Jan 31, 2021

FRidh commented Nov 3, 2018 •

edited

FRidh commented Nov 4, 2018 •

edited

eadwu commented Sep 17, 2020 •

edited