-
-
Notifications
You must be signed in to change notification settings - Fork 15k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make lib
more unified and overridable
#157056
Conversation
I don't think the lib should be overridable. The point of overriding is to be able to modify dependencies, but I don't know why anyone would be want do do that with the lib. If you want to merely add new stuff, you don't need overrides. |
@Ericson2314, my real life case is #154031 related to |
The first step of the reasoning is that if you have a bunch of functions you want to use in your NixOS modules, extending the But then we're left with an awkward situation where |
I think passing more parameters is good. People not liking passing individual parameters skirts the danger of state/entanglement. I still think the fundamental difference between overlays and this is and overlays are for overriding dependencies, and this is not. Put another way, if we only extended package sets with downstream packages, and never modified upstream ones, I would not approve of overlays. |
I see your point. I don't think that's the only thing overlays are for, though: they allow fixing broken packages (or in this case broken lib attributes), and they make everything available under a single namespace. For nixpkgs, only the first thing applies since nixpkgs expressions won't be using our extensions, so I understand if you don't want to add a For NixOS modules though, the second thing also applies, and I think this justifies the last commit, so I will split it into a separate PR (along with the first one which is orthogonal). EDIT: #160459 |
I believe with #160489, |
It should be
|
Or alternatively have the type of |
It won't produce an explanation like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
types...raw
is available now, so the type can be improved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @edolstra and @Ericson2314's intent. In a perfect world, their argument would block this change, but in our messy one, we have to deal with stuff like incompatibilities between nixpkgs versions and potentially other use cases. lib.extend
already exists and continues to exist for such reasons. Just that it shouldn't be the preferred choice, doesn't mean that it should be crippled.
I think extending the library is just a weird idea in general. It's confusing and bad for UX, because it means that you can't count on a function What exactly is the problem with putting one's own functions in |
There rarely is, unless you're polyfilling missing functionality in the stable release for example. Like the "incompatibilities" link in my previous comment. |
It's supposed to be an end user thing!
|
The "end user" here can also be a colleague who think it's a good idea to extend
Why? That's what basically every other language does: if you want something from |
I think I would agree with you if it was named I can see the problem with figuring out where things are coming from, but I think this is outweighed by the benefit of having a relatively unified boilerplate for modules, plus the practical uses @roberth and I mentioned. For a programming language analogy, think of Haskell, where the dependency plumbing happens in the Cabal file (i.e. |
I am very much in favor of this, @edolstra @Ericson2314, do you oppose the merging of this PR? |
This is necessary for `lib` to still be extensible in the modules evaluated by `evalModules`. Also inline the call to `fix'` for performance.
Makes it easy to override the lib used across pkgs, for example to stay in sync with the lib used in NixOS modules.
Unify the lib used in stdenv/generic with the one passed to pkgs/top-level
Allows overriding the lib passed to the default nixpkgs invocation. Defaults to the `lib` argument to modules.
Makes lib extensions available in modules by default
This is a PR I really want to see go through. I have a lot of stuff where I override things in nixpkgs and this PR will make things easier. |
Rebase is needed. |
Why close? 🤔 |
A broad attempt at fixing #156312-like issues. With this, the following flake works as expected:
See each commit's description for details.
This will probably need release notes.
The type
lazyAttrsOf unspecified
fornixpkgs.lib
is the most precise I could make it. WithattrsOf
we get warnings from trying to evaluate deprecated attributes, and withanything
instead ofunspecified
there's a weird infinite recursion.