Skip to content

Commit ea194a5

Browse files
committed
flakes: ensure that flake packages don't throw during construction
Our `tools` construction returns `null` for packages that are missing (for whatever reason). While this works okay for our internal module system, external consumers of the packages output sometimes make the assumption that these are all valid derivations. At some point we added a filter that would remove the nulls, however this introduced even worse issues. To filter out the nulls, we need to evaluate each tool. But not all tools can be evaluated: some are stubs that throws (useful) errors. So now we're forced to supress removal notices and other messages to be able to evaluate the tool list. This commit refactors tools to return placeholder derivations. These can be safely filtered out if needed.
1 parent 20e71a4 commit ea194a5

File tree

7 files changed

+155
-89
lines changed

7 files changed

+155
-89
lines changed

.github/workflows/ci.yaml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ jobs:
3939
- run: rm -rf /opt&
4040

4141
- name: Check nixpkgs-unstable
42-
run: nix flake check -L --show-trace
42+
working-directory: ./dev
43+
run: nix flake check -L --show-trace --no-write-lockfile
4344

4445
- run: nix eval .#lib.x86_64-linux.run --show-trace
4546

@@ -58,4 +59,5 @@ jobs:
5859
- run: rm -rf /opt&
5960

6061
- name: Check nixpkgs-stable
62+
working-directory: ./dev
6163
run: nix flake check -L --show-trace --override-input nixpkgs github:NixOS/nixpkgs/nixos-25.05

dev/flake.nix

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
description = "A very basic flake";
3+
4+
inputs = {
5+
git-hooks.url = "path:..";
6+
};
7+
8+
outputs =
9+
{ git-hooks
10+
, ...
11+
}:
12+
{
13+
inherit (git-hooks) legacyPackages checks;
14+
};
15+
}

flake.nix

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@
4040
'';
4141
};
4242

43+
legacyPackages = self.packages;
44+
45+
# The set of tools exposed by git-hooks.
46+
# Each entry is guaranteed to be a derivation, but broken packages are not filtered out.
47+
# `nix flake check` will likely not work.
4348
packages = forAllSystems ({ exposed, ... }: exposed.packages // {
4449
default = exposed.packages.pre-commit;
4550
});
@@ -50,7 +55,7 @@
5055
};
5156
});
5257

53-
checks = forAllSystems ({ exposed, ... }: lib.filterAttrs (k: v: v != null) exposed.checks);
58+
checks = forAllSystems ({ exposed, ... }: exposed.checks);
5459

5560
lib = forAllSystems ({ exposed, ... }: { inherit (exposed) run; });
5661

modules/hooks.nix

