-
-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
In lib/trivial/makeExtensible*
allow extenders to refer to refixer …
#20927
Conversation
@Ericson2314, thanks for your PR! By analyzing the history of the files in this pull request, we identified @cstrahan, @peti and @edolstra to be potential reviewers. |
…function As [reported][1] by @FRidh, `pkgs.overridePackages` no longer works because that function is added to the attribute set *after* fixing. Precisely, that means the `include pkgs` in `allpackages.nix` doesn't pick it up with the rest of the final fixed package set. This changes changes `makeExtensibleWithCustomName`, and by extension plain `makeExtensible` so that the refixer function is added as a final extension before fixing. [1]: NixOS#19496 (comment)
b1b6a09
to
f4a3bef
Compare
My comment on the issue #19496 (comment) |
I honestly think we should get rid of Otherwise, we should change @FRidh , could you describe your use-case of |
The crucial difference is that will reinfer a bunch of parameters, while with overridePackages one can modify a nixpkgs without worrying about what arguments were elsewhere provided. For a concrete usecase, think how big nix blobs like https://github.com/reflex-frp/reflex-platform/blob/develop/default.nix#L1-L19 are currently not very composable, but could be with |
@nbp if I understand correctly I've added the following example to the Nixpkgs manual
|
Also, I don't know whether it is possible, but there should be a 'straightforward' way to exclude configuration. E.g., if I test something locally it might not work elsewhere because of some setting in |
True, but you can provide a custom one as argument of let
newpkgs = import <nixpkgs> {
config = {
packageOverrides = pkgs: { ... };
};
};
in
newpkgs.inkscape This example will also exclude the per-user configuration, as the |
@nbp so that then also covers my second comment, right?
Anyway, I'm fine using the snippet you show. The docs just need to be updated to include this, because it is quite an important feature I would argue. |
That works for 1 customization, but not for the composition-of-customizations usecase I describe above. |
I guess for the composition we could just accept multiple new I guess we could add both environment variable / list of paths, and load all overlays in the order in which they are listed. Of course, if some argument are provided to nixpkgs, these overlays would take precedence. This way, we have options to get rid of I will prototype this idea tomorrow. |
@Ericson2314 With this PR, if one were to use
that would be no different than the current approach of
right? Actually, now that I've thought about it some more, I suppose the difference is that If we were worried about shadowing
EDIT: No need to change Another thought: worrying about shadowing TL;DR: I'm thinking we should merge this PR. Thoughts, @Ericson2314? Sorry for the winding, stream of consciousness style prose. I figure if anyone is as sleepy as I am, they too might miss some of the finer details, so I figured I'd leave this comment as is. |
Any update on this pull request? |
This is no longer important. Since then we have other recommended methods that work fine so I am closing this. |
(note I did not go through @cstrahan 's message. Feel free to reopen if needed) |
Motivation for this change
As reported by @FRidh,
pkgs.overridePackages
no longer works becausethat function is added to the attribute set after fixing. Precisely, that
means the
include pkgs
inallpackages.nix
doesn't pick it up with therest of the final fixed package set.
This changes changes
makeExtensibleWithCustomName
, and by extension plainmakeExtensible
so that the refixer function is added as a final extensionbefore fixing.
Things done
Tested manually using
nix-repl
. Should we create an automatic test for this?CC @nbp for posterity---I don't expect you to have any time for this.