-
-
Notifications
You must be signed in to change notification settings - Fork 161
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
[RFC 0067] Common override interface derivations #67
Conversation
[alternatives]: #alternatives | ||
|
||
An alternative would be to add a new method for overriding, `.overrideArgs`, thus allowing one to still call `.overrideAttrs` to override `stdenv.mkDerivation`. | ||
|
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.
Another alternative could be some mechanism to unify override
/overrideAttrs
into a single override mechanism supporting multiple (probably named?) levels, like overrideCall pkg "callPackage" (x: {…})
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.
Very much this, and it's also what NixOS/nixpkgs#87394 seems to implement. I think this is the only way to not end up with a mess when you have arbitrarily deeply nested function calls.
With this
.override
becomes.overrideCall "callPackage" { ... }
.overrideAttrs
becomes.overrideCall "mkDerivation" { ... }
.overridePythonAttrs
becomes.overrideCall "buildPythonPackage" { ... }
.overrideDerivation
becomes.overrideCall "derivation" { ... }
- etc.
With this, there's much less confusion as to how you need to override what. And it's a unified interface for the current override functions.
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 like this approach, but I do want to clarify that from a UI point of view, end users should typically need one and only one function that helps them. That is my motivation for shadowing .overrideAttrs
. Of course, if we can provide something like proposed here, as well as a call that is expected to override what is typically expected, then that would be great.
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 fail to see how this helps to be honest, because the difficult part (at least in my opinion) is understanding the semantics of the different override
functions, not their actual name. But all this (I'm using @infinisil's example here because it is concrete) does is rename override
to overrideCall "callPackage"
etc. but I still have to look at the code to see which of the strings (instead of functions) I have to use to achieve some specific goal.
I also don't know if shoving all these functions under one name will be very helpful documentation wise, as now I have to read more, and more importantly about a wider range of topics, to find out if this one function will solve my problem or not.
I think the problem lies more in the fact that it is difficult to grasp all the mechanics at play in nixpkgs, but I don't think that's solved by overloading a function.
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.
Yes, we need to be better at documenting the parameters and how they transform the data. In the past there were ideas for attaching docstrings to functions NixOS/nixpkgs#23505.
The RFC in its current state is a bit sparse in details. Is it possible to add more details on the implementation and how it would work when builders are nested? |
Nominating myself as a shepherd |
I will clearly need to extend this but just dumping here some thoughts. Consider e.g. the Python packages set that contains primarily Python packages ( In the last years that we've had |
This, apparently, happens because you have these four lines of code that allow arbitrary overrides of I am not sure whether we want to commit to always having this in every package creation function, and not use generic nested overrides. I am not even sure it is better in the case #58 gets unstuck and implemented and released and usable in Nixpkgs in a couple of years. Separately, if we are to touch the overrides at all, maybe say that the correct way to use overrides is |
Since opening NixOS/nixpkgs#87394 and being redirected here I experimented a bit more with different overriding variants. I'm currently favoring a layered approach over shadowing lower functions to keep the current flexibility of nixpkgs. The difference might not be a big deal in nixpkgs because it's self contained and developers have full control of the definition and usage of a function, but projects outside nixpkgs or even the The current set of overriding capabilities by all the Especially when working outside nixpkgs (projects that use nixpkgs) it can sometimes take two or three of these to replace a single dependency, so removing override functions might restrict users from using the nixpkgs package and simply copy paste the code into their own files:
Sure, it can all be done with one
Even if Since NixOS/nixpkgs#87394 I have tried to implement a layered variant of The first attempt used the shadowing approach for The second attempt (B4dM4n/nixpkgs@2793c4a) uses a "named layers" approach where each named layer can be specifically overridden and also allows to override layers bottom up without naming them. Unnamed layers (produced by It currently adds two new functions to provide it's features:
Theses two examples are equivalent (with the current set of named layers): nixpkgs-fmt.overrideLayers {
derivation = (old: {
buildInputs = old.buildInputs ++ [ hello ];
});
rust = {
version = "0.9.1";
cargoSha256 = "1wybvm9qckx9cd656gx9zrbszmaj66ihh2kk6qqdb6maixcq5k0b";
};
} nixpkgs-fmt.overrideLayers [
(old: {
buildInputs = old.buildInputs ++ [ hello ];
})
{
version = "0.9.1";
cargoSha256 = "1wybvm9qckx9cd656gx9zrbszmaj66ihh2kk6qqdb6maixcq5k0b";
}
] As a side effect from moving all override handling into one place, this also allows to use attribute sets and functions interchangeably as override arguments. With this approach |
We still don't have enough shepherds here to move this RFC to the next phase. |
I'd like to nominate myself to help shepherd this PR. |
Clarified the scope is overriding of derivations, not package sets. |
I think someone else could also provide doodle link. |
Hooks often rely on environment variables. When a builder builds upon that hook, it means the environment variables used by the hook need to be declared by the builder as well. |
@FRidh can you organize a meeting? https://www.when2meet.com/ |
Is this RFC stalled? @infinisil, @edolstra, @Taneb, @7c6f434c |
Hm. I am not sure, @infinisil posted a pretty nice proposal as intermediate and I am not sure if there was any later changes to it not shared or should we start from there and try to move forward (I still hold the opinion expressed in the shpeherd meeting notes that the approach needs some changes, indeed in the direction @infinisil has tried) |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/help-with-overrideattrs/13667/3 |
@infinisil, @edolstra, @Taneb, @7c6f434c since this RFC has stalled we will close it for the time being. Please feel free to re-open it again once work on this continues |
NixOS/nixpkgs#119942 (which, I should add, is fully backwards compatible and has merit in its own right) changes the landscape a bit. It could probably simplify the python expressions by making |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/using-nix-today-a-usability-quagmire/22323/2 |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
I think it's worth noting that if you can separate your overriding pipeline into separate functions each with a name, you'r basically one step away from putting those attrsets in a module together, with the additional benefit of having access to all layers without even having to think about layers. I suppose either a module-like solution or the more constrained interface proposed here can solve the issue of As a final thought, having to serialize the chain of overriding methods doesn't generalize too well. Perhaps we should first have a pattern where each overridable thing has a function as its parameter that specifies how its creator can be overridden. That may solve aforementioned architectural problem when, say, a package defined an extra overriding methods using |
This RFC came after the discussion on NixOS/nixpkgs#46842 and NixOS/nixpkgs#82772.
rendered