Skip to content

lib.makeOverridable: observe <result>.__overriders #454997

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.

Sign up for GitHub

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

Open
ShamrockLee wants to merge 3 commits into
base: master
Choose a base branch
from
Open

lib.makeOverridable: observe <result>.__overriders #454997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
97bb209
lib.makeOverridable: observe existing overriders in <pkg>.__overriders
ShamrockLee Oct 23, 2025
4bd7f34
lib.customisation: makeScope, makeScopeWithSplicing': provide __overr…
ShamrockLee Oct 23, 2025
cbdd103
lib.makeExtensibleWithCustomName: provide __overriders
ShamrockLee Oct 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions lib/customisation.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ let
extends
toFunction
id
genAttrs
subtractLists
;
inherit (lib.strings) levenshtein levenshteinAtMost;

Expand Down Expand Up @@ -194,10 +196,19 @@ rec {
);
# Change the result of the function call by applying g to it
overrideResult = g: makeOverridable (mirrorArgs (args: g (f args))) origArgs;
newOverriderNames =
filter (n: n != "override" && n != "overrideDerivation" && result ? ${n}) (
subtractLists (result.__overriders or [ ]) [ "overrideAttrs" ] ++ result.__overriders or [ ]
)
++ [
"override"
"overrideDerivation"
];
in
if isAttrs result then
result
// {
__overriders = newOverriderNames;
override = overrideArgs;
overrideDerivation = fdrv: overrideResult (x: overrideDerivation x fdrv);
${if result ? overrideAttrs then "overrideAttrs" else null} =
Expand All @@ -213,10 +224,16 @@ rec {
# design/tech debt issue: https://github.com/NixOS/nixpkgs/issues/273815
fdrv: overrideResult (x: x.overrideAttrs fdrv);
}
// optionalAttrs (result ? __overriders) (
genAttrs (filter (
x: x != "override" && x != "overrideAttrs" && x != "overrideDerivation" && result ? ${x}
) result.__overriders) (name: fdrv: overrideResult (x: x.${name} fdrv))
Comment on lines +228 to +230
Copy link
Copy Markdown
Member

@emilazy emilazy Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An interesting option here would be to explicitly allow override here, which could potentially be used to allow migrating foo = bar.override { … }; definitions into by-name without breaking override interfaces. However, there are other complications there, with regards to the mirrorArgs stuff. (I got something to allow composing override interfaces working locally, but wasn’t entirely happy with the trade-offs.)

Copy link
Copy Markdown
Contributor Author

@ShamrockLee ShamrockLee Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An interesting option here would be to explicitly allow override here

Could you please elaborate?

We cannot implement the override attribute with overrideResult in lib.makeOverridable. It will always get overwritten.

As for overrideAttrs, we could do it here instead of defining explicitly above, but I'm worried about the potential performance implication.

Copy link
Copy Markdown
Member

@emilazy emilazy Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can’t directly recall what I was thinking at the time, but I believe that what I meant was that we could allow packages to expose their own .override field rather than always overriding it, which lets you callPackage/.override some other package in by-name without having the usual callPackage override interface clobber the one you’d like to inherit. (Hence why we have some overrides directly in all-packages.nix still.) Though in further thinking about this whole class of problem I decided it was more complicated than that, because sometimes you don’t want to clobber the callPackage override interface in that way.

)
else if isFunction result then
# Transform the result into a functor while propagating its arguments
setFunctionArgs result (functionArgs result)
// {
__overriders = newOverriderNames;
override = overrideArgs;
}
else
Expand Down Expand Up @@ -639,6 +656,7 @@ rec {
newScope = scope: newScope (self // scope);
overrideScope = g: makeScope newScope (extends g f);
packages = f;
__overriders = [ "overrideScope" ];
};
in
self;
Expand Down Expand Up @@ -757,6 +775,7 @@ rec {
f = extends g f;
});
packages = f;
__overriders = [ "overrideScope" ];
};
in
self;
Expand Down
1 change: 1 addition & 0 deletions lib/fixed-points.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ rec {
(rattrs self)
// {
${extenderName} = f: makeExtensibleWithCustomName extenderName (extends f rattrs);
__overriders = [ extenderName ];
}
);

Expand Down
67 changes: 67 additions & 0 deletions lib/tests/misc.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ let
meta
mod
nameValuePair
optionalAttrs
optionalDrvAttr
optionAttrSetToDocList
overrideExisting
Expand Down Expand Up @@ -4810,16 +4811,19 @@ runTests {
directory = ./packages-from-directory/scope;
});
expected = lib.recurseIntoAttrs {
__overriders = [ "overrideScope" ];
a = "a";
b = "b";
# Note: Other files/directories in `./test-data/c/` are ignored and can be
# used by `package.nix`.
c = "c";
my-namespace = lib.recurseIntoAttrs {
__overriders = [ "overrideScope" ];
d = "d";
e = "e";
f = "f";
my-sub-namespace = lib.recurseIntoAttrs {
__overriders = [ "overrideScope" ];
g = "g";
h = "h";
};
Expand Down Expand Up @@ -4876,6 +4880,69 @@ runTests {
expected = {
foo = "foo-value-custom";
bar = "bar-value-custom";

# Test if `makeOverridable` correctly handles `result.ovrerrideScope`,
# so that `makeScope`-constructed, `callPackageWith`-called scope
# won't lose `<scope>.override` after `<scope>.overrideScope` overriding.
testMakeOverridableForCallPackageAndScope =
let
scope-orig = callPackageWith (lib // { a = 3; }) (
{
callPackagesWith,
makeScope,
a,
}:
makeScope callPackagesWith (final: {
inherit a;
b = 5;
c = final.a * final.b;
})
) { };
scope-os = scope-orig.overrideScope (
final: previous: {
b = 7;
}
);
scope-os-ov = scope-os.override { a = 11; };
in
{
expr = {
scope-orig-a = scope-orig.a;
scope-orig-b = scope-orig.b;
scope-orig-c = scope-orig.c;
scope-orig-has-override = scope-orig ? override;
scope-orig-has-overrideScope = scope-orig ? overrideScope;
}
// optionalAttrs (scope-orig ? overrideScope) {
scope-os-a = scope-os.a;
scope-os-b = scope-os.b;
scope-os-c = scope-os.c;
scope-os-has-override = scope-os ? override;
scope-os-has-overrideScope = scope-os ? overrideScope;
}
// optionalAttrs (scope-os ? override) {
scope-os-ov-a = scope-os-ov.a;
scope-os-ov-b = scope-os-ov.b;
scope-os-ov-c = scope-os-ov.c;
scope-os-ov-has-override = scope-os-ov ? override;
scope-os-ov-has-overrideScope = scope-os-ov ? overrideScope;
};
expected = rec {
scope-orig-a = 3;
scope-orig-b = 5;
scope-orig-c = scope-orig-a * scope-orig-b;
scope-orig-has-override = true;
scope-orig-has-overrideScope = true;
scope-os-a = scope-orig-a;
scope-os-b = 7;
scope-os-c = scope-os-a * scope-os-b;
scope-os-has-override = true;
scope-os-has-overrideScope = true;
scope-os-ov-a = 11;
scope-os-ov-b = scope-os-b;
scope-os-ov-c = scope-os-ov-a * scope-os-ov-b;
scope-os-ov-has-override = true;
scope-os-ov-has-overrideScope = true;
};
};

Expand Down
Loading