Make formatter not strict in all systems and try to...

... avoid broken attributes in cases where we can predict that
we don't have any.

This is far from ideal, but it's the best we can do.
If it becomes a shit show, flake authors can opt out.
This commit is contained in:
Robert Hensing 2025-05-27 00:33:49 +02:00
parent 14a4a8d4fe
commit 157c95de4d
2 changed files with 128 additions and 8 deletions

View file

@ -464,4 +464,68 @@ in
expected = "dogfood";
};
};
formatter = {
"test: conditional null throws helpful error" = {
expr =
let
result = mkFlake { inputs.self = { }; } {
systems = [ "x86_64-linux" "aarch64-darwin" ];
perSystem = { system, ... }: {
formatter =
if system == "x86_64-linux" then
derivation { name = "fmt"; builder = "x"; system = system; }
else null;
};
};
in
result.formatter.aarch64-darwin;
expectedError.type = "ThrownError";
expectedError.msg = "could not determine statically(.|\n)*disabledModules(.|\n)*inputs\\.flake-parts\\.modules\\.formatter";
};
"test: empty when never defined" = {
expr =
let
result = mkFlake { inputs.self = { }; } {
systems = [ "x86_64-linux" "aarch64-darwin" ];
perSystem = { ... }: { };
};
in
result.formatter == { };
expected = true;
};
"test: present for all systems" = {
expr =
let
result = mkFlake { inputs.self = { }; } {
systems = [ "x86_64-linux" "aarch64-darwin" ];
perSystem = { system, ... }: {
formatter = derivation { name = "fmt"; builder = "x"; inherit system; };
};
};
in
result ? formatter && result.formatter ? x86_64-linux && result.formatter ? aarch64-darwin;
expected = true;
};
"test: lazy per-system" = {
expr =
let
result = mkFlake { inputs.self = { }; } {
systems = [ "x86_64-linux" "aarch64-darwin" ];
perSystem = { system, ... }: {
formatter =
if system == "x86_64-linux" then
derivation { name = "fmt"; builder = "x"; system = system; }
else
throw "should not evaluate aarch64-darwin formatter";
};
};
in
result.formatter.x86_64-linux.name;
expected = "fmt";
};
};
}

View file

@ -1,7 +1,6 @@
{ config, lib, flake-parts-lib, ... }:
let
inherit (lib)
filterAttrs
mapAttrs
mkOption
optionalAttrs
@ -10,11 +9,41 @@ let
inherit (flake-parts-lib)
mkPerSystemOption
;
# Do not copy this pattern! (probe, haveFormatterProbably, optionalAttrs)
# It kind of works somewhat for `formatter`, but it is bad.
# Nothing critical must rely on this!
# - `tryEval` makes debugging harder and more annoying
# - There's a performance cost to this otherwise useless evaluation
# - We only use it *in `formatter`* because the effects are limited to that
# particular attribute. This makes the risk somewhat manageable.
probe =
config.perSystem
# Elaborate error message that users should never see.
# The reason to even do this, is that we can sidestep
# https://github.com/hercules-ci/flake-parts/issues/288 in many cases,
# without flake author intervention to keep `nix flake check` happy.
(throw ''
For the purpose of finding out whether an option may be unset for all systems, flake-parts probes the perSystem module without a valid `system` argument, and tries to catch this exception if it finds that `system` is required for this determination. If you see this message, it means that for some reason, flake-parts was unable to catch this exception.
This may be a bug in Nix or in flake-parts, but ultimately this is due to the quirky requirement of flakes that the "system" attribute does not come first.
Flake-parts tries its best to correct that UX, but ultimately, this needs to be solved in Nix.
'');
haveFormatterProbably =
let
ev =
builtins.tryEval
probe.formatter;
in
# If it fails, we can't assume that we don't have a formatter, because it may well evaluate for a real system.
!ev.success
|| ev.value != null;
in
{
options = {
flake.formatter = mkOption {
type = types.lazyAttrsOf types.package;
type = types.lazyAttrsOf (types.nullOr types.package);
default = { };
description = ''
An attribute set of per system a package used by [`nix fmt`](https://nixos.org/manual/nix/stable/command-ref/new-cli/nix3-fmt.html).
@ -36,12 +65,39 @@ in
};
config = {
flake.formatter =
mapAttrs
(k: v: v.formatter)
(filterAttrs
(k: v: v.formatter != null)
config.allSystems
);
# Work around `nix flake check` not allowing `null` values in output attributes.
optionalAttrs
haveFormatterProbably
(mapAttrs
(k: v:
if v.formatter != null then v.formatter
else
throw ''
flake-parts could not determine statically that no formatter is defined for *all* systems.
What happened?
1. For performance reasons, flake-parts must not query `perSystem` for every system, so it uses a heuristic to determine whether a formatter is defined for all systems.
2. Unfortunately, this heuristic is not perfect, and it wasn't able to determine for your flake that `perSystem.formatter` is always `null` (if it even is always `null`).
As a consequence of (1), flake-parts had to provide the output attribute `formatter.${k}`, but as a consequence of (2), you're seeing this error.
What to do?
This whole situation should be temporary. `nix flake check`/`show` can be changed to allow `null` values, which gives flake-parts and other frameworks a way to avoid this situation.
If you don't want a `formatter` output attribute, you may remove it in your `mkFlake` call.
(`disabledModules` must not be used in modules that are intended for reuse)
{ inputs, ... }:
{
disabledModules = [
inputs.flake-parts.modules.formatter
];
# perSystem = ...; etc
}
'')
config.allSystems);
perInput = system: flake:
optionalAttrs (flake?formatter.${system}) {