Coverage for src/rhiza_hooks/check_workflow_make_targets.py: 100%
105 statements
« prev ^ index » next coverage.py v7.16.1, created at 2026-09-15 06:14 +0000
1#!/usr/bin/env python3
2"""Check that every ``make`` target CI invokes actually exists.
4A workflow calling a target the Makefile no longer defines fails only when that
5workflow next runs — which, for a scheduled or path-filtered job, can be weeks
6after the commit that broke it. The template has already produced this failure:
7``make validate`` existed up to rhiza v1.1.3 and was removed by v1.2.1, so
8anything still naming it reported healthy repos as broken.
10``check-makefile-targets`` asserts that a handful of *recommended* targets exist.
11This hook checks the opposite direction — that the targets actually **invoked** are
12defined — which is what catches a removal or a rename.
14Both sides come from files in the repo, so the check is offline and mechanical:
16* **targets** — the root ``Makefile`` plus everything it ``include``s, transitively,
17 globs expanded (rhiza's own layout is ``Makefile`` → ``local.mk``, and was
18 ``Makefile`` → ``.rhiza/rhiza.mk`` → ``.rhiza/make.d/*.mk`` before v1.4.0); read by
19 :mod:`rhiza_hooks._makefile`, shared with ``check-makefile-targets``;
20* **invocations** — the shell snippets of every CI definition: ``run:`` in GitHub
21 workflows, ``script:``/``before_script:``/``after_script:`` in ``.gitlab-ci.yml``.
23A **catch-all rule silences this check**, and honestly so: with ``%:`` in the
24Makefile every name resolves, so an invocation of a target that does not exist is no
25longer distinguishable from one that does — make itself cannot tell either, which is
26why the rhiza-task shim leaves "unknown task" to the CLI. The comparison is skipped
27rather than guessed at, and the run says so in its summary; the alternative, reading
28the CLI's task list, would mean running it from a pre-commit hook.
30Invocations are read out of parsed YAML rather than raw text, so ``name: make sure
31the cache is warm`` cannot be mistaken for an invocation of a target called
32``sure``. An invocation naming a target through a variable or a matrix expression
33(``make ${{ matrix.task }}``) is unresolvable, and is skipped rather than reported:
34a false positive here blocks every commit, which is worse than a missed check.
36**Only inline shell steps are read.** A workflow that delegates to a reusable one::
38 jobs:
39 ci:
40 uses: jebel-quant/rhiza/.github/workflows/rhiza_ci.yml@v1.5.1
42keeps every command in another repository, behind a pinned ref. Nothing in the
43checkout holds them, so this hook cannot see them — and since that is the shape of
44every rhiza-managed repo, the check can pass while inspecting nothing at all. That
45is indistinguishable, from the outside, from passing on merit.
47Two things make the difference visible. The run always reports what it worked from
48(``inspected N CI file(s), found M resolvable make target invocation(s)``), and
49``--require-invocations`` turns a zero into a failure, for a repo that believes it
50has inline invocations to check. The flag is opt-in because zero is the correct,
51permanent answer for a repo whose CI is entirely delegated: making it fatal by
52default would report every such repo as broken, which is the mistake this hook
53already refuses to make for unresolvable target names.
55Following ``uses:`` into the reusable workflow would restore real coverage, but it
56means network access from a pre-commit hook against a pinned ref — a much larger
57change, and one better answered in the template's own CI, where a removed target is
58reachable without leaving the repo that removed it.
60Exit codes:
61 0 - every resolvable invocation names a defined target
62 1 - at least one invocation names a target nothing defines, or
63 ``--require-invocations`` was given and there were no invocations to check
64"""
66from __future__ import annotations
68import argparse
69import sys
70from itertools import chain, takewhile
71from pathlib import Path
72from typing import Any
74import yaml
76from rhiza_hooks._makefile import GLOB_CHARS, VARIABLE_CHARS, MakefileTargets, collect_targets
77from rhiza_hooks._repo import find_repo_root
79# Where CI definitions live. Workflow files are globbed; the GitLab file is a
80# fixed path. Both are optional — a repo may use either platform, or neither.
81WORKFLOW_GLOB = ".github/workflows/*.y*ml"
82GITLAB_CI = ".gitlab-ci.yml"
84# YAML keys whose values are shell snippets, across both platforms.
85_SHELL_KEYS = frozenset({"run", "script", "before_script", "after_script"})
87# Shell tokens that end the command a `make` word belongs to.
88_SEPARATORS = frozenset({"&&", "||", ";", "|", "#"})
90# Make flags that consume the following token, so `make -C dir test` does not read
91# `dir` as a target. `-j` may appear with or without a value; treating its argument
92# as consumed only when it is not itself a target-shaped word is more machinery than
93# this is worth, so `-j` is listed and a bare `make -j test` loses `test`. Prefer
94# `make -j4 test`, which is what the workflows here write.
95_FLAGS_WITH_VALUE = frozenset(
96 {"-C", "--directory", "-f", "--file", "--makefile", "-I", "--include-dir", "-o", "-W", "-j", "--jobs"}
97)
99# A token that cannot be resolved to a literal target name: a variable or command
100# substitution, or a glob. Both meanings come from the makefile parser, so the two
101# hooks agree on what "unresolvable" means.
102_DYNAMIC = (*VARIABLE_CHARS, *GLOB_CHARS)
105def _command_strings(value: Any) -> list[str]:
106 """Return the shell strings held by the value of a command key.
108 A command key carries either one snippet (GitHub's ``run:``) or a list of them
109 (GitLab's ``script:``). Anything else — a number, a mapping — is not a command,
110 and non-string entries inside a list are dropped rather than crashing the walk.
111 """
112 if isinstance(value, str):
113 return [value]
114 if isinstance(value, list):
115 return [item for item in value if isinstance(item, str)]
116 return []
119def _snippets_under(key: Any, value: Any) -> list[str]:
120 """Return the snippets one mapping entry contributes: its own, or its subtree's."""
121 if key in _SHELL_KEYS:
122 return _command_strings(value)
123 return _shell_snippets(value)
126def _shell_snippets(node: Any) -> list[str]:
127 """Collect every shell snippet in a parsed CI document.
129 Walks the whole tree rather than assuming a platform's schema, gathering the
130 string values (and lists of strings) under the keys both platforms use for
131 commands.
132 """
133 if isinstance(node, dict):
134 return list(chain.from_iterable(_snippets_under(key, value) for key, value in node.items()))
135 if isinstance(node, list):
136 return list(chain.from_iterable(_shell_snippets(item) for item in node))
137 return []
140def _is_dynamic(word: str) -> bool:
141 """Report whether a word names something only make or a shell can resolve."""
142 return any(char in word for char in _DYNAMIC)
145def _drop_flags(words: list[str]) -> list[str]:
146 """Drop make's flags and, for the flags that take one, the value that follows."""
147 kept: list[str] = []
148 skip_next = False
149 for word in words:
150 if skip_next:
151 skip_next = False
152 elif word.startswith("-"):
153 skip_next = word in _FLAGS_WITH_VALUE
154 else:
155 kept.append(word)
156 return kept
159def _targets_in_command(words: list[str]) -> list[str]:
160 """Read the target names out of the words following a ``make`` token.
162 Stops at the first shell separator, skips flags (and the value of a flag that
163 takes one), and skips ``VAR=value`` overrides.
165 A single dynamic word abandons the **whole** command rather than just itself.
166 ``make ${{ matrix.task }}`` is three words once split, and dropping only the
167 ``${{`` would leave ``matrix.task`` and ``}}`` looking like targets — reporting
168 two invented names and blocking every commit. Losing a resolvable target that
169 happens to share a command with a dynamic one is the cheaper mistake. Flags are
170 dropped *first*, so a dynamic flag value (``make -C $DIR test``) costs nothing:
171 it is never a target name either way.
172 """
173 candidates = _drop_flags(list(takewhile(lambda word: word not in _SEPARATORS, words)))
174 if any(_is_dynamic(word) for word in candidates):
175 return []
176 return [word for word in candidates if "=" not in word]
179def invoked_targets(snippet: str) -> set[str]:
180 """Return the resolvable make targets a shell snippet invokes.
182 >>> sorted(invoked_targets("make fmt && make test"))
183 ['fmt', 'test']
185 Flags are dropped, including the value of a flag that takes one, so the
186 argument of ``-C`` is never read as a target:
188 >>> sorted(invoked_targets("make -C sub build"))
189 ['build']
191 ``VAR=value`` overrides are not targets either:
193 >>> sorted(invoked_targets("make CFLAGS=-O2 release"))
194 ['release']
196 A word only make or a shell can resolve abandons the whole command, rather
197 than contributing invented target names:
199 >>> sorted(invoked_targets("make ${{ matrix.task }}"))
200 []
201 """
202 targets: set[str] = set()
203 for line in snippet.splitlines():
204 words = line.replace(";", " ; ").replace("&&", " && ").split()
205 for index, word in enumerate(words):
206 if word == "make":
207 targets.update(_targets_in_command(words[index + 1 :]))
208 return targets
211def _ci_files(repo_root: Path) -> list[Path]:
212 """Return the CI definition files the repo actually has, in a stable order."""
213 candidates = [*sorted(repo_root.glob(WORKFLOW_GLOB)), repo_root / GITLAB_CI]
214 return [path for path in candidates if path.is_file()]
217def _targets_invoked_by(path: Path) -> set[str]:
218 """Return every make target one CI definition invokes.
220 A file that will not parse as YAML contributes nothing: ``check-yaml`` and
221 ``actionlint`` report that, and guessing at a broken document would only produce
222 noise.
223 """
224 try:
225 document = yaml.safe_load(path.read_text(encoding="utf-8"))
226 except (yaml.YAMLError, OSError, UnicodeDecodeError, ValueError, OverflowError):
227 return set()
228 targets: set[str] = set()
229 for snippet in _shell_snippets(document):
230 targets |= invoked_targets(snippet)
231 return targets
234def _ci_invocations(paths: list[Path], repo_root: Path) -> dict[str, set[str]]:
235 """Map each CI definition file that invokes make to the targets it names.
237 Takes the file list rather than finding it, so a caller that also needs to
238 report how many files were inspected does not walk the tree twice — and cannot
239 report a count that disagrees with what was actually read.
240 """
241 invocations: dict[str, set[str]] = {}
242 for path in paths:
243 targets = _targets_invoked_by(path)
244 if targets:
245 invocations[path.relative_to(repo_root).as_posix()] = targets
246 return invocations
249def _undefined_targets(defined: MakefileTargets, invocations: dict[str, set[str]]) -> list[str]:
250 """Report the invocations, already collected, that name a target nothing defines."""
251 if not defined:
252 return []
254 return [
255 f"{filename} runs `make {target}`, but no Makefile or include defines that target."
256 for filename, invoked in invocations.items()
257 for target in sorted(target for target in invoked if not defined.defines(target))
258 ]
261def summarize(ci_file_count: int, invocations: dict[str, set[str]], *, catch_all: bool = False) -> str:
262 """Return the one-line account of what a run actually inspected.
264 A guard that finds no input passes exactly like one that passes on merit, so the
265 run says what it read. The count is of distinct (file, target) pairs, which is
266 what the check compares — ``make fmt test`` contributes two, and a target named
267 twice in one file contributes one:
269 >>> summarize(4, {"ci.yml": {"fmt", "test"}, ".gitlab-ci.yml": {"test"}})
270 'inspected 4 CI file(s), found 3 resolvable `make` target invocation(s)'
272 Zero is the answer worth being able to see. It is what a repo whose CI is
273 entirely delegated to reusable workflows gets, and it means the check compared
274 nothing:
276 >>> summarize(8, {})
277 'inspected 8 CI file(s), found 0 resolvable `make` target invocation(s)'
279 A catch-all rule is the other way to compare nothing, and the more surprising
280 one, because the invocations are right there and every one of them passes:
282 >>> line = summarize(2, {"ci.yml": {"test"}}, catch_all=True)
283 >>> line.startswith("inspected 2 CI file(s), found 1 resolvable")
284 True
285 >>> line.endswith("a catch-all rule (`%:`) defines every name, so none was compared")
286 True
287 """
288 found = sum(len(targets) for targets in invocations.values())
289 line = f"inspected {ci_file_count} CI file(s), found {found} resolvable `make` target invocation(s)"
290 if catch_all:
291 line += "; a catch-all rule (`%:`) defines every name, so none was compared"
292 return line
295def check_workflow_make_targets(repo_root: Path) -> list[str]:
296 """Report every CI invocation of a make target that nothing defines.
298 Args:
299 repo_root: Root directory of the repository.
301 Returns:
302 List of error messages, one per (file, missing target) pair. Empty when
303 there is no Makefile — with no targets to compare against, every invocation
304 would be reported, which is noise rather than signal.
305 """
306 return _undefined_targets(collect_targets(repo_root), _ci_invocations(_ci_files(repo_root), repo_root))
309def main(argv: list[str] | None = None) -> int:
310 """Run the hook and return a process exit code."""
311 parser = argparse.ArgumentParser(description="Check every make target invoked by CI exists")
312 parser.add_argument(
313 "filenames",
314 nargs="*",
315 help="Filenames (ignored: a target removal must be caught as well as a workflow edit)",
316 )
317 parser.add_argument(
318 "--require-invocations",
319 action="store_true",
320 help="Fail when the repo has CI files but none of them invokes make, rather than passing vacuously",
321 )
322 args = parser.parse_args(argv) # filenames are consumed for pre-commit's sake and unused
324 repo_root = find_repo_root()
325 ci_files = _ci_files(repo_root)
326 invocations = _ci_invocations(ci_files, repo_root)
327 defined = collect_targets(repo_root)
328 errors = _undefined_targets(defined, invocations)
330 # Always, and before the errors: on a failing run this is the context for them, and
331 # on a passing one it is the only thing distinguishing a real check from an empty
332 # one. pre-commit hides a passing hook's output unless the hook sets `verbose: true`,
333 # which is why --require-invocations exists rather than this line alone.
334 print(summarize(len(ci_files), invocations, catch_all=defined.catch_all), file=sys.stderr)
336 # A repo with no CI files at all is not claiming to have invocations, so it is not
337 # held to the flag; one that ships CI files and yields nothing is what the flag is for.
338 if args.require_invocations and ci_files and not invocations:
339 errors.append(
340 "no CI file invokes `make`, and --require-invocations was given. "
341 "A workflow that delegates to a reusable one (`uses:`) keeps its commands in "
342 "another repository, which this hook does not read; drop the flag if that is "
343 "the intended shape of this repo's CI."
344 )
346 for error in errors:
347 print(f"ERROR: {error}", file=sys.stderr)
349 return 1 if errors else 0
352if __name__ == "__main__": # pragma: no mutate
353 sys.exit(main())