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

A function to compose overlays? #33258

Closed
michalrus opened this issue Jan 1, 2018 · 20 comments
Closed

A function to compose overlays? #33258

michalrus opened this issue Jan 1, 2018 · 20 comments
Assignees

Comments

@michalrus
Copy link
Member

michalrus commented Jan 1, 2018

Wouldn’t it be useful to have a function in lib to compose overlays? It’s short to define, but perhaps non-trivial (given we have no types in Nix — nb., I’m not sure if my definition is really correct):

{
  composeOverlays = overlays: self: super:
    lib.foldl' (lib.flip lib.extends) (lib.const super) overlays self;
}

It would also be more in line with what is promised in these slides: https://nbp.github.io/slides/NixCon/2017.NixpkgsOverlays/ (→ slide 25).

This way the user can define expression files that themselves are valid overlays (not lists of overlays) and which themselves comprise of other valid overlays.

I.e. the user can compose the overlays…

@michalrus
Copy link
Member Author

/cc @edolstra @nbp @peti @copumpkin @ttuegel

@michalrus
Copy link
Member Author

I’m not sure if my definition is really correct

It seems to be fine.

With it, you can do even nested overlays that work as expected (and definition sites need not—and should not—know about the nesting):

https://github.com/michalrus/dotfiles/blob/c5c352b0dccb70a0515277ab79f591080cd03e6c/nixos-config/overlays/default.nix#L25-L49.

It’s not super convenient, but at least it’s something. ¯\_(ツ)_/¯

@ttuegel
Copy link
Member

ttuegel commented Jan 1, 2018

About the name: this is not only useful to compose overlays, but also Haskell, Qt, and Emacs package sets. (All three use the self: super: pattern.)

@michalrus
Copy link
Member Author

@volth, the comment says of two only? And to my eye, the implementation confirms that:

# Compose two extending functions of the type expected by 'extends'
# into one where changes made in the first are available in the
# 'super' of the second
composeExtensions =
f: g: self: super:
let fApplied = f self super;
super' = super // fApplied;
in fApplied // g self super';

@peti peti closed this as completed Jan 2, 2018
@FRidh FRidh self-assigned this Jan 18, 2018
@FRidh
Copy link
Member

FRidh commented Jan 18, 2018

Reopening because I think such function should be added to lib.

@FRidh FRidh reopened this Jan 18, 2018
@michalrus
Copy link
Member Author

@FRidh thank you!

@michalrus
Copy link
Member Author

@volth, would it be very terrible to add your definition under the name of composeNExtensions? It’d probably be better to name that one compose2Extensions and this one composeExtensions but… too late.

@copumpkin
Copy link
Member

Eh, from the standpoint of "provide the simplest building blocks to be composed as needed by users" there's a lot of precedent (in other functional languages, math, etc.) for just providing the two-argument version and letting people fold the two together.

Here's a concept I like in this space:

The Fairbairn threshold is the point at which the effort of looking up or
keeping track of the definition is outweighed by the effort of rederiving
it or inlining it.
[...]
The primary use of the Fairbairn threshold is as a litmus test to avoid
giving names to trivial compositions, as there are a potentially explosive
number of them. In particular any method whose definition isn't much longer
than its name (e.g. fooBar = foo . bar) falls below the threshold.
[...]
The effect is to encourage simple combinators that can be used in multiple
situations, while avoiding naming the explosive number of combinations of
those combinators.

@michalrus
Copy link
Member Author

michalrus commented Jan 18, 2018

@volth, emm, is the sum/product example an argument for or against? Other fine libraries do define them:

https://hackage.haskell.org/package/base-4.10.1.0/docs/Data-Foldable.html#v:sum

@copumpkin, if this is not needed in general, fine. I can define it myself, no problem. However, intuitively (but my intuition in Nixpkgs is very lacking!), I would say that from an OS declaration standpoint, I’d be much more likely to use the list version rather than the pair version. If you could quickly, in 1-2 sentences, discuss why the pair version is more useful in general, I’d love to read that!

@peti
Copy link
Member

peti commented Jan 18, 2018

👎 on adding a function that's just a trivial fold over composeExtensions. I'd be in favor of extending the documentation of composeExtensions to point out this use-case, but adding yet another function to accomplish nothing that can't be accomplished already is not a good idea IMHO.

@michalrus
Copy link
Member Author

@peti @copumpkin @volth, why is there lib.map that’s also trivial to define using fold? (ノ^_^)ノ

Adding documentation is probably even more work than adding the function.

But, I guess, the real question is why that lib.composeExtensions function takes a pair and not a list. 🤔 Why is that more useful in declaring an operating system? If I want to compose just two, there are… 2-element lists available out there?

@michalrus
Copy link
Member Author

Or maybe!

I’d like to propose that we add my trivial fold and remove this silliness, in line with your argumentation, if I get it right:

nixpkgs/lib/lists.nix

Lines 80 to 94 in bdb2985

/* Map with index starting from 0
Example:
imap0 (i: v: "${v}-${toString i}") ["a" "b"]
=> [ "a-0" "b-1" ]
*/
imap0 = f: list: genList (n: f n (elemAt list n)) (length list);
/* Map with index starting from 1
Example:
imap1 (i: v: "${v}-${toString i}") ["a" "b"]
=> [ "a-1" "b-2" ]
*/
imap1 = f: list: genList (n: f (n + 1) (elemAt list n)) (length list);

Net, we’d all win. 😀

@FRidh
Copy link
Member

FRidh commented Jan 18, 2018

Eh, from the standpoint of "provide the simplest building blocks to be composed as needed by users" there's a lot of precedent (in other functional languages, math, etc.) for just providing the two-argument version and letting people fold the two together.

I agree, although I would like to note that this is Nixpkgs, not Nix. Here we have a wider audience to consider.

emm, is the sum/product example an argument for or against? Other fine libraries do define them:

In my opinion overlays is a fundamental concept of how we (want to) do package sets in Nixpkgs, and therefore I think it should become trivial to merge extensions using a single function, just as most languages offer a sum or product function for their fundamental building blocks: integers and floats.

Now that we recommend using overlays in various places throughout Nixpkgs, I don't think users should need to know or learn how to use a higher-order function like fold. Sure, this new function would still be a higher-order function but thinking in overlays it becomes trivial to understand how it should be used.

@Andrei-Pozolotin
Copy link

+1

@michalrus
Copy link
Member Author

@stale

This comment has been minimized.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 4, 2020
@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/infinite-recursion-when-composing-overlays/7594/1

@deliciouslytyped
Copy link
Contributor

deliciouslytyped commented Jul 1, 2020

Common idioms form a domain specific language. If people keep independently rediscovering and implementing their own helper functions (as linked above), that should be obvious and definitive evidence of the need for a library function.

I think someone comfortable with it just needs to make the PR and get it merged. :)

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 1, 2020
@FRidh
Copy link
Member

FRidh commented Dec 19, 2020

The proposed function was sneaked in with #103061. I don't like the name however.

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/namespacing-scoping-a-group-of-packages/13782/10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants