allow extending lib with an overlay #51797

ottidmes · 2018-12-09T21:20:22Z

Motivation for this change

I want to be able to extend lib, not just the lib argument being passed to NixOS modules, but I also want to be able to use the extended lib in e.g. my Nix scripts. I made a comment about this in the existing proposal to extend lib (#51109), but I felt this was not being sufficiently addressed, so I made this counter proposal. I will outline the problems I see with the existing proposal and my own initial plan, and how this proposal addresses them.

The problems with the existing proposal (#51109):

For all calls to ./lib/eval-config.nix to behave consistently, they would need to be passed the same lib, but right now they are only being passed to the calls to ./lib/eval-config.nix being made in nixos/default.nix, but just to give an example, ./lib/eval-config.nix is also being called in nixos/lib/build-vms.nix. So placing the logic to determine the custom lib in nixos/default.nix seems like the wrong place to me.
It adds an extra argument to the nixos/default.nix function. At first this might look like it could be considered a feature, because it would allow you to easily overwrite the lib being passed to modules as an argument, but it would suffer from the same issues as described in (1). This is actually also being warned about in a comment above the evalModules function defined in lib/modules.nix. The feature of being able to just change the lib being passed to modules as an argument could be implemented without triggering this issue, e.g. by setting { _module.args.lib = customLib; } and changing lib/modules.nix from inheriting lib to lib = _module.args.lib or lib, but I consider this is a separate feature request / issue than being able to extend lib in general.
It is better not to try and reverse engineer looking up paths in NIX_PATH, so rather than your looks-like-nixos-lib, it is probably better to use something based on tryEval, like how <nixpkgs-overlays> is being looked up in pkgs/top-level/impure.nix.
Even if the goal was to only change the lib being used when defining your configuration, then the proposed approach would still only affect lib being passed as module argument and does not consider other references to lib commonly used when defining a configuration, like pkgs.lib. They would still refer to the default lib.
There are multiple locations throughout Nixpkgs where lib is being imported directly via some relative path like ../lib, so these would still refer to default lib. It could be argued that this is fine, because they were implemented with the default lib in mind, unlike the NixOS configuration if we are allowed to modify lib, but it misses the point. I want to be able to modify lib, like Nixpkgs overrides/overlays allow me to modify pkgs without having to modify a Nixpkgs checkout directly, since the modifications could include tweaks or bug fixes that I might want to have applied everywhere lib is being used.
Taking the above into account, I do not like naming the NIX_PATH prefix nixos-lib, because it implies the lib only being used for NixOS.

My own initial plan to extend lib was to just duplicate what we have to overlay pkgs. In my prototype I used <nixpkgs-lib-overlays> and ~/.config/nixpkgs/lib/overlays{,.nix}. It worked, but it too has problems aside those already mentioned above:

The scale of pkgs is much larger than that of lib. The need for multiple overlays is clear in the case of pkgs, since even if you were to forget user defined overlays, it is already needed (or at least useful) in pkgs/top-level/stage.nix. In case of extending lib, the most common solution right now is to just do something like with import ./custom-lib.nix;, where ./custom-lib.nix tends to just hold a few definitions, not something you need complex layering for.
In this context nixpkgs implicitly refers to the the Nix packages, not the nixpkgs repository, so the naming and paths feel wrong. I am not sure what the better naming/paths would have been, but since I will not be proposing this solution, it is irrelevant.

Other problems as already noted in the existing proposal by @4z3:

Although it would be nice if we could specify a custom lib via the NIX_PATH prefix <nixpkgs/lib>, i.e. using the path itself as prefix, you can than no longer extend the default lib (without tricks) by referring to <nixpkgs/lib>, this would now refer to the custom lib and so result in infinite recursion.

This proposal for an extendable lib solves the above mentioned problems as follows:

Not requiring changes to the call site of ./lib/eval-config.nix, this solves (1).
There are no additional arguments added anywhere, except for lib/modules.nix, which needs a reference to the final lib (i.e. the extended one) in order to pass it along as an argument, but that is a safe argument to add and completely internal, so (2) does not apply.
I have copied over the try function defined in pkgs/top-level/impure.nix and added it to the default lib. I wish I could remove the duplication and use lib.try in pkgs/top-level/impure.nix, but that would require having to import (inefficient to reuse just one definition) or pass along lib as an argument (I am not sure what the impact of that is, considering potential problems as described in (2)). By defining and using try (the interface I would have liked to see for the tryEval builtin), we avoid (3).
By using an overlay, we solve the issue of having to reference to the default lib via <nixpkgs/lib> when defining our custom lib, solving (9). And the overlay will be applied in nixpkgs/lib/default.nix itself, so everything will automatically refer to the extended lib, with the exception of the default lib itself, which will still be defined in terms of itself (i.e. the way overlays work), which solves (4) and (5).
To make clear that lib will be extended via an overlay rather than replacing it for a custom lib as a whole (as proposed in the existing proposal), I thought it best to name the NIX_PATH prefix lib-overlay. No, nixos prefix (solves (6)), because that would imply too much coupling with NixOS, which is not the case, nor a nixpkgs prefix, because like mentioned in (8) nixpkgs in NIX_PATH and the NixOS nixpkgs options refer to Nix packages rather than nixpkgs the repo. If you talk about lib in the context of NixOS or some Nix script, it always refers to <nixpkgs/lib>, so I propose no prefix.
For my configurations I actually do want the heavy machinery of having multiple overlays for my lib, hence my initial idea, but I can realize that by implementing my lib overlay such that it in turn is defined by multiple overlays, solving (7).

That should cover all the above mentioned problems.

I just saw the following comment by Eelco at the existing proposal:

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.

So I am not sure how long this pull request will remain relevant, but in the meantime I will be using this.

Things done

roberth

Thank you for your PR. I appreciate your desire and willingness to improve nixpkgs. However, this part of the code is quite sensitive because it is part of many code paths, as also witnessed by its history.
Below are my thoughts about this change. I might have missed something, so please do respond if you find something wrong about my argumentation. I like to learn :)

This change breaks (unwritten?) rules about purity, breaking the purity of pinned projects, which I consider bad as a new default and worse as the only behavior.

Overlay functionality is hard to get right. In my understanding, to implement this for NixOS it will require (lazily) evaluating the configuration twice. One pass to determine lib with the built-in ../lib and once more with the new lib. I'm skeptical about the cost vs benefit here, but I can be convinced with a PR and benchmark. One cost is that some code will have to work for both versions of lib.

I am also concerned about the complexity caused by having another code path to override lib. It can already be done from within an overlay, so the differences between the two approaches will have to be explained and kept in mind when making further improvements to NixOS/Nixpkgs top-level glue.

I am not quite convinced of the benefits of this feature yet. The use cases I can think of for overriding lib are either modifications or additions.
Modifying the behavior of lib is probably not a good idea because a lot of code depends on the current behavior. So all other modifications should be bugfixes, which are best resolved by PR or patch. New behaviors should be new functions - additions.
Additions are new and therefore under your control, so I don't see why they need to be in lib. In fact, if you're not qualifying your names with some specific prefix, your addition may in the future collide with an upstream addition.
So why not put an attrset of your own in a regular overlay, or just import a certain file where you need it?

roberth · 2018-12-09T22:10:13Z

lib/default.nix

@@ -7,6 +7,9 @@ let

  inherit (import ./fixed-points.nix {}) makeExtensible;

+  # Allow extending lib by specifying an overlay file.
+  finalLib = lib.try (lib.fix (lib.extends (import <lib-overlay>) (self: lib))) lib;


This defeats the split between for example pkgs/top-level/impure.nix and pkgs/top-level/default.nix.
Pinning Nix expressions only works because Nixpkgs (a) provides pure versions of things or (b) lets the caller set the argument instead of an impure default. This not only breaks (a), but also breaks (b).

roberth · 2018-12-09T22:12:25Z

lib/default.nix

@@ -29,7 +32,7 @@ let
    versions = callLibs ./versions.nix;

    # module system
-    modules = callLibs ./modules.nix;
+    modules = import ./modules.nix  { lib = self; inherit finalLib; }; # module argument needs the final lib


Is self not equal to finalLib? If equal, finalLib can be omitted here. If not, either the naming is not according to convention, or there's a bug somewhere.

No they are not equal, self refers to the original lib and finalLib refers to the potentially extended lib. Only when there has been no overlay specified via lib-overlay in NIX_PATH will they be equal (see the definition of finalLib).

ottidmes · 2018-12-10T00:48:04Z

You seem to misunderstand the workings of overlays. They are just a Nix language trick for which you can lookup the definition yourself. The way I use them will not lead to the configuration being evaluated twice, the configuration has no effect on the final lib at all.

With your last question you seemed to have missed the whole point of this PR. You seem solely focused on Nixpkgs (which reflects you thinking about overlays and suggested problems and solutions, etc.), while I was mostly focused on Nix/NixOS when making this PR, and this indeed made me miss the important argument you make about purity in Nixpkgs. I have some ideas on how to solve the purity issue and I will fix them, if not only for myself, and I will probably update this pull request to reflect this, but I will not be going to put a lot more effort in this, because of two reasons, 1) even if the purity issue is solved, and even though it is completely optional to use, there will remain people not liking the idea, so it is very unlikely to get merged anyway, and 2) like Eelco mentioned, there are going to big changes in the near future that might make all this work obsolete.

I had some other contributions lined up (outside Nix packages), but I am not going to bother, I do not feel like it is worth the effort of trying to get it through (now especially with the big changes ahead), so I will just keep them for local use for now.

roberth · 2018-12-10T11:58:03Z

The way I use them will not lead to the configuration being evaluated twice

Yes, but taking this feature to its logical conclusion will require it. (Which may or may not be a problem)

I had some other contributions lined up (outside Nix packages), but I am not going to bother,

How hard it is really depends on the change you're proposing. This one had a very unfortunate complexity because this code is used in many different situations. Most changes will not be like this, so I hope that this pull request will not stop you from contributing in the future.

ottidmes · 2018-12-10T15:34:41Z

Yes, but taking this feature to its logical conclusion will require it. (Which may or may not be a problem)

I am not sure why you keep thinking this. What is this logical conclusion you are referring to. Could you elaborate? Again, the configuration itself has no effect on lib, you might change pkgs.lib, but this will not effect any other references to lib, just like it does not right now.

Most changes will not be like this, so I hope that this pull request will not stop you from contributing in the future.

Quite the contrary, almost any change made outside Nix packages and NixOS (application) modules will be like that.

roberth · 2018-12-10T16:16:08Z

When lib overlays are not injected through NIX_PATH but through NixOS configuration, NixOS will have to evaluate the configuration in order to know what lib to inject into the configuration, resulting in a loop.
A similar loop exists for pkgs, but that typically terminates because the part of the configuration that is demanded to determine pkgs (nixpkgs.*) typically does not depend on pkgs.
For lib this results in an infinite recursion of evaluation because to evaluate the option that defines the lib overlays, it will already need some of lib.types, lib.mkOption, lib.mkForce, lib.mkIf etc. So unlike for example pkgs, lazy evaluation is not sufficient and the cycle must be broken by evaluating part of the configuration with the unextended lib.

Quite the contrary

What I was trying to convey is that other changes may be worthwhile to try to upstream, but it's your call.

ottidmes · 2018-12-10T16:28:35Z

When lib overlays are not injected through NIX_PATH but through NixOS configuration, NixOS will have to evaluate the configuration in order to know what lib to inject into the configuration, resulting in a loop.

Which is why this approach is not being taken.

A similar loop exists for pkgs, but that typically terminates because the part of the configuration that is demanded to determine pkgs (nixpkgs.*) typically does not depend on pkgs.

Indeed.

For lib this results in an infinite recursion of evaluation because to evaluate the option that defines the lib overlays, it will already need some of lib.types, lib.mkOption, lib.mkForce, lib.mkIf etc.

Again, which is why this approach is not taken.

So unlike for example pkgs, lazy evaluation is not sufficient and the cycle must be broken by evaluating part of the configuration with the unextended lib.

If you just mean nix.nixPath, than that is a fundamental problem with setting NIX_PATH through configuration (e.g. my configuration sets nixpkgs=, which already has this problem), otherwise unrelated to the issue at hand.

What I was trying to convey is that other changes may be worthwhile to try to upstream, but it's your call.

That might very well be, but if I feel my changes are not taken seriously, as for example, it seems to me you have not really looked closely at what I have done and just went with some of your assumptions about overlays, then this does not exactly make me more forthcoming to do so.

But back on topic, I have made a different approach that should still address my initially mentioned problems and should also address the valid feedback you made about the potential impurity it could cause, which I will force push after some more testing.

ottidmes · 2018-12-10T17:46:50Z

The update in the PR has simplified things further, and now does only effect NixOS configurations, to lay the issues of potential impurities at rest, and since it does not affect nixpkgs/lib/default.nix anymore, it no longer has to be an overlay, so I changed it to no longer be one, so that I can still easily leverage it in my Nix scripts and REPL. The updated list:

Not requiring changes to the call site of ./lib/eval-config.nix, this solves (1).
There are no additional arguments added anywhere, ~~except for lib/modules.nix, which needs a reference to the final lib (i.e. the extended one) in order to pass it along as an argument, but that is a safe argument to add and completely internal,~~ so (2) does not apply.
I have copied over the try function defined in pkgs/top-level/impure.nix and added it to the default lib. I wish I could remove the duplication and use lib.try in pkgs/top-level/impure.nix, but that would require having to import (inefficient to reuse just one definition) or pass along lib as an argument (I am not sure what the impact of that is, considering potential problems as described in (2)). By defining and using try (the interface I would have liked to see for the tryEval builtin), we avoid (3). By still using builtins.tryEval we still avoid (3). I still wish for the reuse of try, but an extra file just for try or something like inherit (import ../../lib/trivial.nix {}) try; does not seem worth the effort for a one liner.
By using an overlay, we solve the issue of having to reference to the default lib via <nixpkgs/lib> when defining our custom lib, solving (9). And the overlay will be applied in nixpkgs/lib/default.nix itself, so everything will automatically refer to the extended lib, with the exception of the default lib itself, which will still be defined in terms of itself (i.e. the way overlays work), which solves (4) and (5).
The problem of (9) is now solved in the same way as the original proposal solved it, by using nixos-lib. The problems of (4) and (5) will no longer be addressed, because they might impact the purity of Nix packages, i.e. NIX_PATH should not impact code intended to be pure. (Not sure how I missed that one, so thank you @roberth for bringing it to my attention.)
To make clear that lib will be extended via an overlay rather than replacing it for a custom lib as a whole (as proposed in the existing proposal), I thought it best to name the NIX_PATH prefix lib-overlay. No, nixos prefix (solves (6)), because that would imply too much coupling with NixOS, which is not the case, nor a nixpkgs prefix, because like mentioned in (8) nixpkgs in NIX_PATH and the NixOS nixpkgs options refer to Nix packages rather than nixpkgs the repo. If you talk about lib in the context of NixOS or some Nix script, it always refers to <nixpkgs/lib>, so I propose no prefix. Now the scope of changing lib has changed to just changing it for the module argument passed to NixOS configurations, it now does need a prefix, and then nixos-lib is probably best.
For my configurations I actually do want the heavy machinery of having multiple overlays for my lib, hence my initial idea, but I can realize that by implementing my lib ~~overlay~~ such that it in turn is defined by multiple overlays, solving (7).

roberth · 2018-12-10T20:17:12Z

The default argument is still based on NIX_PATH, so this is a breaking change for those who want purity in NixOS configurations.
I'd prefer that this mechanism does not introduce impurity by default. This is achievable through NixOS configuration by making NixOS evaluate configuration twice as mentioned, once for determining lib using a fixed lib and once more for the final configuration. I'd like to hear someone else's opinion on this.

