Add pkgs.nixos function #32422

roberth · 2017-12-07T20:22:43Z

Motivation for this change

This introduces a new function to streamline the use of NixOS in the context of NixPkgs.
Applications of this function include

creating a binary cache using CI for NixOps deployments
packages that depend on virtual machine images

This is an area where Nix really shines, so for me and probably others, it was a surprise that a function like this was not available.
Although it's a small addition, it is easily discoverable (pkgs.nixos<TAB> in repl) and steers towards using NixOS without using the legacy options in eval-config.nix.

The default value for system is pkgs.system, so in its current state, users on OS X have to pass a system value explicitly. I have considered introducing some logic to choose linux x86-64 automatically, but that might do more harm than good. It should be easy enough to pass it explicitly or override the default in an overlay.

WIP

in other PR, implement config.nixpkgs.externalPkgs option
use externalPkgs (should fix current ordering mistake)
~~split into pkgs.nixos and pkgs.nixosNew~~ renamed to nixos. pkgs.nixosNew doesn't really use pkgs, so adds little value and could be confusing
~~add assertions in pkgs.nixos to prevent unintended use of externalPkgs~~ not feasible at this point. Need to introspect priorities of definitions, which isn't possible now.
update documentation of ~~both~~ functions
add to release notes

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"
Used and tested this in a project ~~Tested execution of all binary files (usually in ./result/bin/)~~
Fits CONTRIBUTING.md.

nbp · 2017-12-17T15:58:30Z

pkgs/top-level/all-packages.nix

+    import (pkgs.path + "/nixos/lib/eval-config.nix") {
+      inherit modules system;
+      pkgs = self;
+    };


Any reasons to not have short-cuts such as:
https://github.com/NixOS/nixpkgs/blob/master/nixos/default.nix#L35-L39

nixos/default.nix defines three shortcuts, system, vm and vmWithBootLoader.

Adding system overloads the meaning of 'system' for (at least?) the third time. It can already mean 'system architecture' or config.system.

The vm shortcut seems arbitrary, because vm is only one of a number of useful attributes in config.system.build. vmWithBootloader is redundant because it only defines an option.
Also, these vm shortcuts may cause the configuration to be evaluated more than once, even when you're invoking nixosEvaluate only once. This can be surprising. For example, when you write a function that returns vm and config, that looks very reasonable, but config does not actually correspond to the configuration of vm.

I prefer to keep this function clean. Perhaps, maybe, something like let result = ...eval-config...; in result // { inherit (result.config.system) build; } might be future proof and unsurprising, but only if you agree.

This sounds good to me, to have { config, options, build }, pkgs should not be necessary as it is already exposed through config._module.args.pkgs

roberth · 2017-12-20T10:52:16Z

@nbp, thanks for the review. I have taken the liberty to simplify it further, based on the insights:

system can not be changed without changing the system everywhere in self, so that must be set when evaluating nixpkgs. This was a bug.
If you want to use config or options directly, you should probably be writing a module anyway. People with very weird requirements can expose these terms in build if they must, or use eval-config directly.

As a result I believe the interface is simpler, correct and equally powerful.

> (pkgs.nixosEvaluate { fileSystems."/".device = "_"; boot.loader.grub.enable = false; }).toplevel
«derivation ...

nbp

This patch looks good, with the comment addressed.

I also think that this at least deserved to figure in the release notes with an example, as this might help a lot of users to add NixOS configurations to Hydra.

nbp · 2017-12-26T09:30:25Z

pkgs/top-level/all-packages.nix

