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
haskell-lib: Create flipped version of haskell-lib #100732
Conversation
More or less closes NixOS#37177 Created with https://github.com/expipiplus1/nix-flipper
I don‘t have a strong opinion about this. On the one hand this is neat. On the other hand I think the haskell.lib is already a very crowded place and this are a lot lines of code. And it never occured to me to be annoyed by the order of arguments. I think if we had the So, yeah, not sure … (But I don‘t intend and don‘t feel like I have the authority to veto this.) Maybe if someone showed me some real examples of code that would be much prettier with this I would be more enthusiastic. |
@maralorn Here's the first example which came to mind: https://github.com/expipiplus1/vulkan/blob/9e3229904c170d1e92c80e574dc610e97e36c1c9/examples/default.nix#L29-L31, but I've bumped into this enough times that it became worth the 30 minutes writing the generation script :) The irritation here is that one has to read this expression inside out (or outside in) because the It's not like this is impossible to read, but it is a bit of an irritation.
Could certainly be made more compact by having something like
Yes please! I suggest A more sensible midway point might be to include a function in vulkan-utils = foldEndo [
(addExtraLibrary pkgs.vulkan-headers)
doHaddock
disableLibraryProfiling
] (self.callCabal2nix "" ../utils { }); Of course I think this is nicest vulkan-utils = (addExtraLibrary pkgs.vulkan-headers) ∘ doHaddock
∘ disableLibraryProfiling $ self.callCabal2nix "" ../utils { }; |
Okay, that example is convincing. I really like the I think in general I am in favor of generating less code. So I have slight preference to the The remaining question is, do we even want functions that can‘t be flipped in the flipped attrSet? To be honest I wouldn‘t be surprised if @peti had some more concrete opinions about it, so we probably should wait a while for him to answer. |
If it's best, it shouldn't take more than a few minutes tweaking the script.
I'd like them in there because it means one only has to import
Absolutely, I'm in no hurry at all. |
I'm not sure whether I see when and where those flipped libraries functions are supposed to be used. I mean, surely it's not a good idea to mix both variants in the same file, is it? So ... since all our files in Nixpkgs are pretty much committed to the original "unflipped" version, where are those flipped variants supposed to be used? |
Why, I don't think it should be hard to change this? (Not that I'm advocating for any change in nixpkgs).
In the code of people who might otherwise use |
I guess we could start of by importing unflipped qualified. And maybe we could use it in new files or rewrite some of the old. (Although I see the pain point of never knowing which way around the arguments are in a certain file …) But for me having this for downstream users is already a benefit. |
I think that switching over everything in nixpkgs at once would be easier than using one-here one-there and having the stress that comes with that. However, I really don't want to distract or gate this PR on its possible application in nixpkgs, it certainly wasn't the intention! :) |
I've just noticed that Edit: and |
In the meantime, I've done some work with hnix to make making programs to modify nix code even nicer than the aforementioned hacky script, https://github.com/expipiplus1/update-nix-fetchgit/blob/master/src/Nix/Match/Typed.hs If a polished script to generate the flipped version is required, it could be done. |
An example from nixpkgs (haskell-updates branch atm), although it's a bit longer I think it's much easier to see what's going on {
update-nix-fetchgit = let deps = [ pkgs.git pkgs.nix pkgs.nix-prefetch-git ];
in generateOptparseApplicativeCompletion "update-nix-fetchgit" (overrideCabal
(addTestToolDepends (super.update-nix-fetchgit.overrideScope (self: super: {
optparse-generic = self.optparse-generic_1_4_4;
optparse-applicative = self.optparse-applicative_0_16_0_0;
})) deps) (drv: {
buildTools = drv.buildTools or [ ] ++ [ pkgs.makeWrapper ];
postInstall = drv.postInstall or "" + ''
wrapProgram "$out/bin/update-nix-fetchgit" --prefix 'PATH' ':' "${
pkgs.lib.makeBinPath deps
}"
'';
}));
# With flipped functions
update-nix-fetchgit = let
deps = [ pkgs.git pkgs.nix pkgs.nix-prefetch-git ];
compose = foldr (f: g: x: f (g x)) (x: x);
in compose [
(generateOptparseApplicativeCompletion "update-nix-fetchgit")
(addTestToolDepends deps)
(overrideCabal (drv: {
buildTools = drv.buildTools or [] ++ [ pkgs.makeWrapper ];
postInstall = drv.postInstall or "" + ''
wrapProgram "$out/bin/update-nix-fetchgit" --prefix 'PATH' ':' "${
pkgs.lib.makeBinPath deps
}"
'';
}))
(drv:
drv.overrideScope (self: super: {
optparse-generic = self.optparse-generic_1_4_4;
optparse-applicative = self.optparse-applicative_0_16_0_0;
})
)
] super.update-nix-fetchgit;
} |
What's the word on this? I'm happy to do whatever cleanup is necessary. |
I have the following problem. I agree that the flipped style gives nicer code, but IMHO the opportunity to change the functions in Nixpkgs has passed. It's too late. We have tons of code that use the current argument order and we cannot chance our functions without causing a major annoyance to our users. Now, adding a second set of functions that do the same thing but with their arguments reversed feels like a really bad idea. The last thing I want is to have code within Nixpkgs use two different sets of functions that share the same name but have their arguments reversed. That is not going to improve the readability of that code, which kinda defeats the purpose of this whole endeavor. So, basically, I think that these new functions give nicer code and I am all for people using them, but I am against people using them in Nixpkgs. Which begs the question of whether these functions should be a part of Nixpkgs ... and I tend to think: no, they should not. Can you see my point? |
Absolutely, I certainly wouldn't advocate for mixing the two. I think where we disagree is whether this should be in nixpkgs for all downstream users. One potential way forward would be to deprecate the use of the current |
Yes, that would be possible. Personally, I don't think the benefits are worth the effort, though. If someone else wants to pick up that torch and run with it, I would not object. I'm just not going to do it myself. |
I marked this as stale due to inactivity. → More info |
More or less closes #37177
Created with this hacky script: https://github.com/expipiplus1/nix-flipper
WIP, could use a bit of neatening, and probably some checking!
would it be worth making it 100% automated to ease keeping this up to
date?
lib
in terms oflib-flipped
?haskell.lib.flipped
a better place anhaskell.lib-flipped
bil.nix
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)