Skip to content
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

Extensible attrsets #2511

Closed
Ekleog opened this issue Nov 3, 2018 · 13 comments
Closed

Extensible attrsets #2511

Ekleog opened this issue Nov 3, 2018 · 13 comments
Labels

Comments

@Ekleog
Copy link
Member

Ekleog commented Nov 3, 2018

This issue is here to track discussion on extensible attrsets.

See https://gist.github.com/edolstra/29ce9d8ea399b703a7023073b0dbc00d (linked with @edolstra's permission) for more details.

cc @matthewbauer @joepie91 @jwiegley @danbst who commented on the gist

@Ekleog
Copy link
Member Author

Ekleog commented Nov 3, 2018

So I was thinking some more about extensible attrsets and noticed that they could actually be implemented almost completely in pure-nix as of today, I think.

As a proof, here is an example implementation and use: https://github.com/Ekleog/nix-extensible-attrsets/blob/master/example.nix (disclaimer: it was done in ~2h30, is completely imperfect, likely has bugs and was intentionally kept as simple and readable as possible. IOW, it's a POC)

The two features missing for a nicer usage are:

  • Late-bound variables, ie. having nix auto-generate these {builtInputs, ...}: function definitions from the set of variables used in the value and defined (or with metadata defined) in the set -- will likely require a special kind of set to make this transparent
  • A nicer syntax for metadata definition

The advantage is that if these two features are implemented (and not directly extensible attrsets), then we get smaller primitives, and thus more unexpected features that will be usable later. For instance, the metadata format is completely extensible and easily introspectable (just access the .metadata.doc field of the package), which makes for easy overriding or reusing in other contexts than the strictly defined one.

Also, we gain agility should we notice that the module system is not actually what we want, as if the module system is directly embedded into nix then we could never remove it unless we're willing to break all the packages (including those unrelated to nixpkgs).

@danbst
Copy link
Contributor

danbst commented Nov 4, 2018

There are many orthogonal problems described in @edolstra gist. One of them is adding "configurations" (or "modules", or "extensible attrsets") to Nix syntax, another is to rewrite stdenv and pkgs using it, so no more .override (hello, NixOS/rfcs#3)

If you think your approach covers "extensible attrsets" requirements, then you can try to rewrite stdenv and mkDerivation using it. Seems like you don't need any syntax changes for proof of concept. So go ahead, me personally I'd like to look at result. If it proves to be nice, the rewrite approach may even be reused after necessary syntax changes are added.

I somewhat like metadata approach. The check/merge probably should be unified into one attribute type, so it's possible to do

...
type = with builtins.types; listOf string;
...

and teach Nix parser to parse with builtins.types; listOf string into some C primitive.

@periklis
Copy link

periklis commented Nov 5, 2018

Why not move the gist to an official RFC?

@edolstra
Copy link
Member

edolstra commented Nov 5, 2018

It's not in a state yet to be turned into an RFC.

@Ekleog
Copy link
Member Author

Ekleog commented Nov 5, 2018

@danbst The main reason why I didn't move forward with type is that I really think we should use @Profpatsch's types-simple.nix here so as not to rewrite the world twice, but they are currently still lacking merge capability and I haven't come up with a solution that cleanly covers all the use cases the NixOS type system covers yet.

@domenkozar
Copy link
Member

On extensible attrs I'd like to reference Russell O'Connor on fake dynamic binding in Nix:

The syntax virtual (self: with self; { bindings }) is a little heavy. One could imagine adding a virtual keyword to Nix, similar to rec, so that virtual { bindings } would denote this expression.

As noted by @edolstra in NixCon talk, making .override a Nix language construct would mean better memory performance and I'd really like this to be a built-in function rather than special syntax.

@Ekleog
Copy link
Member Author

Ekleog commented Nov 6, 2018

The virtual proposal and the late-bound proposals are not the same, though: virtual puts the self outside of the set, while late would put the arguments inside the set, one for each function. This is a big difference between the two, I think. Because it means that the virtual proposal can handle only override, and not merging, and the gist mentions the need for merging. (disclaimer: there may be a way I didn't think of, but the use of a fix point makes this appear unlikely to me, while on the other hand the late-bound proposal doesn't do a fix point until one is explicitly requested)

About performance, Nix will have to call into the .nix code for every merge anyway, as the merge function is provided by the .nix. I agree that it may, maybe, have a moderate memory improvement (by not keeping in memory a copy of the parameter list for each attribute of the set), but this sounds like premature optimization. Oh, and I think that postponing the fix point until the time when it is actually required will already help performance.

As we're talking about a redesign of the way the whole nixpkgs ecosystem works, that will likely break lots of stuff, I think getting ourselves locked up in specifics (by making nix implement them before we even start migrating) would be more harmful than first keeping nix ext-attrset-agnostic (with just late-bound sets and metadata syntax -- two orthogonal features), then porting nixpkgs and tweaking our ext-attrset implementation, and finally, once it has stabilized and we're happy with it, integrate the resulting ext-attrset idea in nix -- if it can indeed bring significant speed/memory gains.

@7c6f434c
Copy link
Member

7c6f434c commented Nov 6, 2018

@Ekleog isn't there a problem that migrated Nixpkgs might be too slow to evaluate without direct Nix support for the new features?

@Ekleog
Copy link
Member Author

Ekleog commented Nov 6, 2018 via email

@oxij
Copy link
Member

oxij commented Feb 26, 2019

Also see my critique of https://gist.github.com/edolstra/29ce9d8ea399b703a7023073b0dbc00d in NixOS/nixpkgs#56227 (comment) where I argue that 80% of the problems it tries to address can be solved by simply adding optional types to attrsets and .override should be (mostly) eliminated away.

@Ekleog
Copy link
Member Author

Ekleog commented May 16, 2019

If I read correctly, the latest flakes proposal has changed to no longer actually include extensible attrsets: https://github.com/NixOS/nix/blob/b0fc5bcee9f74c717d8ca564c193a5ad7846e5c7/doc/flakes/design.md (side-note: I personally like that new design much more than the one linked in the top-post)

If I am not mistaken, then I think this issue can be closed, as extensible attrsets are no longer under discussion. (not closing myself for now as maybe flakes and extensible attrsets are under parallel discussion)

@stale
Copy link

stale bot commented Feb 20, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Feb 20, 2021
@Ekleog
Copy link
Member Author

Ekleog commented Feb 21, 2021

Well, let's close I guess

@Ekleog Ekleog closed this as completed Feb 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants