Skip to content

Commit

Permalink
RFC 127: Add implementation tests
Browse files Browse the repository at this point in the history 
* Also fix merge conflicts
* Make sure that performance PR is conserved
* Use RunCommand instead of alias RunCommandNoCC
* Add enum types to meta-types.nix and use them for problemTypes
* Nixfmt (excepted check-meta.nix)
* "import" elem
* Remove dead code
  • Loading branch information
@AkechiShiro
AkechiShiro committed Sep 4, 2024
1 parent 2c52dcf commit 69b5584
Show file tree
Hide file tree
Showing 36 changed files with 616 additions and 108 deletions.
86 changes: 34 additions & 52 deletions pkgs/stdenv/generic/check-meta.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,19 +353,9 @@ let
in
builtins.trace msg 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 = lib.mergeDefinitions [ ] type [
{ file = lib.unknownModule; inherit value; }
];
eval = builtins.tryEval (builtins.deepSeq merged.mergedValue null);
in eval.success;


metaTypes = let
types = import ./meta-types.nix { inherit lib; };
inherit (types) str union int attrs attrsOf any listOf bool;
inherit (types) str union int attrs attrsOf any listOf bool enum;
platforms = listOf (union [ str (attrsOf any) ]); # see lib.meta.platformMatch
in {
# These keys are documented
Expand Down Expand Up @@ -404,8 +394,11 @@ let
insecure = bool;
# This is checked in more detail further down
problems = attrsOf (attrsOf any);
# 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
problemTypes = {
kind = enum problemKindsManual;
message = str;
urls = listOf str;
};
tests = {
name = "test";
verify = x: x == {} || ( # Accept {} for tests that are unsupported
Expand Down Expand Up @@ -437,15 +430,8 @@ let
badPlatforms = platforms;
};

# Type of a meta.problems.* value
problemTypes = with lib.types; {
# Only allow some problem kinds to be used here in `meta`
kind = enum problemKindsManual;
message = str;
urls = listOf str;
};

checkMetaAttr = let
# Check that a value matches a specific type. Returns an error message, or null if the check passed
checkMetaAttr = let
# Map attrs directly to the verify function for performance
metaTypes' = mapAttrs (_: t: t.verify) metaTypes;
in k: v:
Expand All @@ -458,15 +444,7 @@ let
}" ]
else
[ "key 'meta.${k}' is unrecognized; expected one of: \n [${concatMapStringsSep ", " (x: "'${x}'") (attrNames metaTypes)}]" ];

checkMeta =
meta:
# Basic attribute checks
lib.optionals config.checkMeta (
lib.remove null (
lib.mapAttrsToList (checkMetaAttr "meta" metaTypes) meta
)
)
checkMeta = meta: optionals config.checkMeta (concatMap (attr: checkMetaAttr attr meta.${attr}) (attrNames meta)

# Extended checks for problems, as they do not fit the module system
++ lib.optionals (meta ? problems) (
Expand All @@ -476,7 +454,7 @@ let
(lib.mapAttrs (name: problem: { kind = name; } // problem))
(lib.mapAttrsToList (
name: problem:
lib.mapAttrsToList (checkMetaAttr "meta.problems.${name}" problemTypes) problem
lib.mapAttrsToList (checkMetaAttr "meta.problems.${name}" meta.problemTypes) problem
))
(lib.concatLists)
(lib.remove null)
Expand Down Expand Up @@ -511,11 +489,11 @@ let
(lib.mapAttrsToList (
kind: names:
"keys [ ${
concatMapStringsSep " " (name: "'meta.problems.${name}'") names
lib.concatMapStringsSep " " (name: "'meta.problems.${name}'") names
} ] all have the same problem kind, which is not allowed for kind '${kind}'"
))
]
);
));

checkOutputsToInstall = attrs: let
expectedOutputs = attrs.meta.outputsToInstall or [];
Expand All @@ -533,21 +511,24 @@ let
# reason is one of "unfree", "blocklisted", "broken", "insecure", ...
# !!! reason strings are hardcoded into OfBorg, make sure to keep them in sync
# Along with a boolean flag for each reason
checkValidity =
let
validYes = {
valid = "yes";
handled = true;
};
in
attrs:
checkValidity = attrs:
# 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:${concatMapStrings (x: "\n - " + x) res}\n"; }
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 - " + x) res}\n";
unfree = false; nonSource = false; broken = false; unsupported = false; insecure = false;
}
else {
unfree = hasUnfreeLicense attrs;
nonSource = hasNonSourceProvenance attrs;
broken = isMarkedBroken attrs;
unsupported = hasUnsupportedPlatform attrs;
insecure = isMarkedInsecure attrs;
} // (
# 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 malformed metadata:${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"; }
Expand All @@ -559,19 +540,21 @@ let
{ valid = "no"; reason = "blocklisted"; errormsg = "has a blocklisted license (‘${showLicense attrs.meta.license}’)"; }
else if hasDeniedNonSourceProvenance attrs then
{ valid = "no"; reason = "non-source"; errormsg = "contains elements not built from source (‘${showSourceType attrs.meta.sourceProvenance}’)"; }

# --- Put checks that can be ignored here ---
else if !allowBroken && attrs.meta.broken or false then
{ valid = "no"; reason = "broken"; errormsg = "is marked as broken"; }
else if !allowUnsupportedSystem && hasUnsupportedPlatform attrs then
let toPretty' = toPretty {
let toPretty = lib.generators.toPretty {
allowPrettyValues = true;
indent = " ";
};
in { valid = "no"; reason = "unsupported";
errormsg = ''
is not available on the requested hostPlatform:
hostPlatform.config = "${hostPlatform.config}"
package.meta.platforms = ${toPretty' (attrs.meta.platforms or [])}
package.meta.badPlatforms = ${toPretty' (attrs.meta.badPlatforms or [])}
package.meta.platforms = ${toPretty (attrs.meta.platforms or [])}
package.meta.badPlatforms = ${toPretty (attrs.meta.badPlatforms or [])}
'';
}
else if !(hasAllowedInsecure attrs) then
Expand Down Expand Up @@ -602,8 +585,7 @@ let
;
}
# -----
else validYes;

else { valid = "yes"; });

