From 4a5f2bd6e88c1bc3e16c0c2a19c9d5e65b3a1830 Mon Sep 17 00:00:00 2001 From: adisbladis Date: Wed, 13 Dec 2023 21:52:30 +1300 Subject: [PATCH] stdenv/check-meta: Use bespoke type checking Aka `checkMeta` goes brrr. Using the module system type checking works OK & generates good error messages. The performance of using it however is terrible because of the value merging it does being very allocation heavy. By implementing a very minimal type checker we can drastically improve the performance when nixpkgs is evaluated with `checkMeta = true`. --- pkgs/stdenv/generic/check-meta.nix | 85 +++++++++++++----------------- pkgs/stdenv/generic/meta-types.nix | 76 ++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 47 deletions(-) create mode 100644 pkgs/stdenv/generic/meta-types.nix diff --git a/pkgs/stdenv/generic/check-meta.nix b/pkgs/stdenv/generic/check-meta.nix index 62a6cd8ef02e..63c853e3dc31 100644 --- a/pkgs/stdenv/generic/check-meta.nix +++ b/pkgs/stdenv/generic/check-meta.nix @@ -13,17 +13,15 @@ let findFirst isDerivation length - mapAttrsToList - mergeDefinitions + concatMap mutuallyExclusive optional optionalAttrs optionalString optionals - remove - unknownModule isAttrs isString + mapAttrs ; inherit (lib.lists) @@ -283,44 +281,37 @@ let isEnabled = findFirst (x: x == reason) null showWarnings; in if isEnabled != null then builtins.trace msg true else true; - # Deep type-checking. Note that calling `type.check` is not enough: see `lib.mkOptionType`'s documentation. - # We don't include this in lib for now because this function is flawed: it accepts things like `mkIf true 42`. - typeCheck = type: value: let - merged = mergeDefinitions [ ] type [ - { file = unknownModule; inherit value; } - ]; - eval = builtins.tryEval (builtins.deepSeq merged.mergedValue null); - in eval.success; - - # TODO make this into a proper module and use the generic option documentation generation? metaTypes = let - inherit (lib.types) - anything - attrsOf - bool - either - int - listOf - mkOptionType - str - unspecified - ; - - platforms = listOf (either str (attrsOf anything)); # see lib.meta.platformMatch + types = import ./meta-types.nix { inherit lib; }; + inherit (types) str union int attrs attrsOf any listOf bool; + platforms = listOf (union [ str (attrsOf any) ]); # see lib.meta.platformMatch in { # These keys are documented description = str; mainProgram = str; longDescription = str; branch = str; - homepage = either (listOf str) str; + homepage = union [ + (listOf str) + str + ]; downloadPage = str; - changelog = either (listOf str) str; + changelog = union [ + (listOf str) + str + ]; license = let - licenseType = either (attrsOf anything) str; # TODO disallow `str` licenses, use a module - in either licenseType (listOf licenseType); - sourceProvenance = listOf lib.types.attrs; - maintainers = listOf (attrsOf anything); # TODO use the maintainer type from lib/tests/maintainer-module.nix + # TODO disallow `str` licenses, use a module + licenseType = union [ + (attrsOf any) + str + ]; + in union [ + (listOf licenseType) + licenseType + ]; + sourceProvenance = listOf attrs; + maintainers = listOf (attrsOf any); # TODO use the maintainer type from lib/tests/maintainer-module.nix priority = int; pkgConfigModules = listOf str; inherit platforms; @@ -329,16 +320,13 @@ let unfree = bool; unsupported = bool; insecure = bool; - # TODO: refactor once something like Profpatsch's types-simple will land - # This is currently dead code due to https://github.com/NixOS/nix/issues/2532 - tests = attrsOf (mkOptionType { + tests = { name = "test"; - check = x: x == {} || ( # Accept {} for tests that are unsupported + verify = x: x == {} || ( # Accept {} for tests that are unsupported isDerivation x && x ? meta.timeout ); - merge = lib.options.mergeOneOption; - }); + }; timeout = int; # Needed for Hydra to expose channel tarballs: @@ -354,7 +342,7 @@ let executables = listOf str; outputsToInstall = listOf str; position = str; - available = unspecified; + available = any; isBuildPythonPackage = platforms; schedulingPriority = int; isFcitxEngine = bool; @@ -363,17 +351,20 @@ let badPlatforms = platforms; }; - checkMetaAttr = k: v: + checkMetaAttr = let + # Map attrs directly to the verify function for performance + metaTypes' = mapAttrs (_: t: t.verify) metaTypes; + in k: v: if metaTypes?${k} then - if typeCheck metaTypes.${k} v then - null + if metaTypes'.${k} v then + [ ] else - "key 'meta.${k}' has invalid value; expected ${metaTypes.${k}.description}, got\n ${ + [ "key 'meta.${k}' has invalid value; expected ${metaTypes.${k}.name}, got\n ${ lib.generators.toPretty { indent = " "; } v - }" + }" ] else - "key 'meta.${k}' is unrecognized; expected one of: \n [${concatMapStringsSep ", " (x: "'${x}'") (attrNames metaTypes)}]"; - checkMeta = meta: optionals config.checkMeta (remove null (mapAttrsToList checkMetaAttr meta)); + [ "key 'meta.${k}' is unrecognized; expected one of: \n [${concatMapStringsSep ", " (x: "'${x}'") (attrNames metaTypes)}]" ]; + checkMeta = meta: optionals config.checkMeta (concatMap (attr: checkMetaAttr attr meta.${attr}) (attrNames meta)); checkOutputsToInstall = attrs: let expectedOutputs = attrs.meta.outputsToInstall or []; diff --git a/pkgs/stdenv/generic/meta-types.nix b/pkgs/stdenv/generic/meta-types.nix new file mode 100644 index 000000000000..ddbd1daca696 --- /dev/null +++ b/pkgs/stdenv/generic/meta-types.nix @@ -0,0 +1,76 @@ +{ lib }: +# Simple internal type checks for meta. +# This file is not a stable interface and may be changed arbitrarily. +# +# TODO: add a method to the module system types +# see https://github.com/NixOS/nixpkgs/pull/273935#issuecomment-1854173100 +let + inherit (builtins) isString isInt isAttrs isList all any attrValues isFunction isBool concatStringsSep isFloat; + isTypeDef = t: isAttrs t && t ? name && isString t.name && t ? verify && isFunction t.verify; + +in +lib.fix (self: { + string = { + name = "string"; + verify = isString; + }; + str = self.string; # Type alias + + any = { + name = "any"; + verify = _: true; + }; + + int = { + name = "int"; + verify = isInt; + }; + + float = { + name = "float"; + verify = isFloat; + }; + + bool = { + name = "bool"; + verify = isBool; + }; + + attrs = { + name = "attrs"; + verify = isAttrs; + }; + + list = { + name = "list"; + verify = isList; + }; + + attrsOf = t: assert isTypeDef t; let + inherit (t) verify; + in { + name = "attrsOf<${t.name}>"; + verify = + # attrsOf can be optimised to just isAttrs + if t == self.any then isAttrs + else attrs: isAttrs attrs && all verify (attrValues attrs); + }; + + listOf = t: assert isTypeDef t; let + inherit (t) verify; + in { + name = "listOf<${t.name}>"; + verify = + # listOf can be optimised to just isList + if t == self.any then isList + else v: isList v && all verify v; + }; + + union = types: assert all isTypeDef types; let + # Store a list of functions so we don't have to pay the cost of attrset lookups at runtime. + funcs = map (t: t.verify) types; + in { + name = "union<${concatStringsSep "," (map (t: t.name) types)}>"; + verify = v: any (func: func v) funcs; + }; +})