Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions lib/tests/modules.sh
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,10 @@ checkConfigOutput 'ok' config.freeformItems.foo.bar ./adhoc-freeformType-survive
# because of an `extendModules` bug, issue 168767.
checkConfigOutput '^1$' config.sub.specialisation.value ./extendModules-168767-imports.nix

checkConfigOutput '^24$' config.result.test ./override-fn-type.nix
checkConfigOutput '^25$' config.result.test2 ./override-fn-type.nix
checkConfigOutput '^1$' config.result.test3 ./override-fn-type.nix

cat <<EOF
====== module tests ======
$pass Pass
Expand Down
34 changes: 34 additions & 0 deletions lib/tests/modules/override-fn-type.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{ lib, config, ... }: with lib; {
options.packageOverrides = mkOption {
default = const {};
type = types.overrideFnType;
};
options.result = mkOption {
type = types.attrs;
};
config = mkMerge [
{
packageOverrides = pkgs: {
test = pkgs.num + 1;
};
}
{
packageOverrides = pkgs: {
test2 = pkgs.test + 1;
};
}
{
packageOverrides = mkAfter (pkgs: {
test3 = 1;
});
}
{
packageOverrides = pkgs: {
test3 = 2;
};
}
{
result = config.packageOverrides { num = 23; };
}
];
}
23 changes: 23 additions & 0 deletions lib/types.nix
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

let
inherit (lib)
const
elem
flip
isAttrs
Expand Down Expand Up @@ -884,6 +885,28 @@ rec {
# Augment the given type with an additional type check function.
addCheck = elemType: check: elemType // { check = x: elemType.check x && check x; };

# EXPERIMENTAL
# Known issue:
# Overrides composition does not compose symmetrically, exposing the arbitrary
# nature of module `imports` ordering in a potentially harmful manner.
# TODO
# - Restrict override composition by `throw`-ing when any two layers contain the
# same attribute name, which may create an `imports` ordering dependency.
# (a layer is the result of applying one or more override functions)
# - Apply this restriction only to unordered layers, which means layers that
# have the same `mkOrder` number.
# - Document this type in the NixOS manual
overrideFnType = mkOptionType {
name = "overrides";
check = builtins.isFunction;
merge = const
(foldl'
(final: override: pkgs:
let
final' = final pkgs;
in final' // (override.value (pkgs // final')))
(const {}));
Comment on lines +902 to +908
Copy link
Member

Choose a reason for hiding this comment

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

This changes the merging behavior from before. Most notably the old merging function didn't propagate changes from earlier overrides to later ones, there's only one pkgs that is reused as the input for all override functions.

Copy link
Member

Choose a reason for hiding this comment

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

Is that functionTo attrs then?

Copy link
Member

Choose a reason for hiding this comment

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

Almost. I think not because of the optCall, which allows either functions or attribute sets directly

Copy link
Member

Choose a reason for hiding this comment

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

Ok, then it needs a coercedTo.

optCall can be replaced by lib.toFunction, but I guess we'll only need lib.const anyway.

};
};
};

Expand Down
30 changes: 1 addition & 29 deletions nixos/modules/misc/nixpkgs.nix
Original file line number Diff line number Diff line change
Expand Up @@ -14,34 +14,6 @@ let
then f x
else f;

mergeConfig = lhs_: rhs_:
let
lhs = optCall lhs_ { inherit pkgs; };
rhs = optCall rhs_ { inherit pkgs; };
in
recursiveUpdate lhs rhs //
optionalAttrs (lhs ? packageOverrides) {
packageOverrides = pkgs:
optCall lhs.packageOverrides pkgs //
optCall (attrByPath ["packageOverrides"] ({}) rhs) pkgs;
} //
optionalAttrs (lhs ? perlPackageOverrides) {
perlPackageOverrides = pkgs:
optCall lhs.perlPackageOverrides pkgs //
optCall (attrByPath ["perlPackageOverrides"] ({}) rhs) pkgs;
};

configType = mkOptionType {
name = "nixpkgs-config";
description = "nixpkgs config";
check = x:
let traceXIfNot = c:
if c x then true
else lib.traceSeqN 1 x false;
in traceXIfNot isConfig;
merge = args: foldr (def: mergeConfig def.value) {};
};

