-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
lib.customisation: uncurry makeScopeWithSplicing #245824
Conversation
Deeply-curried functions are pretty error-prone in untyped languages like Nix. This is a particularly bad case because `top-level/splice.nix` *also* declares a makeScopeWithSplicing, but it takes *two fewer arguments*. Let's switch to attrset-passing form, to provide some minimal level of sanity-checking.
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.
diff --git a/lib/customisation.nix b/lib/customisation.nix
index a46913dc5bde..4dfe75023b96 100644
--- a/lib/customisation.nix
+++ b/lib/customisation.nix
@@ -279,7 +279,7 @@ rec {
/* Like the above, but aims to support cross compilation. It's still ugly, but
hopefully it helps a little bit. */
- makeScopeWithSplicing = { splicePackages, newScope }: { otherSplices, keep, extra, f }:
+ makeScopeWithSplicing = { splicePackages, newScope, otherSplices, keep, extra, f }:
let
spliced0 = splicePackages {
pkgsBuildBuild = otherSplices.selfBuildBuild;
@@ -296,8 +296,7 @@ rec {
# N.B. the other stages of the package set spliced in are *not*
# overridden.
overrideScope = g: (makeScopeWithSplicing
- { inherit splicePackages newScope; }
- { inherit otherSplices keep extra;
+ { inherit splicePackages newScope otherSplices keep extra;
f = lib.fixedPoints.extends g f;
});
packages = f;
diff --git a/pkgs/top-level/splice.nix b/pkgs/top-level/splice.nix
index 57cedb12a2f5..01407c107a16 100644
--- a/pkgs/top-level/splice.nix
+++ b/pkgs/top-level/splice.nix
@@ -144,7 +144,7 @@ in
newScope = extra: lib.callPackageWith (splicedPackagesWithXorg // extra);
# prefill 2 fields of the function for convenience
- makeScopeWithSplicing = lib.makeScopeWithSplicing { inherit splicePackages; inherit (pkgs) newScope; };
+ makeScopeWithSplicing = a: lib.makeScopeWithSplicing ({ inherit splicePackages; inherit (pkgs) newScope; } // a);
# generate 'otherSplices' for 'makeScopeWithSplicing'
generateSplicesForMkScope = attr:
Alternative which would not require 2 input sets, which looks nicer if lib.makeScopeWithSplicing is used directly, IMO.
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.
functionArgs won't work on pkgs.makeScopeWithSplicing though
These are often unneeded by the user.
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 guess I can't "approve" my own PR, but I approve of the commit @Artturin pushed onto it.
This is not okay, this is a breaking change to |
You're right. For some reason I thought Will revert and reintroduce with a new name for the uncurried version. |
I have merged #245955 |
Thanks! |
Reimplemented with a different name and a backward-compatible shim: #245957 |
Description of changes
Deeply-curried functions are pretty error-prone in untyped languages like Nix. This is a particularly bad case because
top-level/splice.nix
also declares a makeScopeWithSplicing, but it takes two fewer arguments.Let's switch to attrset-passing form, to provide some minimal level of sanity-checking.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)