From faa8dea32895dee0f201722b2fa3508520d57355 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 25 Oct 2026 19:58:48 -0700 Subject: [PATCH 1/4] Answer --help and Refuse Unknown Arguments in Every Script Every tracked script now answers --help and -h with its usage on stdout, exits 0, and refuses an argument it does not recognize, before it reads anything. make-release.sh checks its arguments before it sources an environment file, so a flag can no longer start a release. The checks/ gates and the capture/ scripts parse with argparse. The two shell scripts in checks/ and deploy/, and run-wp2hugo.sh, check their own arguments. checks/tests/test_cli_usage.py discovers every tracked executable and runs each with --help, -h, and an unknown flag, under stubbed commands and empty scratch directories, and fails on any effect. A script it does not classify fails too, so a new one is held to the rule. Refs #539 Co-Authored-By: Claude Opus 5.5 --- capture/build_golden.py | 4 + capture/build_redirects.py | 26 ++-- capture/classify.py | 4 + capture/clean_content.py | 13 +- capture/enumerate_media.py | 4 + capture/localize_external.py | 13 +- capture/restructure_content.py | 13 +- capture/run-wp2hugo.sh | 14 ++ checks/check-live-urls.sh | 33 ++++- checks/env_docs_gate.py | 9 +- checks/github_env_gate.py | 5 +- checks/media_metadata_gate.py | 5 +- checks/media_redactions_gate.py | 5 +- checks/tests/test_cli_usage.py | 199 +++++++++++++++++++++++++++ checks/tests/test_github_env_gate.py | 2 +- checks/url_parity_gate.py | 13 +- deploy/make-release.sh | 34 ++++- 17 files changed, 343 insertions(+), 53 deletions(-) create mode 100644 checks/tests/test_cli_usage.py diff --git a/capture/build_golden.py b/capture/build_golden.py index 59aed2c6..55f9e251 100755 --- a/capture/build_golden.py +++ b/capture/build_golden.py @@ -17,6 +17,7 @@ choose to redirect to the parent post rather than reproduce. """ +import argparse import json import math import os @@ -29,6 +30,9 @@ from pathlib import Path from queue import Queue +# Parsed before the capture values are read, so --help answers where none is set. +argparse.ArgumentParser(description=(__doc__ or "").partition("\n")[0]).parse_args() + def env(name: str) -> str: """A required capture value, refused rather than guessed when unset. diff --git a/capture/build_redirects.py b/capture/build_redirects.py index 0001c850..8c515e7e 100755 --- a/capture/build_redirects.py +++ b/capture/build_redirects.py @@ -9,6 +9,7 @@ that read the capture rather than in checks/ beside the gates. """ +import argparse import os import pathlib import re @@ -69,27 +70,20 @@ def write_map(path, pairs): return len(pairs) -def main(argv): +def main(argv=None): # CAPTURE_ROOT is the default and an argument wins over it, the same shape DEPLOY_ROOT uses. # Unset and unsupplied is refused rather than guessed: a wrong-but-plausible capture yields maps that are empty and indistinguishable from working ones. # --print-export names the selected export and writes nothing. # The run-wp2hugo.sh script calls it rather than reimplementing the choice, so the conversion and the maps are provably built from the same file. # A shell reimplementation cannot match this anyway: the test is that ONE item carries both post_type=post and status=publish, and two greps over a whole file would accept a media-only export that happens to contain both words. - args = [a for a in argv[1:] if a != "--print-export"] - print_export = "--print-export" in argv[1:] - # An unknown option is refused rather than taken as a path. - # Without this, `--help` is read as a capture directory and the run fails with "no export XML found under --help", which sends the reader looking for a missing file rather than a mistyped flag. - unknown = [a for a in args if a.startswith("-")] - if unknown: - print(f"unknown option: {unknown[0]}", file=sys.stderr) - print(f"usage: {argv[0]} [--print-export] [capture-dir]", file=sys.stderr) - return 2 - if len(args) > 1: - print(f"usage: {argv[0]} [--print-export] [capture-dir]", file=sys.stderr) - return 2 - root = args[0] if args else os.environ.get("CAPTURE_ROOT", "") + parser = argparse.ArgumentParser(description=(__doc__ or "").partition("\n")[0]) + parser.add_argument("--print-export", action="store_true", help="name the selected export and write nothing") + parser.add_argument("capture", nargs="?", help="the capture directory, else CAPTURE_ROOT") + args = parser.parse_args(argv) + print_export = args.print_export + root = args.capture or os.environ.get("CAPTURE_ROOT", "") if not root: - print(f"usage: {argv[0]} [--print-export] [capture-dir]", file=sys.stderr) + parser.print_usage(sys.stderr) print("CAPTURE_ROOT is not set and no capture directory was given", file=sys.stderr) print("see .secrets/example.env and ENVIRONMENT.md", file=sys.stderr) return 2 @@ -273,4 +267,4 @@ def main(argv): if __name__ == "__main__": - sys.exit(main(sys.argv)) + sys.exit(main()) diff --git a/capture/classify.py b/capture/classify.py index 33f1a954..ec2aa34f 100755 --- a/capture/classify.py +++ b/capture/classify.py @@ -9,6 +9,7 @@ from it is an attachment page, not content. """ +import argparse import os import re import sys @@ -16,6 +17,9 @@ from collections import Counter from pathlib import Path +# Parsed before the capture values are read, so --help answers where none is set. +argparse.ArgumentParser(description=(__doc__ or "").partition("\n")[0]).parse_args() + def env(name: str) -> str: diff --git a/capture/clean_content.py b/capture/clean_content.py index 54eea800..bc454729 100755 --- a/capture/clean_content.py +++ b/capture/clean_content.py @@ -30,6 +30,7 @@ consumes it), and every `_`-prefixed WordPress internal. """ +import argparse import os import re import shutil @@ -59,15 +60,15 @@ def capture_root() -> pathlib.Path: return pathlib.Path(root) -def converted_site(argv) -> pathlib.Path: +def converted_site(site: str | None) -> pathlib.Path: """The converted site: a first argument, else the one generated-* under the capture. Ambiguity aborts rather than picking, the same rule build_redirects.py applies to the export. wp2hugo stamps the directory with a run timestamp, so there is no fixed name to default to and a glob that matched two would otherwise choose by filesystem order. """ - if len(argv) > 1 and not argv[1].startswith("--"): - return pathlib.Path(argv[1]) + if site: + return pathlib.Path(site) found = sorted((capture_root() / "converted").glob("generated-*")) if len(found) != 1: sys.exit( @@ -123,4 +124,8 @@ def main(root: Path, apply: bool): if __name__ == "__main__": - main(converted_site(sys.argv), "--apply" in sys.argv) + parser = argparse.ArgumentParser(description=(__doc__ or "").partition("\n")[0]) + parser.add_argument("site", nargs="?", help="the converted site, else the one generated-* under the capture") + parser.add_argument("--apply", action="store_true", help="write the changes rather than report them") + args = parser.parse_args() + main(converted_site(args.site), args.apply) diff --git a/capture/enumerate_media.py b/capture/enumerate_media.py index 3b7b7cce..78f66317 100755 --- a/capture/enumerate_media.py +++ b/capture/enumerate_media.py @@ -15,6 +15,7 @@ Writes: media-urls.tsv (host, url, post_id, post_url), posts.json (raw archive). """ +import argparse import json import os import re @@ -24,6 +25,9 @@ from collections import Counter, defaultdict from pathlib import Path +# Parsed before the capture values are read, so --help answers where none is set. +argparse.ArgumentParser(description=(__doc__ or "").partition("\n")[0]).parse_args() + def env(name: str) -> str: """A required capture value, refused rather than guessed when unset. diff --git a/capture/localize_external.py b/capture/localize_external.py index fc31d74f..3592e117 100755 --- a/capture/localize_external.py +++ b/capture/localize_external.py @@ -20,6 +20,7 @@ the mapping in external-media-map.tsv for auditability, and rewrites every reference. """ +import argparse import hashlib import pathlib import os @@ -58,15 +59,15 @@ def capture_root() -> pathlib.Path: return pathlib.Path(root) -def converted_site(argv) -> pathlib.Path: +def converted_site(site: str | None) -> pathlib.Path: """The converted site: a first argument, else the one generated-* under the capture. Ambiguity aborts rather than picking, the same rule build_redirects.py applies to the export. wp2hugo stamps the directory with a run timestamp, so there is no fixed name to default to and a glob that matched two would otherwise choose by filesystem order. """ - if len(argv) > 1 and not argv[1].startswith("--"): - return pathlib.Path(argv[1]) + if site: + return pathlib.Path(site) found = sorted((capture_root() / "converted").glob("generated-*")) if len(found) != 1: sys.exit( @@ -176,4 +177,8 @@ def main(site: pathlib.Path, apply: bool): if __name__ == "__main__": - sys.exit(main(converted_site(sys.argv), "--apply" in sys.argv)) + parser = argparse.ArgumentParser(description=(__doc__ or "").partition("\n")[0]) + parser.add_argument("site", nargs="?", help="the converted site, else the one generated-* under the capture") + parser.add_argument("--apply", action="store_true", help="write the changes rather than report them") + args = parser.parse_args() + sys.exit(main(converted_site(args.site), args.apply)) diff --git a/capture/restructure_content.py b/capture/restructure_content.py index e5fe39a7..afe06cff 100755 --- a/capture/restructure_content.py +++ b/capture/restructure_content.py @@ -18,6 +18,7 @@ make Hugo publish a `/pages/` listing URL, the same unwanted extra as `/posts/`. """ +import argparse import collections import os import pathlib @@ -43,15 +44,15 @@ def capture_root() -> pathlib.Path: return pathlib.Path(root) -def converted_site(argv) -> pathlib.Path: +def converted_site(site: str | None) -> pathlib.Path: """The converted site: a first argument, else the one generated-* under the capture. Ambiguity aborts rather than picking, the same rule build_redirects.py applies to the export. wp2hugo stamps the directory with a run timestamp, so there is no fixed name to default to and a glob that matched two would otherwise choose by filesystem order. """ - if len(argv) > 1 and not argv[1].startswith("--"): - return pathlib.Path(argv[1]) + if site: + return pathlib.Path(site) found = sorted((capture_root() / "converted").glob("generated-*")) if len(found) != 1: sys.exit( @@ -149,4 +150,8 @@ def main(site: pathlib.Path, apply: bool): if __name__ == "__main__": - sys.exit(main(converted_site(sys.argv), "--apply" in sys.argv)) + parser = argparse.ArgumentParser(description=(__doc__ or "").partition("\n")[0]) + parser.add_argument("site", nargs="?", help="the converted site, else the one generated-* under the capture") + parser.add_argument("--apply", action="store_true", help="write the changes rather than report them") + args = parser.parse_args() + sys.exit(main(converted_site(args.site), args.apply)) diff --git a/capture/run-wp2hugo.sh b/capture/run-wp2hugo.sh index 8c6109df..0d58e6cd 100755 --- a/capture/run-wp2hugo.sh +++ b/capture/run-wp2hugo.sh @@ -8,6 +8,20 @@ # The selection cannot be reimplemented in shell: the test is that ONE item carries both post_type=post and status=publish, where two greps over a whole file would accept the media-only export. set -Eeuo pipefail +# Arguments are checked before CAPTURE_ROOT is read, so --help answers where none is set. +case "${1:-}" in +-h | --help) + echo "usage: $0" + echo " converts the export build_redirects.py selects under CAPTURE_ROOT" + exit 0 + ;; +esac +if [ "$#" -gt 0 ]; then + echo "unknown argument: $1" >&2 + echo "usage: $0" >&2 + exit 2 +fi + # A `go install`ed wp2hugo lands here. $HOME rather than a literal path. export PATH="$HOME/.local/bin:$PATH" diff --git a/checks/check-live-urls.sh b/checks/check-live-urls.sh index ec2ec6ed..7732198d 100755 --- a/checks/check-live-urls.sh +++ b/checks/check-live-urls.sh @@ -5,17 +5,36 @@ set -uo pipefail +usage() { + echo "usage: $0 | --print-sample" +} + PRINT_SAMPLE='' BASE="${1:-}" -if [ "$BASE" = "--print-sample" ]; then +if [ "$#" -gt 1 ]; then + echo "unexpected argument: $2" >&2 + usage >&2 + exit 2 +fi +case "$BASE" in +-h | --help) + usage + exit 0 + ;; +--print-sample) PRINT_SAMPLE=1 BASE='' -else - [ -n "$BASE" ] || { - echo "usage: $0 | --print-sample" >&2 - exit 2 - } -fi + ;; +'') + usage >&2 + exit 2 + ;; +-*) + echo "unknown option: $BASE" >&2 + usage >&2 + exit 2 + ;; +esac BASE="${BASE%/}" case "${SAMPLE_CONTRACT:-}" in diff --git a/checks/env_docs_gate.py b/checks/env_docs_gate.py index 2908de35..96ccb275 100755 --- a/checks/env_docs_gate.py +++ b/checks/env_docs_gate.py @@ -24,6 +24,7 @@ from __future__ import annotations +import argparse import re import sys from pathlib import Path @@ -36,7 +37,7 @@ # Workflow references. # `vars.X` and `secrets.X` are the GitHub Environment surface, and a value added there is exactly as undocumented as one added to a template. -WORKFLOWS = sorted((REPO / ".github" / "workflows").glob("*.yml")) +WORKFLOWS = REPO / ".github" / "workflows" # Set per invocation rather than stored, so they appear in no template and would otherwise be invisible to this check. # Listed here because the doc has a table for them, and a knob nobody documented is the same failure as an undocumented file value. @@ -76,7 +77,9 @@ DOC_INLINE = re.compile(r"`([A-Z][A-Z0-9_]{2,})(?:=[^`]*)?`") -def main() -> int: +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=(__doc__ or "").partition("\n")[0]) + parser.parse_args(argv) if not DOC.exists(): print(f"ERROR: {DOC.name} does not exist", file=sys.stderr) return 1 @@ -103,7 +106,7 @@ def note(name: str, where: str) -> None: for name in COMMENTED_DECL.findall(text): note(name, rel) - for path in WORKFLOWS: + for path in sorted(WORKFLOWS.glob("*.yml")): rel = path.relative_to(REPO).as_posix() for name in GH_REF.findall(path.read_text(encoding="utf-8")): note(name, rel) diff --git a/checks/github_env_gate.py b/checks/github_env_gate.py index 79969f39..e00fd8cb 100755 --- a/checks/github_env_gate.py +++ b/checks/github_env_gate.py @@ -29,6 +29,7 @@ from __future__ import annotations +import argparse import re import subprocess import sys @@ -191,7 +192,9 @@ def other(kind: str) -> str: return findings -def main() -> int: +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=(__doc__ or "").partition("\n")[0]) + parser.parse_args(argv) try: listed = read_table(DOC.read_text(encoding="utf-8")) except (OSError, ValueError) as error: diff --git a/checks/media_metadata_gate.py b/checks/media_metadata_gate.py index a5f33ade..f815bad3 100755 --- a/checks/media_metadata_gate.py +++ b/checks/media_metadata_gate.py @@ -25,6 +25,7 @@ pixels and writing a fresh file that is clean by construction. """ +import argparse import hashlib import lzma import os @@ -1439,7 +1440,9 @@ def walk_archive(path: pathlib.Path, name: str) -> list[tuple[str, set[str]]]: return out -def main() -> int: +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=(__doc__ or "").partition("\n")[0]) + parser.parse_args(argv) found = findings() scanned = sum( 1 diff --git a/checks/media_redactions_gate.py b/checks/media_redactions_gate.py index 0ecbbbce..a6779939 100755 --- a/checks/media_redactions_gate.py +++ b/checks/media_redactions_gate.py @@ -7,6 +7,7 @@ `scripts/redact_media.py` is what brings either back. """ +import argparse import hashlib import json import pathlib @@ -22,7 +23,9 @@ def declared(entry: dict) -> str: return hashlib.sha256(json.dumps(spec, sort_keys=True).encode()).hexdigest() -def main() -> int: +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=(__doc__ or "").partition("\n")[0]) + parser.parse_args(argv) files = json.loads(MANIFEST.read_text())["files"] failures = [] for name, entry in files.items(): diff --git a/checks/tests/test_cli_usage.py b/checks/tests/test_cli_usage.py new file mode 100644 index 00000000..d60c24c6 --- /dev/null +++ b/checks/tests/test_cli_usage.py @@ -0,0 +1,199 @@ +"""Hold every tracked script to the fleet's command-line argument rule by running it. + +Each script runs with `--help`, with `-h`, and with an unknown `--bogus-flag`. Help exits 0 and +prints the same text for both spellings, opening on a usage line that names the script. The unknown +flag exits non-zero with a refusal naming it. A script needing a positional refuses on the missing +argument before it reaches the flag, so BEFORE_FLAG gives it one, and every other script is +declared in NO_REQUIRED_ARGUMENT. A script declared in neither fails, so a new one is classified +the moment it is tracked. + +No run may change anything. Stubs ahead of the real commands on PATH record any call to them, the +home and working directories are empty scratch directories that must stay empty, and the tree's +status must not move. ENV_FILE names a file that does not exist, so make-release.sh refuses rather +than building should its argument check ever regress. +""" + +import importlib.util +import os +import pathlib +import re +import subprocess +import sys +import tempfile +import unittest + +REPO = pathlib.Path(__file__).resolve().parent.parent.parent + +BEFORE_FLAG = { + "checks/csp_browser_gate.py": ("https://example.invalid",), + "checks/csp_gate.py": ("public",), + "checks/url_parity_gate.py": ("public",), +} + +NO_REQUIRED_ARGUMENT = frozenset( + { + "capture/build_golden.py", + "capture/build_redirects.py", + "capture/classify.py", + "capture/clean_content.py", + "capture/enumerate_media.py", + "capture/localize_external.py", + "capture/restructure_content.py", + "capture/run-wp2hugo.sh", + "checks/check-live-urls.sh", + "checks/env_docs_gate.py", + "checks/github_env_gate.py", + "checks/media_metadata_gate.py", + "checks/media_parser_fuzz_gate.py", + "checks/media_redactions_gate.py", + "checks/text_pii_gate.py", + "deploy/make-release.sh", + "scripts/normalize_media.py", + "scripts/normalize_text.py", + "scripts/redact_media.py", + } +) + +# A third-party module a script imports before it parses, skipped where this interpreter lacks it. +NEEDS_MODULE = { + "capture/clean_content.py": "yaml", + "scripts/redact_media.py": "PIL", +} + +REFUSED_AS_OPTION = re.compile( + r"(unknown|unrecogni[sz]ed) (option|argument)[^\n]*--bogus-flag", re.IGNORECASE +) + +RECORDED = ("git", "gh", "curl", "wget", "ssh", "rsync", "docker", "hugo", "uv", "uvx") + + +def entry_points() -> list[str]: + """Every tracked executable outside the vendored theme and the test directories.""" + listed = subprocess.run( + ["git", "-C", str(REPO), "ls-files", "-s", "-z"], + capture_output=True, + check=True, + ).stdout.decode("utf-8") + found = [] + for record in filter(None, listed.split("\0")): + mode, path = record.split(" ", 1)[0], record.split("\t", 1)[1] + parts = pathlib.PurePosixPath(path).parts + if mode == "100755" and parts[0] != "themes" and "tests" not in parts: + found.append(path) + return sorted(found) + + +def tree_status() -> str: + return subprocess.run( + [ + "git", + "-C", + str(REPO), + "status", + "--porcelain", + "--ignored", + "--untracked-files=all", + ], + capture_output=True, + check=True, + text=True, + ).stdout + + +class CliUsageTests(unittest.TestCase): + def setUp(self) -> None: + scratch = tempfile.TemporaryDirectory() + self.addCleanup(scratch.cleanup) + self.root = pathlib.Path(scratch.name) + stubs = self.root / "stubs" + stubs.mkdir() + self.record = self.root / "recorded" + for name in RECORDED: + stub = stubs / name + stub.write_text(f'#!/bin/sh\necho {name} >>"{self.record}"\nexit 97\n') + stub.chmod(0o755) + self.env = { + key: value + for key, value in os.environ.items() + if not key.startswith(("CAPTURE_", "DEPLOY_", "SITE_", "GH_", "GITHUB_")) + } + # A script importing a sibling would otherwise write __pycache__ into the tree. + self.env.update( + ENV_FILE=str(self.root / "absent.env"), + PATH=f"{stubs}{os.pathsep}{os.environ.get('PATH', '')}", + PYTHONDONTWRITEBYTECODE="1", + ) + + def fresh(self, path: str) -> None: + """Empty home and working directories of this script's own, so one failure stays its own.""" + self.record.unlink(missing_ok=True) + base = self.root / path.replace("/", "_") + self.home, self.cwd = base / "home", base / "cwd" + self.home.mkdir(parents=True) + self.cwd.mkdir() + self.env["HOME"] = str(self.home) + + def run_script(self, path: str, *args: str) -> subprocess.CompletedProcess[str]: + interpreter = [sys.executable] if path.endswith(".py") else ["bash"] + return subprocess.run( + [*interpreter, str(REPO / path), *args], + check=False, + capture_output=True, + text=True, + cwd=self.cwd, + env=self.env, + timeout=60, + ) + + def assert_no_effect(self, path: str, before: str) -> None: + called = self.record.read_text().split() if self.record.exists() else [] + self.assertEqual([], called, f"{path} called a stubbed command") + self.assertEqual([], list(self.home.iterdir()), f"{path} wrote into HOME") + self.assertEqual( + [], list(self.cwd.iterdir()), f"{path} wrote into its working directory" + ) + self.assertEqual(before, tree_status(), f"{path} changed the tree") + + def test_every_entry_point_is_declared(self) -> None: + found = set(entry_points()) + self.assertEqual( + set(), found - NO_REQUIRED_ARGUMENT - set(BEFORE_FLAG), "undeclared" + ) + self.assertEqual( + set(), + (NO_REQUIRED_ARGUMENT | set(BEFORE_FLAG)) - found, + "declared, not tracked", + ) + + def test_help_answers_and_unknown_flag_is_refused(self) -> None: + for path in entry_points(): + with self.subTest(path=path): + module = NEEDS_MODULE.get(path) + if module and importlib.util.find_spec(module) is None: + self.skipTest( + f"{path} needs {module}, which this interpreter lacks" + ) + self.fresh(path) + before = tree_status() + long, short = ( + self.run_script(path, "--help"), + self.run_script(path, "-h"), + ) + self.assertEqual(0, long.returncode, long.stderr) + self.assertEqual(0, short.returncode, short.stderr) + self.assertEqual(long.stdout, short.stdout) + self.assertRegex( + long.stdout.splitlines()[0] if long.stdout else "", + rf"^usage: \S*{re.escape(pathlib.PurePosixPath(path).name)}\b", + ) + refused = self.run_script( + path, *BEFORE_FLAG.get(path, ()), "--bogus-flag" + ) + self.assertNotEqual(0, refused.returncode) + self.assertNotIn("Traceback", refused.stderr) + self.assertRegex(refused.stderr, REFUSED_AS_OPTION) + self.assert_no_effect(path, before) + + +if __name__ == "__main__": + unittest.main() diff --git a/checks/tests/test_github_env_gate.py b/checks/tests/test_github_env_gate.py index ad6ce5f5..97de429e 100644 --- a/checks/tests/test_github_env_gate.py +++ b/checks/tests/test_github_env_gate.py @@ -210,7 +210,7 @@ def run_main(self, held=None, error=None) -> tuple[int, str, str]: contextlib.redirect_stdout(out), contextlib.redirect_stderr(err), ): - code = gate.main() + code = gate.main([]) return code, out.getvalue(), err.getvalue() def test_agreement_exits_zero(self) -> None: diff --git a/checks/url_parity_gate.py b/checks/url_parity_gate.py index cf19d392..a55d74cc 100755 --- a/checks/url_parity_gate.py +++ b/checks/url_parity_gate.py @@ -7,6 +7,7 @@ are checked by check-live-urls.sh instead. """ +import argparse import pathlib import re import sys @@ -385,10 +386,12 @@ def check_galleries(public): return findings -def main(argv): - if len(argv) != 2: - sys.exit(f"usage: {argv[0]} ") - public = pathlib.Path(argv[1]) +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=(__doc__ or "").partition("\n")[0]) + parser.add_argument( + "public", type=pathlib.Path, help="the built site, as hugo wrote it" + ) + public = parser.parse_args(argv).public if not public.is_dir(): sys.exit(f"FAIL: {public} is not a directory - run hugo first") @@ -420,4 +423,4 @@ def main(argv): if __name__ == "__main__": - sys.exit(main(sys.argv)) + sys.exit(main()) diff --git a/deploy/make-release.sh b/deploy/make-release.sh index e6ecc93c..99d03aae 100755 --- a/deploy/make-release.sh +++ b/deploy/make-release.sh @@ -10,13 +10,32 @@ umask 022 KEEP_RELEASES=10 usage() { - echo "usage: $0 [deploy-root] [version]" >&2 - echo " deploy-root defaults to DEPLOY_ROOT, from the environment or \$ENV_FILE" >&2 - echo " ENV_FILE defaults to ~/.secrets/blog.local.production.env, and a relative path resolves against ~/.secrets" >&2 - echo " deploy-root is required when the environment file sets DEPLOY_SSH_HOST, since that root is on another host" >&2 - exit 2 + echo "usage: $0 [deploy-root] [version]" + echo " deploy-root defaults to DEPLOY_ROOT, from the environment or \$ENV_FILE" + echo " ENV_FILE defaults to ~/.secrets/blog.local.production.env, and a relative path resolves against ~/.secrets" + echo " deploy-root is required when the environment file sets DEPLOY_SSH_HOST, since that root is on another host" } +# Arguments are checked before the environment file is read, since a flag taken as the deploy root starts a release. +for arg in "$@"; do + case "$arg" in + -h | --help) + usage + exit 0 + ;; + -*) + echo "unknown option: $arg" >&2 + usage >&2 + exit 2 + ;; + esac +done +if [ "$#" -gt 2 ]; then + echo "unexpected argument: $3" >&2 + usage >&2 + exit 2 +fi + REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" # The release upstream replaced `git whatchanged` with `git log`. @@ -74,7 +93,10 @@ if [ -z "$ROOT_ARG" ] && [ -n "${DEPLOY_SSH_HOST:-}" ]; then fi ROOT="${ROOT_ARG:-${DEPLOY_ROOT:-}}" -[ -n "$ROOT" ] || usage +[ -n "$ROOT" ] || { + usage >&2 + exit 2 +} # CI passes the version so a release directory traces back to the deploy run that built it. VERSION="${2:-$(date -u +%Y%m%d-%H%M%S)}" From 5b473c6deece37a9fbfc2b8a840507de3c531f2d Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 25 Oct 2026 20:12:08 -0700 Subject: [PATCH 2/4] Answer --help Without the Third-Party Modules a Script Needs redact_media.py and clean_content.py imported Pillow and PyYAML before parsing, so neither answered --help where its module was absent, and the checks/ leg of CI, which has neither, would have skipped both on every run. Each now parses first and reports the missing module after, so the CLI test runs every script with no skip. Refs #539 Co-Authored-By: Claude Opus 5.5 --- capture/clean_content.py | 5 ++++- checks/tests/test_cli_usage.py | 20 ++++------------- scripts/redact_media.py | 39 +++++++++++++++++++++++----------- 3 files changed, 35 insertions(+), 29 deletions(-) diff --git a/capture/clean_content.py b/capture/clean_content.py index bc454729..7e0939fc 100755 --- a/capture/clean_content.py +++ b/capture/clean_content.py @@ -38,10 +38,11 @@ import pathlib from pathlib import Path +# A missing PyYAML is reported once the arguments are parsed, so --help answers without it. try: import yaml except ModuleNotFoundError: # the only third-party import in capture/ or checks/ - sys.exit("PyYAML is required by this script: pip install PyYAML (Debian: apt install python3-yaml)") + yaml = None KEEP = ["title", "date", "url", "categories", "tags", "post_id", "cover"] FM = re.compile(r"^---\n(.*?)\n---\n(.*)$", re.S) @@ -128,4 +129,6 @@ def main(root: Path, apply: bool): parser.add_argument("site", nargs="?", help="the converted site, else the one generated-* under the capture") parser.add_argument("--apply", action="store_true", help="write the changes rather than report them") args = parser.parse_args() + if yaml is None: + sys.exit("PyYAML is required by this script: pip install PyYAML (Debian: apt install python3-yaml)") main(converted_site(args.site), args.apply) diff --git a/checks/tests/test_cli_usage.py b/checks/tests/test_cli_usage.py index d60c24c6..4a2c24f2 100644 --- a/checks/tests/test_cli_usage.py +++ b/checks/tests/test_cli_usage.py @@ -3,17 +3,16 @@ Each script runs with `--help`, with `-h`, and with an unknown `--bogus-flag`. Help exits 0 and prints the same text for both spellings, opening on a usage line that names the script. The unknown flag exits non-zero with a refusal naming it. A script needing a positional refuses on the missing -argument before it reaches the flag, so BEFORE_FLAG gives it one, and every other script is -declared in NO_REQUIRED_ARGUMENT. A script declared in neither fails, so a new one is classified -the moment it is tracked. +argument before it reaches the flag, so BEFORE_FLAG gives it one. Every other script is declared +in NO_REQUIRED_ARGUMENT. A script declared in neither fails, so a new one is classified the moment +it is tracked. -No run may change anything. Stubs ahead of the real commands on PATH record any call to them, the +No run may change anything. Stubs ahead of the real commands on PATH record any call to them. The home and working directories are empty scratch directories that must stay empty, and the tree's status must not move. ENV_FILE names a file that does not exist, so make-release.sh refuses rather than building should its argument check ever regress. """ -import importlib.util import os import pathlib import re @@ -54,12 +53,6 @@ } ) -# A third-party module a script imports before it parses, skipped where this interpreter lacks it. -NEEDS_MODULE = { - "capture/clean_content.py": "yaml", - "scripts/redact_media.py": "PIL", -} - REFUSED_AS_OPTION = re.compile( r"(unknown|unrecogni[sz]ed) (option|argument)[^\n]*--bogus-flag", re.IGNORECASE ) @@ -168,11 +161,6 @@ def test_every_entry_point_is_declared(self) -> None: def test_help_answers_and_unknown_flag_is_refused(self) -> None: for path in entry_points(): with self.subTest(path=path): - module = NEEDS_MODULE.get(path) - if module and importlib.util.find_spec(module) is None: - self.skipTest( - f"{path} needs {module}, which this interpreter lacks" - ) self.fresh(path) before = tree_status() long, short = ( diff --git a/scripts/redact_media.py b/scripts/redact_media.py index 3f9706fb..fb61a96a 100755 --- a/scripts/redact_media.py +++ b/scripts/redact_media.py @@ -55,8 +55,32 @@ import subprocess import sys -import PIL -from PIL import Image, ImageDraw, JpegImagePlugin + +def arguments() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "--apply", action="store_true", help="write the files, rather than reporting" + ) + parser.add_argument( + "--record", + action="store_true", + help="for each entry whose declared digest is missing or stale, take the file, which must be the original its source hash records where the entry has one, as the source and record the result", + ) + return parser + + +try: + import PIL + from PIL import Image, ImageDraw, JpegImagePlugin +except ModuleNotFoundError as error: + # Run directly without Pillow, --help still answers and anything else is told how to run this. + if __name__ != "__main__": + raise + arguments().parse_args() + sys.exit( + f"{error.name} is not installed, so run this with: uv run --no-project" + " --with-requirements scripts/requirements.txt scripts/redact_media.py" + ) REPO = pathlib.Path(__file__).resolve().parent.parent MANIFEST = REPO / "checks" / "media-redactions.json" @@ -214,16 +238,7 @@ def recorded_results() -> set[str]: def main() -> int: - parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) - parser.add_argument( - "--apply", action="store_true", help="write the files, rather than reporting" - ) - parser.add_argument( - "--record", - action="store_true", - help="for each entry whose declared digest is missing or stale, take the file, which must be the original its source hash records where the entry has one, as the source and record the result", - ) - args = parser.parse_args() + args = arguments().parse_args() apply = args.apply or args.record # The output is reproducible only under the pinned Pillow, which running this file directly does not install. From 851dd6b7b5f05a24e1bce016dba1ab68d14018b6 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 25 Oct 2026 20:15:35 -0700 Subject: [PATCH 3/4] Name redact_media.py's uv Invocation Once Both refusals, Pillow missing and Pillow at the wrong pin, now print the same RUN constant rather than two copies of the command. Refs #539 Co-Authored-By: Claude Opus 5.5 --- scripts/redact_media.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/scripts/redact_media.py b/scripts/redact_media.py index fb61a96a..acd8620b 100755 --- a/scripts/redact_media.py +++ b/scripts/redact_media.py @@ -55,6 +55,9 @@ import subprocess import sys +# The one invocation that installs the pinned Pillow, named by both refusals below. +RUN = "uv run --no-project --with-requirements scripts/requirements.txt scripts/redact_media.py" + def arguments() -> argparse.ArgumentParser: parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) @@ -77,10 +80,7 @@ def arguments() -> argparse.ArgumentParser: if __name__ != "__main__": raise arguments().parse_args() - sys.exit( - f"{error.name} is not installed, so run this with: uv run --no-project" - " --with-requirements scripts/requirements.txt scripts/redact_media.py" - ) + sys.exit(f"{error.name} is not installed, so run this with: {RUN}") REPO = pathlib.Path(__file__).resolve().parent.parent MANIFEST = REPO / "checks" / "media-redactions.json" @@ -245,9 +245,7 @@ def main() -> int: pins = {line.strip().lower() for line in REQUIREMENTS.read_text().splitlines()} if f"pillow=={PIL.__version__}" not in pins: print(f"Pillow {PIL.__version__} is not the version {REQUIREMENTS.name} pins.") - print( - "Run with `uv run --no-project --with-requirements scripts/requirements.txt scripts/redact_media.py`." - ) + print(f"Run with `{RUN}`.") return 1 manifest = json.loads(MANIFEST.read_text()) From e0c3d911246d8cb06d98aa16fa49860e5452e45c Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 25 Oct 2026 20:27:21 -0700 Subject: [PATCH 4/4] Answer --help After Another Argument in the Shell Scripts check-live-urls.sh counted its arguments before it looked for --help, so `extra --help` was refused as an unexpected argument, and run-wp2hugo.sh looked for --help only in the first position. Both now answer help wherever it appears, as make-release.sh and every argparse script already did, and the CLI test runs help after another argument. Refs #539 Co-Authored-By: Claude Opus 5.5 --- capture/run-wp2hugo.sh | 16 +++++++++------- checks/check-live-urls.sh | 13 +++++++++---- checks/tests/test_cli_usage.py | 6 +++++- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/capture/run-wp2hugo.sh b/capture/run-wp2hugo.sh index 0d58e6cd..cc4878c8 100755 --- a/capture/run-wp2hugo.sh +++ b/capture/run-wp2hugo.sh @@ -9,13 +9,15 @@ set -Eeuo pipefail # Arguments are checked before CAPTURE_ROOT is read, so --help answers where none is set. -case "${1:-}" in --h | --help) - echo "usage: $0" - echo " converts the export build_redirects.py selects under CAPTURE_ROOT" - exit 0 - ;; -esac +for arg in "$@"; do + case "$arg" in + -h | --help) + echo "usage: $0" + echo " converts the export build_redirects.py selects under CAPTURE_ROOT" + exit 0 + ;; + esac +done if [ "$#" -gt 0 ]; then echo "unknown argument: $1" >&2 echo "usage: $0" >&2 diff --git a/checks/check-live-urls.sh b/checks/check-live-urls.sh index 7732198d..b16dd646 100755 --- a/checks/check-live-urls.sh +++ b/checks/check-live-urls.sh @@ -9,6 +9,15 @@ usage() { echo "usage: $0 | --print-sample" } +# Help is answered wherever it appears, before the count is checked. +for arg in "$@"; do + case "$arg" in + -h | --help) + usage + exit 0 + ;; + esac +done PRINT_SAMPLE='' BASE="${1:-}" if [ "$#" -gt 1 ]; then @@ -17,10 +26,6 @@ if [ "$#" -gt 1 ]; then exit 2 fi case "$BASE" in --h | --help) - usage - exit 0 - ;; --print-sample) PRINT_SAMPLE=1 BASE='' diff --git a/checks/tests/test_cli_usage.py b/checks/tests/test_cli_usage.py index 4a2c24f2..c3032ca8 100644 --- a/checks/tests/test_cli_usage.py +++ b/checks/tests/test_cli_usage.py @@ -1,7 +1,8 @@ """Hold every tracked script to the fleet's command-line argument rule by running it. Each script runs with `--help`, with `-h`, and with an unknown `--bogus-flag`. Help exits 0 and -prints the same text for both spellings, opening on a usage line that names the script. The unknown +prints the same text for both spellings, opening on a usage line that names the script. Help after +another argument answers the same way, rather than being refused as an extra one. The unknown flag exits non-zero with a refusal naming it. A script needing a positional refuses on the missing argument before it reaches the flag, so BEFORE_FLAG gives it one. Every other script is declared in NO_REQUIRED_ARGUMENT. A script declared in neither fails, so a new one is classified the moment @@ -170,6 +171,9 @@ def test_help_answers_and_unknown_flag_is_refused(self) -> None: self.assertEqual(0, long.returncode, long.stderr) self.assertEqual(0, short.returncode, short.stderr) self.assertEqual(long.stdout, short.stdout) + late = self.run_script(path, "extra", "--help") + self.assertEqual(0, late.returncode, late.stderr) + self.assertEqual(long.stdout, late.stdout) self.assertRegex( long.stdout.splitlines()[0] if long.stdout else "", rf"^usage: \S*{re.escape(pathlib.PurePosixPath(path).name)}\b",