overlayType = mkOptionType {
name = "nixpkgs-overlay";
description = "nixpkgs overlay";
Expand Down Expand Up @@ -156,7 +128,7 @@ in
''
{ allowBroken = true; allowUnfree = true; }
'';
type = configType;
type = types.submodule ../../../pkgs/top-level/config.nix;
description = lib.mdDoc ''
The configuration of the Nix Packages collection. (For
details, see the Nixpkgs documentation.) It allows you to set
Expand Down
18 changes: 17 additions & 1 deletion pkgs/top-level/config.nix
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,26 @@ let
checkMeta = mkOption {
type = types.bool;
default = false;
description = ''
description = lib.mdDoc ''
Whether to check that the `meta` attribute of derivations are correct during evaluation time.
'';
};

packageOverrides = mkOption {
default = const {};
type = types.overrideFnType;
description = lib.mdDoc ''
**Not recommended.** Use `overlays` instead!
'';
};

perlPackageOverrides = mkOption {
default = const {};
type = types.overrideFnType;
description = lib.mdDoc ''
A function that takes the final set of Perl packages and returns an attribute set of packages to add or change.
Copy link
Member

Choose a reason for hiding this comment

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

In addition, the perlPackageOverrides is super weird, in that it doesn't even take the perl packages as an argument, but rather the main pkgs package set!

❯ nix-instantiate '<nixpkgs>' --arg config '{ perlPackageOverrides = pkgs: builtins.trace (builtins.head (builtins.attrNames pkgs)) {}; }' -A perlPackages.FileFinder
trace: AAAAAASomeThingsFailToEvaluate
trace: AAAAAASomeThingsFailToEvaluate
trace: AAAAAASomeThingsFailToEvaluate
warning: you did not specify '--add-root'; the result might be removed by the garbage collector
/nix/store/sf3bji32b4xw77xjh7w452pj15jq8gh6-perl5.34.1-File-Finder-0.53.drv

(this shows that the first attribute in the passed pkgs is pkgs.AAAAAASomeThingsFailToEvaluate)

Copy link
Member

Choose a reason for hiding this comment

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

Yikes, but it also kinda makes sense, because how else would you access pkgs. Actually doing your own fixpoint might work, but not with features like pkgsCross.

Anyway, thanks @infinisil for catching this and sorry @Ma27 for making a wrong suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

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

no worries, I also missed that!

Copy link
Member Author

Choose a reason for hiding this comment

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

Uhm... I obseve the same behavior on my local master (dfae1c1 currently):

❯ ~/Projects/nixpkgs master → nix-instantiate './.' --arg config '{ perlPackageOverrides = pkgs: builtins.trace (builtins.head (builtins.attrNames pkgs)) {}; }' -A perlPackages.FileFinder
trace: AAAAAASomeThingsFailToEvaluate
trace: AAAAAASomeThingsFailToEvaluate
trace: AAAAAASomeThingsFailToEvaluate
warning: you did not specify '--add-root'; the result might be removed by the garbage collector
/nix/store/f9qgccfq4p6n161jlkdf05qikmq4l9jj-perl5.36.0-File-Finder-0.53.drv

Am I missing something? oO

Copy link
Member

@infinisil infinisil Oct 20, 2022

Choose a reason for hiding this comment

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

That's as expected. This just demonstrates that perlPackageOverrides gets the pkgs as an argument and not pkgs.perlPackages, since there's only pkgs.AAAAAASomeThingsFailToEvaluate and no pkgs.perlPackages.AAAAAASomeThingsFailToEvaluate. Don't mind this specific attribute name, it's not related to this issue.

Copy link
Member

Choose a reason for hiding this comment

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

In addition, the perlPackageOverrides is super weird, in that it doesn't even take the perl packages as an argument, but rather the main pkgs package set!

It's funny. In a discussion today the inverse came up. Without a way to reference pkgs, mere two-function overlays are not good enough for distributing additions/changes to language-specific package sets, because those inevitably tend to need some things from pkgs... but which one?
So without a pkgs parameter it's not suitable for distributed use, and without a prev or self, it's not an overlay.
We need both! ... or do we? self is actually available in pkgs.
But yes, we do, because the location may be ambiguous. Is it haskellPackages or haskell.packages.ghc943?

'';
};
};

in {
Expand Down