diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 264305299f1b..b687c1572d67 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -37,6 +37,7 @@ /pkgs/top-level/splice.nix @Ericson2314 @matthewbauer /pkgs/top-level/release-cross.nix @Ericson2314 @matthewbauer /pkgs/stdenv/generic @Ericson2314 @matthewbauer +/pkgs/stdenv/generic/check-meta.nix @Ericson2314 @matthewbauer @piegamesde /pkgs/stdenv/cross @Ericson2314 @matthewbauer /pkgs/build-support/cc-wrapper @Ericson2314 /pkgs/build-support/bintools-wrapper @Ericson2314 diff --git a/lib/types.nix b/lib/types.nix index 3750ba965558..ed1d5f44064f 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -119,8 +119,22 @@ rec { # - "composite": a phrase with an "of" connective # See the `optionDescriptionPhrase` function. , descriptionClass ? null - , # Function applied to each definition that should return true if - # its type-correct, false otherwise. + , # DO NOT USE WITHOUT KNOWING WHAT YOU ARE DOING! + # Function applied to each definition that must return false when a definition + # does not match the type. It should not check more than the root of the value, + # because checking nested values reduces laziness, leading to unnecessary + # infinite recursions in the module system. + # Further checks of nested values should be performed by throwing in + # the merge function. + # Strict and deep type checking can be performed by calling lib.deepSeq on + # the merged value. + # + # See https://github.com/NixOS/nixpkgs/pull/6794 that introduced this change, + # https://github.com/NixOS/nixpkgs/pull/173568 and + # https://github.com/NixOS/nixpkgs/pull/168295 that attempted to revert this, + # https://github.com/NixOS/nixpkgs/issues/191124 and + # https://github.com/NixOS/nixos-search/issues/391 for what happens if you ignore + # this disclaimer. check ? (x: true) , # Merge a list of definitions together into a single value. # This function is called with two arguments: the location of diff --git a/nixos/doc/manual/from_md/release-notes/rl-2211.section.xml b/nixos/doc/manual/from_md/release-notes/rl-2211.section.xml index 0f1dffc798e7..8f4a7dd002d3 100644 --- a/nixos/doc/manual/from_md/release-notes/rl-2211.section.xml +++ b/nixos/doc/manual/from_md/release-notes/rl-2211.section.xml @@ -474,6 +474,14 @@ maintainer to update the package. + + + The (previously undocumented) Nixpkgs configuration option + checkMeta now defaults to + true. This may cause evaluation failures + for packages with incorrect meta attribute. + + xow package removed along with the diff --git a/nixos/doc/manual/release-notes/rl-2211.section.md b/nixos/doc/manual/release-notes/rl-2211.section.md index 7214937781d2..f6515256dbc6 100644 --- a/nixos/doc/manual/release-notes/rl-2211.section.md +++ b/nixos/doc/manual/release-notes/rl-2211.section.md @@ -164,6 +164,9 @@ Available as [services.patroni](options.html#opt-services.patroni.enable). - riak package removed along with `services.riak` module, due to lack of maintainer to update the package. +- The (previously undocumented) Nixpkgs configuration option `checkMeta` now defaults to `true`. This may cause evaluation + failures for packages with incorrect `meta` attribute. + - xow package removed along with the `hardware.xow` module, due to the project being deprecated in favor of `xone`, which is available via the `hardware.xone` module. - dd-agent package removed along with the `services.dd-agent` module, due to the project being deprecated in favor of `datadog-agent`, which is available via the `services.datadog-agent` module. diff --git a/pkgs/stdenv/generic/check-meta.nix b/pkgs/stdenv/generic/check-meta.nix index f9bea506af6e..85a9c9c4d41e 100644 --- a/pkgs/stdenv/generic/check-meta.nix +++ b/pkgs/stdenv/generic/check-meta.nix @@ -13,10 +13,6 @@ let getName = attrs: attrs.name or ("${attrs.pname or "«name-missing»"}-${attrs.version or "«version-missing»"}"); - # See discussion at https://github.com/NixOS/nixpkgs/pull/25304#issuecomment-298385426 - # for why this defaults to false, but I (@copumpkin) want to default it to true soon. - shouldCheckMeta = config.checkMeta or false; - allowUnfree = config.allowUnfree || builtins.getEnv "NIXPKGS_ALLOW_UNFREE" == "1"; @@ -248,6 +244,16 @@ let isEnabled = lib.findFirst (x: x == reason) null showWarnings; in if isEnabled != null then builtins.trace msg true else true; + + # A shallow type check. We are using NixOS' + # option types here, which however have the major drawback + # of not providing full type checking (part of the type check is + # done by the module evaluation itself). Therefore, the checks + # will not recurse into attributes. + # We still provide the full type for documentation + # purposes and in the hope that they will be used eventually. + # See https://github.com/NixOS/nixpkgs/pull/191171 for an attempt + # to fix this, or mkOptionType in lib/types.nix for more information. metaTypes = with lib.types; rec { # These keys are documented description = str; @@ -297,17 +303,23 @@ let badPlatforms = platforms; }; + # WARNING: this does not check inner values of the attribute, like list elements or nested attributes. + # See metaTypes above and mkOptionType in lib/types.nix for more information checkMetaAttr = k: v: if metaTypes?${k} then - if metaTypes.${k}.check v then null else "key '${k}' has a value ${toString v} of an invalid type ${builtins.typeOf v}; expected ${metaTypes.${k}.description}" - else "key '${k}' is unrecognized; expected one of: \n\t [${lib.concatMapStringsSep ", " (x: "'${x}'") (lib.attrNames metaTypes)}]"; - checkMeta = meta: if shouldCheckMeta then lib.remove null (lib.mapAttrsToList checkMetaAttr meta) else []; + if metaTypes.${k}.check v then + null + else + "key 'meta.${k}' has a value of invalid type ${builtins.typeOf v}; expected ${metaTypes.${k}.description}" + else + "key 'meta.${k}' is unrecognized; expected one of: \n\t [${lib.concatMapStringsSep ", " (x: "'${x}'") (lib.attrNames metaTypes)}]"; + checkMeta = meta: if config.checkMeta then lib.remove null (lib.mapAttrsToList checkMetaAttr meta) else []; checkOutputsToInstall = attrs: let expectedOutputs = attrs.meta.outputsToInstall or []; actualOutputs = attrs.outputs or [ "out" ]; missingOutputs = builtins.filter (output: ! builtins.elem output actualOutputs) expectedOutputs; - in if shouldCheckMeta + in if config.checkMeta then builtins.length missingOutputs > 0 else false; @@ -326,7 +338,17 @@ let unsupported = hasUnsupportedPlatform attrs; insecure = isMarkedInsecure attrs; } - // (if hasDeniedUnfreeLicense attrs && !(hasAllowlistedLicense attrs) then + // ( + # Check meta attribute types first, to make sure it is always called even when there are other issues + # Note that this is not a full type check and functions below still need to by careful about their inputs! + let res = checkMeta (attrs.meta or {}); in if res != [] then + { valid = "no"; reason = "unknown-meta"; errormsg = "has an invalid meta attrset:${lib.concatMapStrings (x: "\n\t - " + x) res}"; } + # --- Put checks that cannot be ignored here --- + else if checkOutputsToInstall attrs then + { valid = "no"; reason = "broken-outputs"; errormsg = "has invalid meta.outputsToInstall"; } + + # --- Put checks that can be ignored here --- + else if hasDeniedUnfreeLicense attrs && !(hasAllowlistedLicense attrs) then { valid = "no"; reason = "unfree"; errormsg = "has an unfree license (‘${showLicense attrs.meta.license}’)"; } else if hasBlocklistedLicense attrs then { valid = "no"; reason = "blocklisted"; errormsg = "has a blocklisted license (‘${showLicense attrs.meta.license}’)"; } @@ -338,10 +360,7 @@ let { valid = "no"; reason = "unsupported"; errormsg = "is not supported on ‘${hostPlatform.system}’"; } else if !(hasAllowedInsecure attrs) then { valid = "no"; reason = "insecure"; errormsg = "is marked as insecure"; } - else if checkOutputsToInstall attrs then - { valid = "no"; reason = "broken-outputs"; errormsg = "has invalid meta.outputsToInstall"; } - else let res = checkMeta (attrs.meta or {}); in if res != [] then - { valid = "no"; reason = "unknown-meta"; errormsg = "has an invalid meta attrset:${lib.concatMapStrings (x: "\n\t - " + x) res}"; } + # --- warnings --- # Please also update the type in /pkgs/top-level/config.nix alongside this. else if hasNoMaintainers attrs then diff --git a/pkgs/top-level/config.nix b/pkgs/top-level/config.nix index 4fedad00f24b..6ddd647a2b1a 100644 --- a/pkgs/top-level/config.nix +++ b/pkgs/top-level/config.nix @@ -128,6 +128,13 @@ let ''; }; + checkMeta = mkOption { + type = types.bool; + default = true; + description = '' + Whether to check that the `meta` attribute of derivations are correct during evaluation time. + ''; + }; }; in {