nixos: Add nixpkgs.pkgs option #33700

roberth · 2018-01-10T14:42:24Z

This lets the user set pkgs directly, so that it can be injected
externally and be reused among evaluations of NixOS.

Motivation for this change

This provides an easy and documented way to set pkgs for applications external to NixOS and it provides easy access to some extra flexibility for users of plain NixOS.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
Fits CONTRIBUTING.md.

I have tested this change manually using nix-repl

(import ./nixos { configuration.imports = [ ({lib,...}: { nixpkgs.pkgs = lib.mkDefault (pkgs // { nginxStable = pkgs.hello; }); }) ({lib,...}: { nixpkgs.pkgs = null; }) ]; }).config.services.nginx.package
  -> nginx

The first module defines a default pkgs. This is what NixOps could do, for example. The second module sets pkgs to null so we get the old behavior in this example. Omitting the second module has the effect of using the default pkgs from 'NixOps'.

This change is relevant for #32422 and improving NixOps performance.

roberth · 2018-01-10T14:43:30Z

@nbp do we have a way to check if nixpkgs.config, nixpkgs.system or nixpkgs.overlays have been set by the user?

roberth · 2018-01-10T14:50:31Z

Note that NixOps should probably not always set pkgs by default, because that's a breaking change. Instead it should provide an opt-in mechanism. The opt-in setting can exist at the network level, machine level or both. Opting out is always possible at the machine level by settings nixpkgs.pkgs = null.

edolstra · 2018-01-12T14:04:50Z

Another approach to preventing repeated evaluation of Nixpkgs: NixOS/nix@0395b9b

roberth · 2018-01-12T14:23:15Z

That's a very good idea, but does not address the general usefulness of being able to inject pkgs directly. If I already have a pkgs that I need to use, I don't want to jump through any hoops to use it for a NixOS.
Also, pkgs provides an 'escape hatch' for modifications to NixPkgs that can not be achieved through the current nixpkgs.* options.

That's the point of PR #32422 in which @nbp suggested externalPkgs. That's what I've implemented here as pkgs, keeping the name simple.

roberth · 2018-01-24T16:33:31Z

Is there anything I can do to improve this more powerful option for setting pkgs in NixOS and/or get it merged?

nbp · 2018-01-25T23:15:34Z

nixos/modules/misc/nixpkgs.nix

+      default = null;
+      defaultText = "Situation-dependent; plain NixOS sets this to <code>null</code>.";
+      type = types.nullOr pkgsType;
+      example = literalExample ''import pkgs.path {}'';


That's sounds like a nice example of an infinite loop, if someone write that within a module.
Maybe use <nixpkgs> instead of pkgs.path.

nbp · 2018-01-25T23:28:14Z

nixos/modules/misc/nixpkgs.nix

+
+  _pkgs =
+    if cfg.pkgs == null
+    then import ../../.. { inherit (cfg) config overlays system; }


nit: This should be part of the config section, as:

{ config = { _module.args.pkgs = cfg.pkgs; nixpkgs.pkgs = mkDefault (import ../../.. { inherit (cfg) config overlays system; }); }; }

I've ditched the null and put the default right in the option definition. That seems to just work.

nbp · 2018-01-25T23:33:57Z

@roberth

@nbp do we have a way to check if nixpkgs.config, nixpkgs.system or nixpkgs.overlays have been set by the user?

Yes you can do:

{ config, options, ...}:

let
  isSpecialized = options.nixpkgs.overlays.isDefined && options.nixpkgs.config.isDefined && options.nixpkgs.system.isDefined;
  isOverwritten = options.nixpkgs.pkgs.isDefined;
in
  …

While I understand what you are looking for, remember that this is a valid use-case to use nixpkgs.pkgs to ignore other options. I guess what you might look for is if there is a priority inversion, where a forced overlays / config / system is not taken into account. In such case we do not yet report the priority, but you might be able to extract it from the opt.definitions.

roberth · 2018-01-29T13:56:23Z

I wasn't able to determine whether options were being set. The module system would have to preserve the priority for that to work, which it doesn't expose. That is, it's not in the output of nix-instantiate --strict --show-trace --eval -E '(import ./nixos { configuration = { }; }).options.nixpkgs.config'. isDefined isn't useful because it is set to true when a default value is present.

roberth · 2018-01-29T14:00:03Z

Ok, I've implemented everything as suggested. Detecting the priority inversion is outside the scope of this feature, because it requires changes to the module system.

edolstra · 2018-01-29T14:05:57Z

nixos/modules/misc/nixpkgs.nix

-  _pkgs = import ../../.. config.nixpkgs;
+  pkgsType = mkOptionType {
+    name = "nixpkgs";
+    description = "An evaluation of NixPkgs; the top level attribute set of packages";


It's Nixpkgs, not NixPkgs.

So Nixpkgs is the odd one out. Fixed.

nbp · 2018-01-29T19:30:22Z

nixos/modules/misc/nixpkgs.nix

+        <code>config.nixpkgs.overlays</code>, etc.
+        ''
+        ;
+      default = import ../../.. { inherit (cfg) config overlays system; };


How does it render in the documentation? Isn't the defaultText supposed to provide a literal example of the default value?

That didn't look so good. Can't just move documentation around.

Now I've removed the docbook tags and added literalExample to remove the quotes. It's a code literal now, which is almost good. I can't put code like import ../../../ { whatever} without an explanation there. That'd be too vague.
So not perfect, but at least the formatting is a lot less wrong now.

Maybe add the rest as part of the documentation attribute, and/or use <nixos> instead of ../../.. even if this is not exactly true.

I've introduced a fake variable nixos in the docs to indicate the relative path. Not bad after all. Thanks for the input.

This lets the user set pkgs directly, so that it can be injected externally and be reused among evaluations of NixOS.

oxij · 2018-02-10T14:48:45Z

I can't understand how this interacts with `nixos/lib/eval-config.nix`. Does it still need the `pkgs` and `pkgsModule`?

roberth · 2018-02-11T18:22:50Z

@oxij, this change does not interact much with nixos/lib/eval-config.nix. eval-config.nix did not really need to implement pkgsModule in the first place, but it seems convenient for callers of eval-config.nix to do so. Technically it could be removed, but at the cost of breaking some users' nix code. I suppose it could be deprecated, but I don't think that it would be worth the cost at this point.

Your question sounds like it is related to PR #32422. That PR deals with calling NixOS from an evaluation of Nixpkgs. It is loosely related to this PR. The one does not require the other, but applying this PR first is more convenient.

roberth · 2018-02-12T09:23:16Z

@oxij coming to think of it, it might be better if eval-config.nix uses config.nixpkgs.pkgs instead of its own pkgsModule. It should probably keep the pkgs argument for now though.

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

nbp approved these changes Jan 25, 2018

View reviewed changes

roberth force-pushed the nixos-externalpkgs branch from a5e7e40 to 7eab3f8 Compare January 29, 2018 13:51

edolstra reviewed Jan 29, 2018

View reviewed changes

roberth force-pushed the nixos-externalpkgs branch from 7eab3f8 to 798a373 Compare January 29, 2018 14:09

nbp approved these changes Jan 29, 2018

View reviewed changes

roberth force-pushed the nixos-externalpkgs branch from 798a373 to d51a53b Compare January 29, 2018 20:08

nixos: Add nixpkgs.pkgs option

f2a45a4

This lets the user set pkgs directly, so that it can be injected externally and be reused among evaluations of NixOS.

roberth force-pushed the nixos-externalpkgs branch from d51a53b to f2a45a4 Compare January 30, 2018 11:57

fpletz merged commit 1fcbc70 into NixOS:master Feb 9, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos: Add nixpkgs.pkgs option #33700

nixos: Add nixpkgs.pkgs option #33700

roberth commented Jan 10, 2018 •

edited

roberth commented Jan 10, 2018

roberth commented Jan 10, 2018

edolstra commented Jan 12, 2018

roberth commented Jan 12, 2018

roberth commented Jan 24, 2018

nbp Jan 25, 2018

nbp Jan 25, 2018

roberth Jan 29, 2018

nbp commented Jan 25, 2018

roberth commented Jan 29, 2018 •

edited

roberth commented Jan 29, 2018

edolstra Jan 29, 2018

roberth Jan 29, 2018

nbp Jan 29, 2018

roberth Jan 29, 2018

nbp Jan 30, 2018

roberth Jan 30, 2018 •

edited

oxij commented Feb 10, 2018 via email

roberth commented Feb 11, 2018

roberth commented Feb 12, 2018

nixos: Add nixpkgs.pkgs option #33700

nixos: Add nixpkgs.pkgs option #33700

Conversation

roberth commented Jan 10, 2018 • edited

Motivation for this change

Things done

roberth commented Jan 10, 2018

roberth commented Jan 10, 2018

edolstra commented Jan 12, 2018

roberth commented Jan 12, 2018

roberth commented Jan 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nbp commented Jan 25, 2018

roberth commented Jan 29, 2018 • edited

roberth commented Jan 29, 2018

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

roberth Jan 30, 2018 • edited

Choose a reason for hiding this comment

oxij commented Feb 10, 2018 via email

roberth commented Feb 11, 2018

roberth commented Feb 12, 2018

roberth commented Jan 10, 2018 •

edited

roberth commented Jan 29, 2018 •

edited

roberth Jan 30, 2018 •

edited