nixos: allow overriding lib #51109

4z3 · 2018-11-27T12:43:48Z

Motivation for this change

Allows overriding the lib argument used in configurations, which in turn allows using library functions while working with imports, like e.g.:

{ lib, ... }: {
  imports = lib.my-imports-generator;
}

The same wouldn't be possible with pkgs.lib, as it would cause an infinite recursion.

I'm not sure this is the best way to implement this feature.

Things done

Allows overriding the `lib` argument used in configurations, which in turn allows using library functions while working with imports, like e.g.: ``` { lib, ... }: { imports = lib.my-imports-generator; } ``` The same wouldn't be possible with `pkgs.lib`, as it would cause an infinite recursion.

infinisil · 2018-12-05T20:35:53Z

nixos/default.nix

@@ -7,12 +16,18 @@ let
  eval = import ./lib/eval-config.nix {
    inherit system;
    modules = [ configuration ];
+    specialArgs = {
+      inherit lib;
+    };


eval-config.nix takes a lib argument which you can use instead.

Except that lib would then not propagate to the module arguments like you might expect. That argument is there as an optimization. In my lib overlays prototype I made that argument work as you expect, but this requires changes to lib/modules.nix, e.g. like this: { inherit config options; lib = args.lib or lib; } // specialArgs.

infinisil · 2018-12-08T19:16:44Z

nixos/default.nix

 { configuration ? import ./lib/from-env.nix "NIXOS_CONFIG" <nixos-config>
+, lib ? if builtins.any looks-like-nixos-lib builtins.nixPath
+          then import <nixos-lib>
+          else import ../lib


Oh actually, we can just use lib ? import <nixpkgs/lib> here. Then users can override it by setting

NIX_PATH=nixpkgs/lib=/path/to/lib:nixpkgs=/path/to/nixpkgs

This is then a single-line change pretty much accomplishing the same :)

Rather than setting lib like this, I think they would prefer to use the modular way (as mentioned in various comments), setting _module.args.lib. This requires some changes to lib/modules.nix to not make lib a special case any more, but I think this would be more in line with what we already have for e.g. pkgs.

I wasn't aware nixpkgs/lib can be overridden ❤️

ottidmes · 2018-12-08T20:00:26Z

I was planning on writing a PR to extend lib myself when @infinisil mentioned this PR. I experimented with making overlays for lib as we have for nixpkgs, which works, but like you mentioned they would have to be defined outside of your configuration, because most modules tend to include lib inside the scope at the top of the file (i.e. with lib;), causing recursion, just like pkgs.lib would.

I was using <nixpkgs-lib-overlays> and ~/.config/nixpkgs/lib/overlays{,.nix}, but I was not particular happy with that approach. Seeing your approach made me realize that it indeed could just be simplified to just pointing to a custom lib. This would still leave open the option to implement that custom lib via overlays, but leave that option completely open to the implementer. I however do not like the coupling between NixOS and lib. For example, I also sometimes use just with import <nixpkgs/lib>;, which would then not have the custom lib. I like the suggestion made by @infinisil instead, to use <nixpkgs/lib> itself to point to the custom lib if you so want to, but for it to work with any existing direct import of nixpkgs/lib/default.nix, I would suggest we change that file to check for <nixpkgs/lib> and use that if it does not refer to itself.

I have some other suggestions, but I better place them at the source code directly.

4z3 · 2018-12-08T20:39:08Z

would suggest we change that file to check for <nixpkgs/lib> and use that if it does not refer to itself.

@ottidmes, ~~I haven't tested the redirection, but~~ I think d145173 implements what you're suggesting. That said, I'm not really sure we should redirect direct imports of nixpkgs/lib/default.nix. Can you elaborate on why we would want to do the redirection?

4z3 · 2018-12-08T20:55:20Z

One downside I'm seeing with overriding nixpkgs/lib is that it's not possible to extend <nixpkgs/lib> easily anymore. The earlier approach, i.e. using <nixos-lib>, allowed to extend <nixpkgs/lib> like this:

import <nixpkgs/lib> // {
  mystuff = "...";
}

Now this causes an infinite recursion, and the only solution I'm seeing is to reimplement <nixpkgs/lib/default.nix> it the custom lib.

EDIT
Without d145173, the above can be accomplished with following code, which would be good enough for me:

import <nixpkgs/nixos/../lib> // {
  mystuff = "...";
}

I'm going to drop d145173 from this PR unless there's a reason to keep the redirection in <nixpkgs/lib/default.nix>.

ottidmes · 2018-12-08T21:52:59Z

