From 5e420c24556053e2a5ad3ca69993afb42fffbc54 Mon Sep 17 00:00:00 2001 From: ckie Date: Tue, 3 May 2022 22:12:34 +0300 Subject: [PATCH 1/3] stdenv/check-meta: turn validity.valid into a str This will allow for adding more validity types in the future, such as a warning type. (which is in the next commit in this series) This is NOT a breaking change because validity.valid is never exposed outside of `stdenv.mkDerivation`. --- pkgs/stdenv/generic/check-meta.nix | 27 ++++++++++++++----------- pkgs/stdenv/generic/make-derivation.nix | 2 +- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/pkgs/stdenv/generic/check-meta.nix b/pkgs/stdenv/generic/check-meta.nix index e0ead55d1a7..5d4f3579b91 100644 --- a/pkgs/stdenv/generic/check-meta.nix +++ b/pkgs/stdenv/generic/check-meta.nix @@ -277,28 +277,31 @@ let insecure = isMarkedInsecure attrs; } // (if hasDeniedUnfreeLicense attrs && !(hasAllowlistedLicense attrs) then - { valid = false; reason = "unfree"; errormsg = "has an unfree license (‘${showLicense attrs.meta.license}’)"; } + { valid = "no"; reason = "unfree"; errormsg = "has an unfree license (‘${showLicense attrs.meta.license}’)"; } else if hasBlocklistedLicense attrs then - { valid = false; reason = "blocklisted"; errormsg = "has a blocklisted license (‘${showLicense attrs.meta.license}’)"; } + { valid = "no"; reason = "blocklisted"; errormsg = "has a blocklisted license (‘${showLicense attrs.meta.license}’)"; } else if !allowBroken && attrs.meta.broken or false then - { valid = false; reason = "broken"; errormsg = "is marked as broken"; } + { valid = "no"; reason = "broken"; errormsg = "is marked as broken"; } else if !allowUnsupportedSystem && hasUnsupportedPlatform attrs then - { valid = false; reason = "unsupported"; errormsg = "is not supported on ‘${hostPlatform.system}’"; } + { valid = "no"; reason = "unsupported"; errormsg = "is not supported on ‘${hostPlatform.system}’"; } else if !(hasAllowedInsecure attrs) then - { valid = false; reason = "insecure"; errormsg = "is marked as insecure"; } + { valid = "no"; reason = "insecure"; errormsg = "is marked as insecure"; } else if checkOutputsToInstall attrs then - { valid = false; reason = "broken-outputs"; errormsg = "has invalid meta.outputsToInstall"; } + { valid = "no"; reason = "broken-outputs"; errormsg = "has invalid meta.outputsToInstall"; } else let res = checkMeta (attrs.meta or {}); in if res != [] then - { valid = false; reason = "unknown-meta"; errormsg = "has an invalid meta attrset:${lib.concatMapStrings (x: "\n\t - " + x) res}"; } - else { valid = true; }); + { valid = "no"; reason = "unknown-meta"; errormsg = "has an invalid meta attrset:${lib.concatMapStrings (x: "\n\t - " + x) res}"; } + else { valid = "yes"; }); assertValidity = { meta, attrs }: let validity = checkValidity attrs; in validity // { - # Throw an error if trying to evaluate an non-valid derivation - handled = if !validity.valid - then handleEvalIssue { inherit meta attrs; } { inherit (validity) reason errormsg; } - else true; + # Throw an error if trying to evaluate a non-valid derivation + # or, alternatively, just output a warning message. + handled = + { + no = handleEvalIssue { inherit meta attrs; } { inherit (validity) reason errormsg; }; + yes = true; + }.${validity.valid}; }; in assertValidity diff --git a/pkgs/stdenv/generic/make-derivation.nix b/pkgs/stdenv/generic/make-derivation.nix index d1b93874a25..53e391ab63f 100644 --- a/pkgs/stdenv/generic/make-derivation.nix +++ b/pkgs/stdenv/generic/make-derivation.nix @@ -370,7 +370,7 @@ else let } // { # Expose the result of the checks for everyone to see. inherit (validity) unfree broken unsupported insecure; - available = validity.valid + available = validity.valid != "no" && (if config.checkMetaRecursively or false then lib.all (d: d.meta.available or true) references else true); From 3a34b6c820b1cd62ed1b1747b6cad6275e81321d Mon Sep 17 00:00:00 2001 From: ckie Date: Tue, 3 May 2022 22:16:04 +0300 Subject: [PATCH 2/3] stdenv/check-meta: add an eval warning option This will be used in the next commit in this patch series. --- pkgs/stdenv/generic/check-meta.nix | 13 +++++++++++++ pkgs/top-level/config.nix | 15 +++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/pkgs/stdenv/generic/check-meta.nix b/pkgs/stdenv/generic/check-meta.nix index 5d4f3579b91..4a9af7a6f67 100644 --- a/pkgs/stdenv/generic/check-meta.nix +++ b/pkgs/stdenv/generic/check-meta.nix @@ -7,6 +7,10 @@ let # If we're in hydra, we can dispense with the more verbose error # messages and make problems easier to spot. inHydra = config.inHydra or false; + # Allow the user to opt-into additional warnings, e.g. + # import { config = { showDerivationWarnings = [ "maintainerless" ]; }; } + showWarnings = config.showDerivationWarnings; + 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 @@ -199,6 +203,14 @@ let else throw; in handler msg; + handleEvalWarning = { meta, attrs }: { reason , errormsg ? "" }: + let + remediationMsg = (builtins.getAttr reason remediation) attrs; + msg = if inHydra then "Warning while evaluating ${getName attrs}: «${reason}»: ${errormsg}" + else "Package ${getName attrs} in ${pos_str meta} ${errormsg}, continuing anyway." + + (if remediationMsg != "" then "\n${remediationMsg}" else ""); + isEnabled = lib.findFirst (x: x == reason) null showWarnings; + in if isEnabled != null then builtins.trace msg true else true; metaTypes = with lib.types; rec { # These keys are documented @@ -300,6 +312,7 @@ let handled = { no = handleEvalIssue { inherit meta attrs; } { inherit (validity) reason errormsg; }; + warn = handleEvalWarning { inherit meta attrs; } { inherit (validity) reason errormsg; }; yes = true; }.${validity.valid}; }; diff --git a/pkgs/top-level/config.nix b/pkgs/top-level/config.nix index 7665815d412..d553b624039 100644 --- a/pkgs/top-level/config.nix +++ b/pkgs/top-level/config.nix @@ -94,6 +94,21 @@ let ''; }; + showDerivationWarnings = mkOption { + type = types.listOf (types.enum [ "maintainerless" ]); + default = []; + description = '' + Which warnings to display for potentially dangerous + or deprecated values passed into `stdenv.mkDerivation`. + + A list of warnings can be found in + /pkgs/stdenv/generic/check-meta.nix. + + This is not a stable interface; warnings may be added, changed + or removed without prior notice. + ''; + }; + }; in { From 4def222ea40aef58ee83fb00bd412b78ee807205 Mon Sep 17 00:00:00 2001 From: ckie Date: Tue, 3 May 2022 22:17:44 +0300 Subject: [PATCH 3/3] stdenv/check-meta: add a "maintainerless" warning This warning logs when a package has no maintainers. It will stay silent if `meta.maintainers` is not set at all, only complaining when it is an empty list. In the future a separate warning could be added to allow for that stricter behavior. Or this warning could be changed. --- pkgs/stdenv/generic/check-meta.nix | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkgs/stdenv/generic/check-meta.nix b/pkgs/stdenv/generic/check-meta.nix index 4a9af7a6f67..1da098dabbe 100644 --- a/pkgs/stdenv/generic/check-meta.nix +++ b/pkgs/stdenv/generic/check-meta.nix @@ -50,6 +50,9 @@ let hasLicense attrs && isUnfree (lib.lists.toList attrs.meta.license); + hasNoMaintainers = attrs: + attrs ? meta.maintainers && (lib.length attrs.meta.maintainers) == 0; + isMarkedBroken = attrs: attrs.meta.broken or false; hasUnsupportedPlatform = attrs: @@ -95,6 +98,7 @@ let insecure = remediate_insecure; broken-outputs = remediateOutputsToInstall; unknown-meta = x: ""; + maintainerless = x: ""; }; remediation_env_var = allow_attr: { Unfree = "NIXPKGS_ALLOW_UNFREE"; @@ -302,6 +306,11 @@ let { 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 + { valid = "warn"; reason = "maintainerless"; errormsg = "has no maintainers"; } + # ----- else { valid = "yes"; }); assertValidity = { meta, attrs }: let