Lines changed: 16 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2618,7 +2618,7 @@ in
26182618
# need version >= 0.4.0 for the --from-stdin flag
26192619
toolVersionCheck = lib.versionAtLeast convco.version "0.4.0";
26202620
in
2621-
lib.throwIf (convco == null || !toolVersionCheck) "The version of Nixpkgs used by git-hooks.nix does not have the `convco` package (>=0.4.0). Please use a more recent version of Nixpkgs."
2621+
lib.throwIfNot toolVersionCheck "The version of Nixpkgs used by git-hooks.nix does not have the `convco` package (>=0.4.0). Please use a more recent version of Nixpkgs."
26222622
builtins.toString
26232623
script;
26242624
stages = [ "commit-msg" ];
@@ -3089,9 +3089,7 @@ lib.escapeShellArgs (lib.concatMap (ext: [ "--ghc-opt" "-X${ext}" ]) hooks.fourm
30893089
"$PRE_COMMIT_COMMIT_MSG_SOURCE" --commit-msg-file "$1"
30903090
'';
30913091
in
3092-
lib.throwIf (hooks.gptcommit.package == null) "The version of Nixpkgs used by git-hooks.nix does not have the `gptcommit` package. Please use a more recent version of Nixpkgs."
3093-
toString
3094-
script;
3092+
toString script;
30953093
stages = [ "prepare-commit-msg" ];
30963094
};
30973095
hadolint =
@@ -3110,13 +3108,7 @@ lib.escapeShellArgs (lib.concatMap (ext: [ "--ghc-opt" "-X${ext}" ]) hooks.fourm
31103108
## https://github.com/Frama-C/headache/blob/master/config_builtin.txt
31113109
files = "(\\.ml[ily]?$)|(\\.fmli?$)|(\\.[chy]$)|(\\.tex$)|(Makefile)|(README)|(LICENSE)";
31123110
package = tools.headache;
3113-
entry =
3114-
## NOTE: `headache` made into in nixpkgs on 12 April 2023. At the
3115-
## next NixOS release, the following code will become irrelevant.
3116-
lib.throwIf
3117-
(hooks.headache.package == null)
3118-
"The version of nixpkgs used by git-hooks.nix does not have `ocamlPackages.headache`. Please use a more recent version of nixpkgs."
3119-
"${hooks.headache.package}/bin/headache -h ${hooks.headache.settings.header-file}";
3111+
entry = "${hooks.headache.package}/bin/headache -h ${hooks.headache.settings.header-file}";
31203112
};
31213113
hindent =
31223114
{
@@ -3646,16 +3638,7 @@ lib.escapeShellArgs (lib.concatMap (ext: [ "--ghc-opt" "-X${ext}" ]) hooks.fourm
36463638
pre-commit-hook-ensure-sops = {
36473639
name = "pre-commit-hook-ensure-sops";
36483640
package = tools.pre-commit-hook-ensure-sops;
3649-
entry =
3650-
## NOTE: pre-commit-hook-ensure-sops landed in nixpkgs on 8 July 2022. Once it reaches a
3651-
## release of NixOS, the `throwIf` piece of code below will become
3652-
## useless.
3653-
lib.throwIf
3654-
(hooks.pre-commit-hook-ensure-sops.package == null)
3655-
"The version of nixpkgs used by git-hooks.nix does not have the `pre-commit-hook-ensure-sops` package. Please use a more recent version of nixpkgs."
3656-
''
3657-
${hooks.pre-commit-hook-ensure-sops.package}/bin/pre-commit-hook-ensure-sops
3658-
'';
3641+
entry = "${hooks.pre-commit-hook-ensure-sops.package}/bin/pre-commit-hook-ensure-sops";
36593642
files = "^secrets";
36603643
};
36613644
# See all CLI flags for prettier [here](https://prettier.io/docs/en/cli.html).
@@ -4134,25 +4117,17 @@ lib.escapeShellArgs (lib.concatMap (ext: [ "--ghc-opt" "-X${ext}" ]) hooks.fourm
41344117
description = "A universal formatter engine within the Tree-sitter ecosystem, with support for many languages.";
41354118
package = tools.topiary;
41364119
entry =
4137-
## NOTE: Topiary landed in nixpkgs on 2 Dec 2022. Once it reaches a
4138-
## release of NixOS, the `throwIf` piece of code below will become
4139-
## useless.
4140-
lib.throwIf
4141-
(hooks.topiary.package == null)
4142-
"The version of nixpkgs used by git-hooks.nix does not have the `topiary` package. Please use a more recent version of nixpkgs."
4143-
(
4144-
let
4145-
topiary-inplace = pkgs.writeShellApplication {
4146-
name = "topiary-inplace";
4147-
text = ''
4148-
for file; do
4149-
${hooks.topiary.package}/bin/topiary --in-place --input-file "$file"
4150-
done
4151-
'';
4152-
};
4153-
in
4154-
"${topiary-inplace}/bin/topiary-inplace"
4155-
);
4120+
let
4121+
topiary-inplace = pkgs.writeShellApplication {
4122+
name = "topiary-inplace";
4123+
text = ''
4124+
for file; do
4125+
${hooks.topiary.package}/bin/topiary --in-place --input-file "$file"
4126+
done
4127+
'';
4128+
};
4129+
in
4130+
"${topiary-inplace}/bin/topiary-inplace";
41564131
files = "(\\.json$)|(\\.toml$)|(\\.mli?$)";
41574132
};
41584133
treefmt =
@@ -4264,11 +4239,7 @@ lib.escapeShellArgs (lib.concatMap (ext: [ "--ghc-opt" "-X${ext}" ]) hooks.fourm
42644239
name = "typstyle";
42654240
description = "Beautiful and reliable typst code formatter";
42664241
package = tools.typstyle;
4267-
entry =
4268-
lib.throwIf
4269-
(hooks.typstyle.package == null)
4270-
"The version of nixpkgs used by git-hooks.nix must contain typstyle"
4271-
"${hooks.typstyle.package}/bin/typstyle -i";
4242+
entry = "${hooks.typstyle.package}/bin/typstyle -i";
42724243
files = "\\.typ$";
42734244
};
42744245
uv-check = {

nix/call-tools.nix

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,17 @@
11
pkgs:
2-
pkgs.lib.flip builtins.removeAttrs [ "override" "overrideDerivation" ]
3-
(pkgs.callPackage ./tools.nix {
4-
cabal-fmt = (pkgs.haskell.lib.enableSeparateBinOutput pkgs.haskellPackages.cabal-fmt).bin;
5-
cabal-gild = (pkgs.haskell.lib.enableSeparateBinOutput pkgs.haskellPackages.cabal-gild).bin;
6-
hindent = pkgs.haskell.lib.enableSeparateBinOutput pkgs.haskellPackages.hindent;
7-
})
2+
pkgs.lib.flip builtins.removeAttrs [ "override" "overrideDerivation" ] (
3+
pkgs.callPackage ./tools.nix {
4+
placeholder = name: {
5+
# Allows checking without forcing evaluation
6+
meta.isPlaceholder = true;
7+
8+
# Throw when the package is actually used
9+
outPath = throw ''
10+
git-hooks: the package `${name} is not available in your nixpkgs revision.
11+
'';
12+
13+
type = "derivation";
14+
name = name + "placeholder";
15+
};
16+
}
17+
)

nix/default.nix

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,46 @@ let
1919
;
2020
};
2121

22-
# Filter out any broken or missing packages from our tests.
23-
filterBrokenPackages = n: package: package != null && !(package.meta.broken or false);
22+
# Filter out broken and placeholder packages.
23+
filterPackageForCheck =
24+
name: package:
25+
let
26+
isPlaceholder = package.meta.isPlaceholder or false;
27+
isBroken = package.meta.broken or false;
28+
29+
check = builtins.tryEval (!(isPlaceholder || isBroken));
30+
result = check.success && check.value;
31+
32+
message =
33+
if !check.success then
34+
''
35+
Skipping ${name} because it failed to evaluate.
36+
''
37+
else if !check.value && isPlaceholder then
38+
''
39+
Skipping ${name} because it is missing from this nixpkgs revision.
40+
''
41+
else if !check.value && isBroken then
42+
''
43+
Skipping ${name} because it is marked as broken.
44+
''
45+
else
46+
""; # Not used
47+
48+
in
49+
lib.warnIf (!result) message result;
2450
in
2551
{
2652
inherit tools run;
27-
# Flake style attributes
28-
packages = (lib.filterAttrs filterBrokenPackages tools) // {
53+
54+
# Flake-style attributes
55+
# Do not remove: these are exposed in the flake.
56+
# Each should also be a valid derivation.
57+
packages = tools // {
2958
inherit (pkgs) pre-commit;
3059
};
31-
checks = self.packages // {
60+
61+
checks = (lib.filterAttrs filterPackageForCheck tools) // {
3262
# A pre-commit-check for nix-pre-commit itself
3363
pre-commit-check = run {
3464
src = ../.;
@@ -57,7 +87,7 @@ let
5787
f n h.package;
5888

5989
allEntryPoints = lib.pipe allHooks [
60-
(lib.filterAttrs (getPackage filterBrokenPackages))
90+
(lib.filterAttrs (getPackage filterPackageForCheck))
6191
(lib.mapAttrsToList getEntry)
6292
];
6393
in

0 commit comments

Comments
 (0)