From 82e538430bd82f3c10dec6905f52280b40258afd Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Wed, 19 Feb 2020 17:34:00 +0100 Subject: [PATCH] nixos/nixpkgs: add override/merging capabilities from the module system to the `nixpkgs.config` option The `nixpkgs.config`[1] option is just a plain attribute-set that is directly passed to ``. This has the following drawbacks: * Missing override/merging capabilities: you can't use module-system features like `mkOverride`/`mkOrder`/`mkIf`/etc. * Conflicting definitions don't cause an evaluation error. The merging is basically a `fold recursiveUpdate {} defs` which means that the last declaration of a value inside `nixpkgs.config` "wins". For example, ``` nix { pkgs, lib, ... }: { nixpkgs = lib.mkMerge [ { config.allowUnfree = false; } { config.allowUnfree = true; } ]; environment.systemPackages = [ pkgs.spotify /* an unfree package */ ]; } ``` would evaluate. Another attempt to use the module-system for nixpkgs-config stalled after a long discussion[2]. This change aims to be minimally-invasive by applying the filters from `` onto the attribute-set without constraining the attributes with a predefined option structure (or in other words: you don't need to change anything in the module-system to add a new nixpkgs-config value). This is done by replacing the old `configType` with a new option-type that emulates the merging behavior of the module-system: * All values are merged together into a single attr-set to ensure that all values that are defined in any included module are taken into account. * Each sub-attribute that is an attr-set and has a `_type`-field (which defines which `mk*`-filter was used for the declaration) will be passed through priority/discharge/ordering filters to resolve all `mk*` statements in `nixpkgs.config`. * The result will be merged together to ensure that no conflicting definitions exist while sequential values will be merged together. Due to the validation mechanisms from the module-system, the following case will break with an evaluation error now: * When declaring values of different types to a single nixpkgs-config option, the eval will fail now (rather than silently replacing the values in `lib.recursiveUpdate`): For example, ``` nix { lib, ... }: { nixpkgs = lib.mkMerge [ { config.chromium.enableWidevine = true; } { config.chromium = 23; } ]; } ``` will break with the following error: ``` Cannot merge definitions of `nixpkgs.config.chromium' given in `' and `'. ``` (Please note that there might be some more edge-cases I'm just missing while writing the commit-message). Additionally the following issues aren't fixed yet: * There's no support for anything like `types.separatedString`. As string-concatenation during merge-phase may be harmful[3] (and there's no way to differentiate a separatedString from a "normal" str without using ``), evaluations like this will always fail for now: ``` nix { lib, ... }: { nixpkgs = lib.mkMerge [ { config.random = '' foo ''; } { config.random = '' bar ''; } ]; } ``` ``` The unique option `nixpkgs.config.random' is defined multiple times, in: - - . ``` Please note that the only reason why this yields `` is because the values were defined in a `mkMerge`. Since `loc` is passed properly to the error-handler, the actual file where a value is declared would be shown if the options were defined in different files. * Expressions inside a `mkMerge` aren't evaluated yet. For instance, in the following expression not all `mk*` would be resolved properly: ``` nix { lib, ... }: { nixpkgs.config = { chromium = lib.mkMerge [ { randomValue = mkIf true "something"; } ]; }; } ``` ``` { chromium = { randomValue = { _type = "if"; condition = true; content = "something"; }; }; } ``` Please note that this only applies to ordering & conditionals, overrides inside a `mkMerge` are resolved without any problems. [1] https://nixos.org/nixpkgs/manual/#idm140737322665904 [2] https://github.com/NixOS/nixpkgs/pull/56227 [3] https://github.com/NixOS/nixpkgs/commit/03391cd336b128a1639c648baf0f6c1a1271e0d2 --- nixos/modules/misc/nixpkgs.nix | 75 ++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 25 deletions(-) diff --git a/nixos/modules/misc/nixpkgs.nix b/nixos/modules/misc/nixpkgs.nix index afb74581e2398..63c29fb0dda59 100644 --- a/nixos/modules/misc/nixpkgs.nix +++ b/nixos/modules/misc/nixpkgs.nix @@ -14,32 +14,57 @@ 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 { + mergableConfigType = 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: fold (def: mergeConfig def.value) {}; + check = isConfig; + merge = loc: defs: + let + # Ensure that each attribute from nixpkgs.config appears at the + # first iteration to make sure that nothing is lost. + allValues = fold recursiveUpdate {} defs; + + # Recurses through the `config` attr-set for nixpkgs and ensures that + # no conflicting definitions exist and that overrides are resolved properly. + mapFun = path: value: + if isAttrs value && !(value ? _type) then value + else let + # Evaluate mkIf/mkMerge with `lib.dischargeProperties` and append the + # value to the list of declarations. If a value is `null`, it means that + # it was not declared in one of the configs and can be ignored. + appendVal = all: value_: next: + let + # FIXME support `mkMerge [{ snens = mkIf cond val; }]` + discharged = dischargeProperties value_; + # FIXME check if there's a case where `discharged` is `[]` and + # therefore breaks `head`. + value = let h = head discharged; in if isAttrs h + then foldl recursiveUpdate {} discharged + else h; + in all ++ (if value == null then [] else [{ + inherit (next) file; + inherit value; + }]); + + # Gather all declarations of a value in the `nixpkgs.config` attr-set. + allDeclarations = foldl + (all: next: appendVal all (optCall (attrByPath path null next) finalPkgs) next) + [] + defs; + + # Evaluate all overrides (defined using mkOverride/mkForce/etc) and pick the + # value with the highes priority. + result = sortProperties (filterOverrides allDeclarations); + + # Merge all declarations (if a list or an attr-set is given, those can be merged together), + # if a "unique" value is given, it's ensured that no conflicting definitions exist. + # + # FIXME This is missing support for separatedString! + merge = let h = (head result).value or null; in + if isList h || isAttrs h then mergeDefaultOption else mergeOneOption; + in + merge (["nixpkgs" "config"] ++ (tail path)) result; + in mapAttrsRecursiveCond (x: !(x ? _type)) mapFun allValues; }; overlayType = mkOptionType { @@ -113,7 +138,7 @@ in '' { allowBroken = true; allowUnfree = true; } ''; - type = configType; + type = mergableConfigType; description = '' The configuration of the Nix Packages collection. (For details, see the Nixpkgs documentation.) It allows you to set