+  nixosEvaluate = configuration:
+    (import (pkgs.path + "/nixos/lib/eval-config.nix") {
+      inherit (pkgs) system;
+      modules = [{ config._module.args.pkgs = self; }]


nit: I understand that this is supposed to be a memory optimization, but in this particular case, this modification would have no effect because the _module.args is an attribute set which is merged with the // update operator.

Also, pinning a specific version of Nixpkgs is frequent in companies, and the intent of this line would prevent the pinning from having any effect. I guess, what you might do is add the nixpkgs.externalPkgs option to nixos/modules/misc/nixpkgs.nix and check that the user wants to use an external Nixpkgs if an another option is true.

What this module should do instead is set nixpkgs.externalPkgs to self, and let the configuration opt-in this optimization. Note, the reason of adding an externalPkgs option should also benefit NixOps, which suffer from exactly the same issue.

So, I suggest to first remove this line, and do the externalPkgs option and setting it here in a follow-up issue.

Thanks for catching my mistake.

I think we disagree on the purpose of the function and both purposes are legitimate, so maybe we should have two functions? Like

pkgs.nixos

build a NixOS from current pkgs

pinned by pinning pkgs

implemented in terms of nixosNew with externalPkgs and assertions that prevent config.nixpkgs.* settings because they conflict.

does not introduce NIX_PATH dependency

pkgs.nixosNew

build a NixOS from a full-blown configuration that defines its own config.nixpkgs.*

NixOS-style pinning with config.nixpkgs.*

evaluates a new NixPkgs based on config unless externalPkgs is set. externalPkgs comes with documentation about ignoring other config.nixpkgs.* options.

does not introduce NIX_PATH dependency

I'll focus on externalPkgs before continuing on this PR though.

roberth · 2018-02-12T11:00:53Z

@nbp, I've added two changelog entries (also one for the nixpkgs.pkgs feature), taking the liberty of putting them at the top because the are concerned with the 'core' of NixOS. I haven't emphasised use with NixOps because the best method of helping those users is not clear cut. When a sufficiently good method is figured out, it should be addressed in the NixOps documentation.

I've also updated the code to use nixpkgs.pkgs and I have improved the documentation.

roberth · 2018-02-12T11:01:44Z

I've checked the rendering of the release notes.

roberth · 2018-02-16T06:53:26Z

I suppose the label "9. needs changelog" can be removed. Is there anything else I can do?

roberth · 2018-02-27T14:26:39Z

@nbp is anything missing?

nbp

This sounds good to me! Please add a changelog entry, and it would be good to merge.

Thanks, and sorry for the delay.

nbp · 2018-03-24T13:41:38Z

pkgs/top-level/all-packages.nix

+      inherit (pkgs) system;
+      modules = [(
+                  {lib,...}: {
+                    config.nixpkgs.pkgs = lib.mkDefault self;


nit: config.nixpkgs.pkgs = lib.mkDefault pkgs; Otherwise this would only capture packages from all-packages and none of the one added by any of the overlays. This is a defect of the all-packages.nix file.

roberth · 2018-03-24T14:18:07Z

Seems like I've pushed an outdated version on the 5th. I've noticed the weird naming in all-packages.nix and created an issue for it, #34881.

roberth · 2018-03-27T10:03:42Z

@nbp The delay is no problem, except 18.03 has been branched off. Shall we add it to 18.03 as well?

matthewbauer · 2018-04-21T19:43:47Z

Sorry this got buried. Could you move the release notes to 18.09 and we can merge for that. (provided no objections by other people).

roberth · 2018-04-22T11:39:01Z

@matthewbauer, done. Thanks for digging this up :)

roberth · 2018-05-11T08:40:52Z

@matthewbauer this PR can be merged

roberth · 2018-10-15T11:57:50Z

Similar PR for nixosTest

lheckemann · 2018-10-18T12:07:30Z

Any reason you get the .config.system.build attr rather than just returning the evaled config? This means there's no way to get the resulting config options using nixpkgs.nixos, which would allow simplifying things like https://github.com/cleverca22/nixos-configs/blob/master/rescue_boot.nix (needs config.boot.kernelParams).

roberth · 2018-10-18T12:39:05Z

@lheckemann config.system.build is where all the 'outputs' were as far as I could tell. What you can do is add your own option, such as system.build.kernelParams that simply re-exports boot.kernelParams.
That could probably be upstreamed as well.

Alternatively, we could make nixos return config.system.build // { inherit config}, but that might be confusing. So I'm not convinced yet that is the right approach, unless we need to access more than just this one option.

I would like to see your module use the nixos function, because that will make your module work when system /= builtins.currentSystem and it will save a Nixpkgs fixpoint computation and preserve overlays.

lheckemann · 2018-10-18T12:58:49Z

There are some other useful bits in the top of the evaluation — options can be used for generating docs or otherwise obtaining info about nixos options for instance.

Also, nixos { nixpkgs.overlays = [(self: super: {foo = "hello";})];} doesn't behave as one might expect because of the fixpoint elimination (which can of course also be advantageous as you mentioned). Any idea how that might be avoided, even if it's just a way of throwing an error if nixpkgs.overlays is defined?

And finally, I can't take credit for the rescue_boot module, it's all @cleverca22 's work :)

roberth · 2018-10-18T13:43:20Z

The same trick should work for anything that's accessible in a module, including options. If you want to add config and options, the best way I think is to open a pull request.

The problem you describe is an instance of a more general pattern that exists in NixOS where some options mask others. There isn't much we can do with the current module system, because nixpkgs.pkgs may have a higher priority than nixpkgs.overlays and we have no way to detect that.
To solve this, I believe it's best to add explicit support for this into the module system, so it will be easy to use and so we can generate documentation based on this information.

For @cleverca22's rescue boot module this is not a problem, because the rescue boot NixOS doesn't declare its own overlays; it will only declare nixpkgs.pkgs by means of the nixos function.

lheckemann · 2018-10-18T13:57:27Z

The same trick should work for anything that's accessible in a module, including options. If you want to add config and options, the best way I think is to open a pull request.

I think I'd prefer removing .config.system.build instead, but it's in a release and therefore legacy we don't want to break now I guess >.<

roberth · 2019-05-26T09:33:04Z

@lheckemann Better late than never #62046

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Dec 7, 2017

roberth force-pushed the nixos-evaluate branch 2 times, most recently from 23b1ac6 to 3ccd49a Compare December 7, 2017 20:41

vcunat requested a review from nbp December 10, 2017 09:07

roberth mentioned this pull request Dec 17, 2017

Make NixOS available first-class in NixPkgs #32354

Closed

nbp reviewed Dec 17, 2017

View reviewed changes

roberth force-pushed the nixos-evaluate branch 2 times, most recently from bd058bc to 00504e0 Compare December 20, 2017 10:37

roberth force-pushed the nixos-evaluate branch from 00504e0 to f1838f8 Compare December 20, 2017 11:03

nbp approved these changes Dec 26, 2017

View reviewed changes

nbp added 0.kind: enhancement 6.topic: nixos 9.needs: changelog labels Dec 26, 2017

roberth changed the title ~~pkgs.nixosEvaluate function~~ [WIP] pkgs.nixosEvaluate function Dec 29, 2017

roberth mentioned this pull request Jan 10, 2018

nixos: Add nixpkgs.pkgs option #33700

Merged

8 tasks

roberth force-pushed the nixos-evaluate branch from f1838f8 to be9f42a Compare February 12, 2018 10:23

GrahamcOfBorg removed the 6.topic: nixos label Feb 12, 2018

roberth force-pushed the nixos-evaluate branch 2 times, most recently from 68eac3f to e2f5af7 Compare February 12, 2018 10:50

GrahamcOfBorg added 6.topic: nixos 8.has: changelog 8.has: documentation labels Feb 12, 2018

roberth changed the title ~~[WIP] pkgs.nixosEvaluate function~~ pkgs.nixos function Feb 16, 2018

roberth changed the title ~~pkgs.nixos function~~ Add pkgs.nixos function Feb 16, 2018

roberth force-pushed the nixos-evaluate branch from e2f5af7 to b20f731 Compare March 5, 2018 06:17

GrahamcOfBorg removed 6.topic: nixos 8.has: changelog 8.has: documentation labels Mar 5, 2018

roberth force-pushed the nixos-evaluate branch from b20f731 to 118b8b4 Compare March 5, 2018 07:34

nbp added the 6.topic: nixos label Mar 24, 2018

nbp approved these changes Mar 24, 2018

View reviewed changes

roberth force-pushed the nixos-evaluate branch from 118b8b4 to e2f5af7 Compare March 24, 2018 14:08

GrahamcOfBorg added 8.has: changelog 8.has: documentation labels Mar 24, 2018

matthewbauer added this to the 18.09 milestone Apr 21, 2018

nixpkgs: add nixos function

1441806

roberth force-pushed the nixos-evaluate branch from e2f5af7 to 1441806 Compare April 22, 2018 11:36

bobvanderlinden mentioned this pull request Jun 10, 2018

Pull requests becoming stale #41793

Closed

Merge branch 'master' into nixos-evaluate

7996889

matthewbauer merged commit 45c463d into NixOS:master Jun 10, 2018

roberth mentioned this pull request Nov 13, 2018

pkgs.nixosTests doesn't make sense #50301

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pkgs.nixos function #32422

Add pkgs.nixos function #32422

roberth commented Dec 7, 2017 •

edited

nbp Dec 17, 2017

roberth Dec 18, 2017

nbp Dec 20, 2017

roberth commented Dec 20, 2017

nbp left a comment

nbp Dec 26, 2017

roberth Dec 29, 2017

roberth commented Feb 12, 2018

roberth commented Feb 12, 2018

roberth commented Feb 16, 2018

roberth commented Feb 27, 2018

nbp left a comment

nbp Mar 24, 2018

roberth commented Mar 24, 2018

roberth commented Mar 27, 2018

matthewbauer commented Apr 21, 2018

roberth commented Apr 22, 2018

roberth commented May 11, 2018

roberth commented Oct 15, 2018

lheckemann commented Oct 18, 2018 •

edited

roberth commented Oct 18, 2018

lheckemann commented Oct 18, 2018

roberth commented Oct 18, 2018

lheckemann commented Oct 18, 2018

roberth commented May 26, 2019

Add pkgs.nixos function #32422

Add pkgs.nixos function #32422

Conversation

roberth commented Dec 7, 2017 • edited

Motivation for this change

WIP

Things done

nbp Dec 17, 2017

Choose a reason for hiding this comment

roberth Dec 18, 2017

Choose a reason for hiding this comment

nbp Dec 20, 2017

Choose a reason for hiding this comment

roberth commented Dec 20, 2017

nbp left a comment

Choose a reason for hiding this comment

nbp Dec 26, 2017

Choose a reason for hiding this comment

roberth Dec 29, 2017

Choose a reason for hiding this comment

pkgs.nixos

pkgs.nixosNew

roberth commented Feb 12, 2018

roberth commented Feb 12, 2018

roberth commented Feb 16, 2018

roberth commented Feb 27, 2018

nbp left a comment

Choose a reason for hiding this comment

nbp Mar 24, 2018

Choose a reason for hiding this comment

roberth commented Mar 24, 2018

roberth commented Mar 27, 2018

matthewbauer commented Apr 21, 2018

roberth commented Apr 22, 2018

roberth commented May 11, 2018

roberth commented Oct 15, 2018

lheckemann commented Oct 18, 2018 • edited

roberth commented Oct 18, 2018

lheckemann commented Oct 18, 2018

roberth commented Oct 18, 2018

lheckemann commented Oct 18, 2018

roberth commented May 26, 2019

roberth commented Dec 7, 2017 •

edited

`pkgs.nixos`

`pkgs.nixosNew`

lheckemann commented Oct 18, 2018 •

edited