The idea behind the redirection was that there are still many places in nixpkgs that references nixpkgs/lib directly via path imports (search nixpkgs with this regex: \./lib[^0-9a-zA-Z/\.\-|"]), and outside nixpkgs there might be people having such references too, but its also fair to say that in those cases you probably do not want to use the custom lib anyway. Its just if you want the custom lib be the default everywhere, redirection is the only sure way I could think of.

4z3 · 2018-12-08T23:39:43Z

Now, after trying to actually use an overridden nixpkgs/lib in my setup, I have to say that I'm not happy with it anymore for following reasons:

the above mentioned import <nixpkgs/nixos/../lib> doesn't look particularly good to me, and I wouldn't like to promote such hacks as the conventional way to do things
and more importantly, it's not possible to use just NIX_PATH=/somepath anymore, because nixpkgs/lib has to be overridden explicitly.

Therefore I'm reverting this PR to the original state again. I'm not sure the implementation is the best possible, but having a separate path like <nixos-lib> feels cleaner to me.

ottidmes · 2018-12-09T20:26:13Z

I get the issues you have with nixpkgs/lib as a prefix, like (1) you mentioned, but I do not quite understand the issue described in (2), could you elaborate on that? It is already impossible to have just NIX_PATH=/path/to/nixpkgs, it needs to be at least NIX_PATH=nixpkgs=/path/to/nixpkgs and probably even NIX_PATH=nixpkgs=/path/to/nixpkgs:/path/to/nixpkgs. And how would using nixos-lib as a prefix address the issue in (2)?

edolstra · 2018-12-09T20:36:34Z

nixos/default.nix

@@ -1,4 +1,13 @@
+let
+  looks-like-nixos-lib = { path, prefix }:


camelCase please.

edolstra · 2018-12-09T20:38:38Z

Not really in favor of adding another ad-hoc override mechanism. Also, in the future, the Nix search path is likely to be deprecated in favor of flakes, where the Nixpkgs library may well be factored out into a separate flake, which would allow it to be overriden easily.

ottidmes · 2018-12-09T21:39:18Z

I will eagerly await more news about these flakes! In the meantime I made a different proposal (#51797) on how to extend lib, which while still ad hoc, tries to address the problems I had with this proposal.

4z3 · 2018-12-10T06:21:48Z

Looking forward to seeing flakes.

4z3 requested a review from nbp as a code owner November 27, 2018 12:43

GrahamcOfBorg added 6.topic: nixos 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Nov 27, 2018

Mic92 requested a review from infinisil December 4, 2018 20:09

infinisil requested changes Dec 5, 2018

View reviewed changes

infinisil requested changes Dec 8, 2018

View reviewed changes

4z3 force-pushed the nixos-lib branch from cbbd839 to d145173 Compare December 8, 2018 20:37

4z3 requested a review from edolstra as a code owner December 8, 2018 20:37

4z3 force-pushed the nixos-lib branch from d145173 to adb8018 Compare December 8, 2018 21:09

4z3 force-pushed the nixos-lib branch from adb8018 to cbbd839 Compare December 8, 2018 23:40

edolstra reviewed Dec 9, 2018

View reviewed changes

nixos/default.nix

@@ -1,4 +1,13 @@

let

looks-like-nixos-lib = { path, prefix }:

Copy link

Member

edolstra Dec 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

camelCase please.

ottidmes mentioned this pull request Dec 9, 2018

allow extending lib with an overlay #51797

Closed

10 tasks

4z3 closed this Dec 10, 2018

oxij mentioned this pull request Mar 28, 2019

On the end goal nix-community/flake.nix#11

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos: allow overriding lib #51109

nixos: allow overriding lib #51109

4z3 commented Nov 27, 2018

infinisil Dec 5, 2018

ottidmes Dec 8, 2018

infinisil Dec 8, 2018

ottidmes Dec 8, 2018

4z3 Dec 8, 2018

ottidmes commented Dec 8, 2018

4z3 commented Dec 8, 2018 •

edited

Loading

4z3 commented Dec 8, 2018 •

edited

Loading

ottidmes commented Dec 8, 2018

4z3 commented Dec 8, 2018

ottidmes commented Dec 9, 2018

edolstra Dec 9, 2018

edolstra commented Dec 9, 2018

ottidmes commented Dec 9, 2018

4z3 commented Dec 10, 2018

nixos: allow overriding lib #51109

nixos: allow overriding lib #51109

Conversation

4z3 commented Nov 27, 2018

Motivation for this change

Things done

infinisil Dec 5, 2018

Choose a reason for hiding this comment

ottidmes Dec 8, 2018

Choose a reason for hiding this comment

infinisil Dec 8, 2018

Choose a reason for hiding this comment

ottidmes Dec 8, 2018

Choose a reason for hiding this comment

4z3 Dec 8, 2018

Choose a reason for hiding this comment

ottidmes commented Dec 8, 2018

4z3 commented Dec 8, 2018 • edited Loading

4z3 commented Dec 8, 2018 • edited Loading

ottidmes commented Dec 8, 2018

4z3 commented Dec 8, 2018

ottidmes commented Dec 9, 2018

edolstra Dec 9, 2018

Choose a reason for hiding this comment

edolstra commented Dec 9, 2018

ottidmes commented Dec 9, 2018

4z3 commented Dec 10, 2018

4z3 commented Dec 8, 2018 •

edited

Loading

4z3 commented Dec 8, 2018 •

edited

Loading