If the impurity by default remains, it should be added to the release notes and this PR should include fixes for pkgs.nixos, pkgs.nixosTest and probably pkgs.nixosTests. Those should remain pure and do not need an override mechanism for lib because these can and should use pkgs.lib which can be overridden by a 'normal' Nixpkgs overlay. (Although they may not use that yet. I haven't checked)

ottidmes · 2018-12-10T20:34:28Z

this is a breaking change for those who want purity in NixOS configurations.

It already is impure at that point in the code. Although that does not invalidate the argument of it making it less pure.

This is achievable through NixOS configuration by making NixOS evaluate configuration twice as mentioned, once for determining lib using a fixed lib and once more for the final configuration. I'd like to hear someone else's opinion on this.

Personally I would find that out of the question, it should not require having to evaluate twice, than your own initial arguments apply and I would be making those same arguments back to you.

Thank you for your feedback, by writing this response I realized that I could probably use the very impurity I referenced to, to make it work the way I want, without effecting others. It would only require one minor change in lib/modules.nix that should be harmless. I will be making another force push when I tested this idea.

ottidmes · 2018-12-10T23:40:00Z

@roberth I see my comment about NIXOS_EXTRA_MODULE_PATH has resulted in you making a PR about it, nicely done. I have tried a few things, but none of the alternatives I could come up were satisfactory. I doubt NIXOS_EXTRA_MODULE_PATH would be safe to use, because surely it is being used by something or it would not exist, and I would then not be able to use that something with my configuration, because it would overwrite my lib setting.

The best alternative would need to add an extra argument at all call sites of eval-config.nix, which I am afraid will be too error prone. However the issue about purity and what the flexibility I would like to see in my configuration have made me rethink the issue. Using NIX_PATH to supply arguments to a configuration is not what it is intended for, but rises out of a lack of another way to do it. I guess what I want is an optional extra layer before eval-config.nix where I would be able to set everything the way I want it. Depending on how you set things, this extra layer would be pure or impure. Right now my configuration is dependent on the environment and hence impure, but such an extra layer could make it completely pure again. To be clear, it would not be like that extra layer would be applied to all configurations, instead it should become part of the configuration somehow. For example, the configuration could be changed from only being allowed to be a module, but it might also be a setup file that would modify the argument set passed to eval-config.nix, not sure how exactly, I am just brainstorming. It would allow me to say things about my setup that I am now unable to do just through config, like overwriting lib for my config.

I will be closing this PR, because I no longer see it as the best course of action.

This is sort of a revert and different take on 2703028, which I decided to do after reading github:NixOS/nixpkgs#51797. Preventing our functions from being transparently accessed by derivations and modules makes it easier to figure out what else needs to be upstreamed with them. Where that is still wanted, the new `lib/extension.nix` is an overlay done right, balking at conflicts between Nixpkgs and us.

ottidmes requested review from edolstra and nbp as code owners December 9, 2018 21:20

ottidmes force-pushed the extend-lib branch from 900ad7f to 1a06bc2 Compare December 9, 2018 21:31

ottidmes mentioned this pull request Dec 9, 2018

nixos: allow overriding lib #51109

Closed

10 tasks

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

roberth requested changes Dec 9, 2018

View reviewed changes

ottidmes closed this Dec 10, 2018

ottidmes reopened this Dec 10, 2018

ottidmes force-pushed the extend-lib branch from 1a06bc2 to 9c2da98 Compare December 10, 2018 17:47

GrahamcOfBorg added the 6.topic: nixos label Dec 10, 2018

allow changing lib, but only for NixOS configurations

64b514c

ottidmes force-pushed the extend-lib branch from 9c2da98 to 64b514c Compare December 10, 2018 17:53

ottidmes closed this Dec 10, 2018

ottidmes mentioned this pull request Dec 12, 2018

configure the evaluation of modules in a modular way #51894

Closed

10 tasks

Janik-Haag added the 12. first-time contribution label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow extending lib with an overlay #51797

allow extending lib with an overlay #51797

ottidmes commented Dec 9, 2018 •

edited

roberth left a comment

roberth Dec 9, 2018

roberth Dec 9, 2018

ottidmes Dec 9, 2018

ottidmes commented Dec 10, 2018

roberth commented Dec 10, 2018

ottidmes commented Dec 10, 2018 •

edited

roberth commented Dec 10, 2018

ottidmes commented Dec 10, 2018 •

edited

ottidmes commented Dec 10, 2018

roberth commented Dec 10, 2018

ottidmes commented Dec 10, 2018

ottidmes commented Dec 10, 2018

allow extending lib with an overlay #51797

allow extending lib with an overlay #51797

Conversation

ottidmes commented Dec 9, 2018 • edited

Motivation for this change

Things done

roberth left a comment

Choose a reason for hiding this comment

roberth Dec 9, 2018

Choose a reason for hiding this comment

roberth Dec 9, 2018

Choose a reason for hiding this comment

ottidmes Dec 9, 2018

Choose a reason for hiding this comment

ottidmes commented Dec 10, 2018

roberth commented Dec 10, 2018

ottidmes commented Dec 10, 2018 • edited

roberth commented Dec 10, 2018

ottidmes commented Dec 10, 2018 • edited

ottidmes commented Dec 10, 2018

roberth commented Dec 10, 2018

ottidmes commented Dec 10, 2018

ottidmes commented Dec 10, 2018

ottidmes commented Dec 9, 2018 •

edited

ottidmes commented Dec 10, 2018 •

edited

ottidmes commented Dec 10, 2018 •

edited