# The meta attribute is passed in the resulting attribute set,
# but it's not part of the actual derivation, i.e., it's not
Expand Down
84 changes: 57 additions & 27 deletions pkgs/stdenv/generic/meta-types.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,20 @@
# 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;
inherit (builtins)
isString
isInt
isAttrs
isList
all
any
attrValues
isFunction
isBool
concatStringsSep
isFloat
elem
;
isTypeDef = t: isAttrs t && t ? name && isString t.name && t ? verify && isFunction t.verify;

in
Expand All @@ -14,7 +27,7 @@ lib.fix (self: {
name = "string";
verify = isString;
};
str = self.string; # Type alias
str = self.string; # Type alias

any = {
name = "any";
Expand Down Expand Up @@ -46,31 +59,48 @@ lib.fix (self: {
verify = isList;
};

attrsOf = t: assert isTypeDef t; let
inherit (t) verify;
in {
name = "attrsOf<${t.name}>";
verify =
# attrsOf<any> can be optimised to just isAttrs
if t == self.any then isAttrs
else attrs: isAttrs attrs && all verify (attrValues attrs);
};
attrsOf =
t:
assert isTypeDef t;
let
inherit (t) verify;
in
{
name = "attrsOf<${t.name}>";
verify =
# attrsOf<any> 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<any> can be optimised to just isList
if t == self.any then isList
else v: isList v && all verify v;
};
listOf =
t:
assert isTypeDef t;
let
inherit (t) verify;
in
{
name = "listOf<${t.name}>";
verify =
# listOf<any> 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;
};
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;
};
enum =
values:
assert isList values && all isString values;
{
name = "enum<${concatStringsSep "," values}>";
verify = v: isString v && elem v values;
};
})
53 changes: 31 additions & 22 deletions pkgs/stdenv/generic/problems.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@

rec {
# Problem handler levels
problemHandlers = [ "error" "warn" "ignore" ];
problemHandlers = [
"error"
"warn"
"ignore"
];

# Take the maximum (highest severity) problem handler
maxProblemHandler =
Expand All @@ -14,8 +18,7 @@ rec {
else if a == "warn" || b == "warn" then
"warn"
else
"ignore"
;
"ignore";

# Currently known list of known problem kinds. Keep up to date with pkgs/top-level/config.nix
problemKinds = [
Expand All @@ -40,7 +43,10 @@ rec {
];
# Same thing but a set with null values (comes in handy at times)
problemKindsUnique' = lib.pipe problemKindsUnique [
(map (name: { inherit name; value = null; }))
(map (name: {
inherit name;
value = null;
}))
lib.listToAttrs
];

Expand All @@ -61,42 +67,45 @@ rec {

# Take the problems plus add automatically generated ones
problems =
(pkg.meta.problems or {})
(pkg.meta.problems or { })
// lib.optionalAttrs (hasNoMaintainers pkg) {
maintainerless = {
message = "This package has no declared maintainer, i.e. an empty `meta.maintainers` attribute";
};
};

# Inject default values, since metaTypes can't do it for us currently
addDefaults = name: problem: { inherit name; kind = name; } // problem;
addDefaults =
name: problem:
{
inherit name;
kind = name;
}
// problem;

# Determine the handler level from config for this problem
addHandler = problem:
addHandler =
problem:
let
handler =
# Try to find an explicit handler
(handlers.${pkgName} or {}).${problem.name}
# Fall back, iterating through the matchers
or (
lib.pipe matchers [
(handlers.${pkgName} or { }).${problem.name}
# Fall back, iterating through the matchers
or (lib.pipe matchers [
# Find matches
(lib.filter
(matcher:
(if matcher.name != null then problem.name == matcher.name else true)
&& (if matcher.kind != null then problem.kind == matcher.kind else true)
&& (if matcher.package != null then pkgName == matcher.package else true)
)
)
(lib.filter (
matcher:
(if matcher.name != null then problem.name == matcher.name else true)
&& (if matcher.kind != null then problem.kind == matcher.kind else true)
&& (if matcher.package != null then pkgName == matcher.package else true)
))
# Extract handler level
(builtins.map (lib.getAttr "handler"))
# Take the strongest matched handler level
(lib.foldl' maxProblemHandler "ignore")
]
);
]);
in
problem // { inherit handler; };
in
builtins.map addHandler
(lib.mapAttrsToList addDefaults problems);
builtins.map addHandler (lib.mapAttrsToList addDefaults problems);
}
23 changes: 23 additions & 0 deletions pkgs/test/problems/cases/deprecated-and-removal-and-maintainerless-error/default.nix
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{ nixpkgs }:
let
pkgs = import nixpkgs {
system = "x86_64-linux";
overlays = [ ];
config = {
problems.handlers = {
"a"."deprecated" = "error";
"a"."removal" = "error";
"a"."maintainerless" = "error";
};
};
};
in
pkgs.stdenvNoCC.mkDerivation {
pname = "a";
version = "0";
meta.problems = {
deprecated.message = "Package is deprecated and replaced by b";
removal.message = "Package will be removed.";
};
meta.maintainers = [ ];
}
Loading

0 comments on commit 69b5584

Please sign in to comment.