From 2199f6dce9e73bb4d5e9f81c38591e7cbd1749a2 Mon Sep 17 00:00:00 2001 From: Eman Resu <78693624+quatquatt@users.noreply.github.com> Date: Tue, 12 May 2026 23:54:36 -0400 Subject: [PATCH 1/7] stdenv/problems: only run problem if it's not ignored (cherry picked from commit 5c89ab858d4470e7469a603cba739b8623e2acd2) --- pkgs/stdenv/generic/problems.nix | 36 ++++++++++++-- pkgs/test/problems/unit.nix | 85 ++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 4 deletions(-) diff --git a/pkgs/stdenv/generic/problems.nix b/pkgs/stdenv/generic/problems.nix index 926075bd8229..43c36edfb7c4 100644 --- a/pkgs/stdenv/generic/problems.nix +++ b/pkgs/stdenv/generic/problems.nix @@ -48,6 +48,8 @@ rec { groupBy subtractLists genAttrs + concatMap + unique ; handlers = rec { @@ -330,7 +332,10 @@ rec { }; }; - Returns both the structure itself for inspection and a function that can query it with very few allocations/lookups + Returns: + - the structure itself for inspection + - a function that can query the structure with very few allocations/lookups + - a list of problem kinds/names/packages that require handling This allows collapsing arbitrarily many problem handlers/matchers into a predictable structure that can be queried in a predictable and fast way */ @@ -352,6 +357,20 @@ rec { ) config.problems.handlers ); + # Lookup table for all the kinds/names/packages that actually need to be + # handled + definedConstraints = listToAttrs ( + map (ident: { + name = "${ident}s"; # plural + value = unique ( + concatMap ( + constraint: + optionals (constraint.${ident} != null && constraint.handler != "ignore") [ (constraint.${ident}) ] + ) constraints + ); + }) identOrder + ); + getHandler = list: (foldl' @@ -410,7 +429,7 @@ rec { switch = doLevel 0 constraints; in { - inherit switch; + inherit switch definedConstraints; handlerForProblem = if isString switch then pname: name: kind: @@ -438,10 +457,19 @@ rec { # This is here so that it gets cached for a (checkProblems config) thunk inherit (genHandlerSwitch config) handlerForProblem + definedConstraints ; + + # All the problem kinds that actually need to be checked + configuredProblems = definedConstraints.kinds ++ definedConstraints.names; + + # Filter out any problems that are always ignored in config.problems. # Makes sure that automatic problems can cache with just config applied - automaticProblemsConfigCache = map ( - problem: problem // { condition = problem.condition config; } + automaticProblemsConfigCache = concatMap ( + problem: + optionals (elem problem.kindName configuredProblems) [ + (problem // { condition = problem.condition config; }) + ] ) automaticProblems; in attrs: diff --git a/pkgs/test/problems/unit.nix b/pkgs/test/problems/unit.nix index 073ddaa12cfa..4a8076a84a7f 100644 --- a/pkgs/test/problems/unit.nix +++ b/pkgs/test/problems/unit.nix @@ -2,6 +2,11 @@ let p = import ../../stdenv/generic/problems.nix { inherit lib; }; + genConstraintsTest = problems: expected: { + expr = (p.genHandlerSwitch { inherit problems; }).definedConstraints; + inherit expected; + }; + genHandlerTest = let slowReference = @@ -150,4 +155,84 @@ lib.runTests { ]; handlers = { }; }; + + testDefinedConstraintsEmpty = + genConstraintsTest + { + matchers = [ ]; + handlers = { }; + } + { + kinds = [ ]; + names = [ ]; + packages = [ ]; + }; + + testDefinedConstraintsMatchers = + genConstraintsTest + { + handlers = { }; + matchers = [ + { + package = null; + name = null; + kind = "k1"; + handler = "warn"; + } + { + package = null; + name = null; + kind = "k2"; + handler = "error"; + } + { + package = null; + name = null; + kind = "k3"; + handler = "ignore"; + } + { + package = "p1"; + name = "n1"; + kind = null; + handler = "error"; + } + { + package = "p2"; + name = "n1"; + kind = null; + handler = "warn"; + } + ]; + } + { + kinds = [ + "k1" + "k2" + ]; + names = [ "n1" ]; + packages = [ + "p1" + "p2" + ]; + }; + + testDefinedConstraintsHandlers = + genConstraintsTest + { + matchers = [ ]; + handlers.p1.n1 = "warn"; + handlers.p1.n2 = "error"; + handlers.p2.n3 = "ignore"; + } + { + kinds = [ ]; + names = [ + "n1" + "n2" + ]; + packages = [ + "p1" + ]; + }; } From 2fef77700c3d197bd9d5f3cdc387b1470e972f3e Mon Sep 17 00:00:00 2001 From: Eman Resu <78693624+quatquatt@users.noreply.github.com> Date: Mon, 25 May 2026 21:22:46 -0400 Subject: [PATCH 2/7] stdenv/problems: avoid creating manual problems attrset (cherry picked from commit b88fba2fba59c0addc1ca7c26445f4d212299bf6) --- pkgs/stdenv/generic/problems.nix | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkgs/stdenv/generic/problems.nix b/pkgs/stdenv/generic/problems.nix index 43c36edfb7c4..04d190471ee1 100644 --- a/pkgs/stdenv/generic/problems.nix +++ b/pkgs/stdenv/generic/problems.nix @@ -475,7 +475,6 @@ rec { attrs: let pname = getName attrs; - manualProblems = attrs.meta.problems or { }; in if # Fast path for when there's no problem that needs to be handled @@ -486,10 +485,10 @@ rec { ) automaticProblemsConfigCache && ( # No manual problems - manualProblems == { } + !attrs ? meta.problems # Or all manual problems are ignored - || all (name: handlerForProblem pname name (manualProblems.${name}.kind or name) == "ignore") ( - attrNames manualProblems + || all (name: handlerForProblem pname name (attrs.meta.problems.${name}.kind or name) == "ignore") ( + attrNames attrs.meta.problems ) ) then From bd8250ab9611e14e669d32f0dda2215957f000fd Mon Sep 17 00:00:00 2001 From: Eman Resu <78693624+quatquatt@users.noreply.github.com> Date: Mon, 25 May 2026 21:51:56 -0400 Subject: [PATCH 3/7] stdenv/problems: use early knowledge of allowBroken and allowBrokenPredicate (cherry picked from commit 5616e3d9255badfaf9ad1940659b9ac6342d01e3) --- pkgs/stdenv/generic/problems.nix | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pkgs/stdenv/generic/problems.nix b/pkgs/stdenv/generic/problems.nix index 04d190471ee1..6dac3163abe1 100644 --- a/pkgs/stdenv/generic/problems.nix +++ b/pkgs/stdenv/generic/problems.nix @@ -121,14 +121,16 @@ rec { allowBroken = config.allowBroken || builtins.getEnv "NIXPKGS_ALLOW_BROKEN" == "1"; allowBrokenPredicate = - if config ? allowBrokenPredicate then - lib.warnIf (lib.oldestSupportedReleaseIsAtLeast 2605) - "config.allowBrokenPredicate is deprecated, use config.problems.handlers.myPackage.broken = \"warn\" for individual packages instead." - config.allowBrokenPredicate - else - x: false; + lib.warnIf (lib.oldestSupportedReleaseIsAtLeast 2605) + "config.allowBrokenPredicate is deprecated, use config.problems.handlers.myPackage.broken = \"warn\" for individual packages instead." + config.allowBrokenPredicate; in - attrs: attrs.meta.broken or false && !allowBroken && !allowBrokenPredicate attrs; + if allowBroken then + attrs: false + else if config ? allowBrokenPredicate then + attrs: attrs.meta.broken or false && !allowBrokenPredicate attrs + else + attrs: attrs.meta.broken or false; value.message = "This package is broken."; } ]; From 3358486adb1c45e4a9318b978387bff61ccd6bf9 Mon Sep 17 00:00:00 2001 From: Eman Resu <78693624+quatquatt@users.noreply.github.com> Date: Mon, 25 May 2026 23:18:10 -0400 Subject: [PATCH 4/7] stdenv/problems: avoid a lookup for meta.broken The stats may be lying to me when they say that this saves 1.86% of attrset lookups, and `?` may just not be accurately tracked. But at worst, performance will stay the same, since the check will fail for all non-broken packages. And who knows, maybe it does help, by nature of not checking the value! (cherry picked from commit 753e43c931d8e6caadf6eca4cbe7bdfe8d7a42de) --- pkgs/stdenv/generic/problems.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/stdenv/generic/problems.nix b/pkgs/stdenv/generic/problems.nix index 6dac3163abe1..348786ab5242 100644 --- a/pkgs/stdenv/generic/problems.nix +++ b/pkgs/stdenv/generic/problems.nix @@ -128,9 +128,9 @@ rec { if allowBroken then attrs: false else if config ? allowBrokenPredicate then - attrs: attrs.meta.broken or false && !allowBrokenPredicate attrs + attrs: attrs ? meta.broken && attrs.meta.broken && !allowBrokenPredicate attrs else - attrs: attrs.meta.broken or false; + attrs: attrs ? meta.broken && attrs.meta.broken; value.message = "This package is broken."; } ]; From dc025af01930f739dbef0a65496e0b7ec4c384b9 Mon Sep 17 00:00:00 2001 From: Eman Resu <78693624+quatquatt@users.noreply.github.com> Date: Mon, 25 May 2026 23:06:04 -0400 Subject: [PATCH 5/7] stdenv/problems: reverse parameter order for handlerForProblem kind and name are more likely to stay the same. Doing this allows us to cache the kind and name when calling the handler for a problem. (cherry picked from commit 41bafcbc3488533fcb64a84e6df885f03c53957c) --- pkgs/stdenv/generic/problems.nix | 22 +++++++++++----------- pkgs/test/problems/unit.nix | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pkgs/stdenv/generic/problems.nix b/pkgs/stdenv/generic/problems.nix index 348786ab5242..0d0260ef6afc 100644 --- a/pkgs/stdenv/generic/problems.nix +++ b/pkgs/stdenv/generic/problems.nix @@ -434,10 +434,10 @@ rec { inherit switch definedConstraints; handlerForProblem = if isString switch then - pname: name: kind: + kind: name: pname: switch else - pname: name: kind: + kind: name: pname: let switch' = switch.kindSpecific.${kind} or switch.kindFallback; in @@ -466,12 +466,14 @@ rec { configuredProblems = definedConstraints.kinds ++ definedConstraints.names; # Filter out any problems that are always ignored in config.problems. - # Makes sure that automatic problems can cache with just config applied + # Makes sure to cache the condition by appliny config, and the handler + # by applying the problem's kind and name automaticProblemsConfigCache = concatMap ( problem: - optionals (elem problem.kindName configuredProblems) [ - (problem // { condition = problem.condition config; }) - ] + optional (elem problem.kindName configuredProblems) { + condition = problem.condition config; + handler = handlerForProblem problem.kindName problem.kindName; + } ) automaticProblems; in attrs: @@ -480,16 +482,14 @@ rec { in if # Fast path for when there's no problem that needs to be handled - # No automatic problems that needs handling all ( - problem: - problem.condition attrs -> handlerForProblem pname problem.kindName problem.kindName == "ignore" + problem: problem.condition attrs -> problem.handler pname == "ignore" ) automaticProblemsConfigCache && ( # No manual problems !attrs ? meta.problems # Or all manual problems are ignored - || all (name: handlerForProblem pname name (attrs.meta.problems.${name}.kind or name) == "ignore") ( + || all (name: handlerForProblem (attrs.meta.problems.${name}.kind or name) name pname == "ignore") ( attrNames attrs.meta.problems ) ) @@ -504,7 +504,7 @@ rec { inherit name; # Kind falls back to the name kind = problem.kind or name; - handler = handlerForProblem pname name kind; + handler = handlerForProblem kind name pname; inherit problem; }) problems ); diff --git a/pkgs/test/problems/unit.nix b/pkgs/test/problems/unit.nix index 4a8076a84a7f..aef201508e63 100644 --- a/pkgs/test/problems/unit.nix +++ b/pkgs/test/problems/unit.nix @@ -10,7 +10,7 @@ let genHandlerTest = let slowReference = - config: package: name: kind: + config: kind: name: package: # Try to find an explicit handler (config.problems.handlers.${package} or { }).${name} # Fall back, iterating through the matchers @@ -36,7 +36,7 @@ let map ( name: - map (kind: f package name kind) [ + map (kind: f kind name package) [ "k1" "k2" "k3" From 5b0477b28d30e609de4371de37e9067f78eda54d Mon Sep 17 00:00:00 2001 From: Eman Resu <78693624+quatquatt@users.noreply.github.com> Date: Mon, 25 May 2026 23:10:29 -0400 Subject: [PATCH 6/7] stdenv/problems: make handlerForProblem more performant with partial application (cherry picked from commit 0b304744b80c6f5146c8a074f41316d94adc92ec) --- pkgs/stdenv/generic/problems.nix | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/pkgs/stdenv/generic/problems.nix b/pkgs/stdenv/generic/problems.nix index 0d0260ef6afc..ea4703954a30 100644 --- a/pkgs/stdenv/generic/problems.nix +++ b/pkgs/stdenv/generic/problems.nix @@ -437,20 +437,21 @@ rec { kind: name: pname: switch else - kind: name: pname: + kind: let - switch' = switch.kindSpecific.${kind} or switch.kindFallback; + kindSwitch = switch.kindSpecific.${kind} or switch.kindFallback; in - if isString switch' then - switch' + if isString kindSwitch then + name: pname: kindSwitch else + name: let - switch'' = switch'.nameSpecific.${name} or switch'.nameFallback; + nameSwitch = kindSwitch.nameSpecific.${name} or kindSwitch.nameFallback; in - if isString switch'' then - switch'' + if isString nameSwitch then + pname: nameSwitch else - switch''.packageSpecific.${pname} or switch''.packageFallback; + pname: nameSwitch.packageSpecific.${pname} or nameSwitch.packageFallback; }; genCheckProblems = From c63637a6607e56568f01ab5e9fda64f3b9816313 Mon Sep 17 00:00:00 2001 From: Eman Resu <78693624+quatquatt@users.noreply.github.com> Date: Mon, 25 May 2026 23:14:39 -0400 Subject: [PATCH 7/7] stdenv/problems: don't create pname variable A package failing an automatic problem (of which there's currently only one) is rare. A package having meta.problems specified is even rarer. Inlining the getName call to its two usages saves a thunk in the vast, vast majority of cases, and I consider it to be worth it. (cherry picked from commit 85e32af8ef67bde9c5a8c7dbf06bb9dcec392c9f) --- pkgs/stdenv/generic/problems.nix | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pkgs/stdenv/generic/problems.nix b/pkgs/stdenv/generic/problems.nix index ea4703954a30..0f03c393e384 100644 --- a/pkgs/stdenv/generic/problems.nix +++ b/pkgs/stdenv/generic/problems.nix @@ -478,27 +478,25 @@ rec { ) automaticProblems; in attrs: - let - pname = getName attrs; - in if # Fast path for when there's no problem that needs to be handled all ( - problem: problem.condition attrs -> problem.handler pname == "ignore" + problem: problem.condition attrs -> problem.handler (getName attrs) == "ignore" ) automaticProblemsConfigCache && ( # No manual problems !attrs ? meta.problems # Or all manual problems are ignored - || all (name: handlerForProblem (attrs.meta.problems.${name}.kind or name) name pname == "ignore") ( - attrNames attrs.meta.problems - ) + || all ( + name: handlerForProblem (attrs.meta.problems.${name}.kind or name) name (getName attrs) == "ignore" + ) (attrNames attrs.meta.problems) ) then null else # Slow path, only here we actually figure out which problems we need to handle let + pname = getName attrs; problems = attrs.meta.problems or { } // genAutomaticProblems config attrs; problemsToHandle = filter (v: v.handler != "ignore") ( mapAttrsToList (name: problem: rec {