diff --git a/scripts/lint/mermaid.sh b/scripts/lint/mermaid.sh index 5dd9c0fefd..1f1a606485 100755 --- a/scripts/lint/mermaid.sh +++ b/scripts/lint/mermaid.sh @@ -20,20 +20,6 @@ if ! command -v mmdc >/dev/null 2>&1; then exit 127 fi -# Limitation: install-lint skips the puppeteer browser download so it stays -# sandbox-safe; mermaid-cli (puppeteer-core) bundles none, so provision the -# matching chrome-headless-shell here. --install-deps also pulls chrome's shared -# libs (libnspr4, libnss3, ...) on root Linux. Idempotent; needs network + unzip. -if command -v apt-get >/dev/null 2>&1; then - apt-get -o DPkg::Lock::Timeout=600 update >/dev/null 2>&1 || - echo "Warning: apt-get update failed; chrome's shared libs may stay unresolved." >&2 -fi - -if ! deps_output="$(npx --yes puppeteer browsers install chrome-headless-shell --install-deps 2>&1)"; then - printf 'Warning: chrome-headless-shell provisioning failed; mermaid rendering may fail:\n%s\n' \ - "${deps_output}" >&2 -fi - workdir="$(mktemp -d)" trap 'rm -rf "${workdir}"' EXIT @@ -47,6 +33,15 @@ if [[ ${#md_files[@]} -eq 0 ]]; then exit 0 fi +printf '%s\n' 'flowchart LR' ' a --> b' >"${workdir}/preflight.mmd" +if ! mmdc --puppeteerConfigFile "${puppeteer_cfg}" -i "${workdir}/preflight.mmd" \ + -o "${workdir}/preflight.svg" >"${workdir}/preflight.log" 2>&1; then + printf "mmdc cannot render a minimal diagram, so none of the %d Markdown file(s) was checked. 'make install-lint-force' provisions its browser:\n" \ + "${#md_files[@]}" >&2 + sed 's/^/ /' "${workdir}/preflight.log" >&2 + exit 1 +fi + : "${INFINITO_WORKER_CPU:?INFINITO_WORKER_CPU must be set (provided by default.env via the env loader)}" jobs="${INFINITO_WORKER_CPU}" diff --git a/tests/unit/python/scripts/lint/__init__.py b/tests/unit/python/scripts/lint/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit/python/scripts/lint/test_mermaid.py b/tests/unit/python/scripts/lint/test_mermaid.py new file mode 100644 index 0000000000..30a7c3e1cd --- /dev/null +++ b/tests/unit/python/scripts/lint/test_mermaid.py @@ -0,0 +1,62 @@ +"""``scripts/lint/mermaid.sh`` renders with the browser install-lint provisioned. + +``make lint`` runs its targets in parallel, so the script must not reach for a +package manager, and a browser that cannot start must fail the lint once with +its reason instead of once per Markdown file. +""" + +from __future__ import annotations + +import os +import subprocess +import tempfile +import unittest +from pathlib import Path + +from utils.cache.files import PROJECT_ROOT + +SCRIPT = Path(PROJECT_ROOT) / "scripts" / "lint" / "mermaid.sh" +LIBRARY_ERROR = ( + "chrome-headless-shell: error while loading shared libraries: libnspr4.so" +) +RECORDING_STUB = '#!/bin/sh\necho "$(basename "$0")" >> "$STUB_CALLS"\n' + + +class TestMermaidLint(unittest.TestCase): + def test_a_browser_that_cannot_start_fails_before_any_file_renders(self): + with tempfile.TemporaryDirectory() as tmp: + bin_dir = Path(tmp) + calls = bin_dir / "calls" + calls.touch() + for name in ("npx", "apt-get"): + (bin_dir / name).write_text(RECORDING_STUB) + (bin_dir / "mmdc").write_text( + f"{RECORDING_STUB}echo '{LIBRARY_ERROR}' >&2\nexit 1\n" + ) + for stub in bin_dir.iterdir(): + stub.chmod(0o755) + env = { + k: v + for k, v in os.environ.items() + if not k.startswith("INFINITO_") and k != "BASH_ENV" + } + env["PATH"] = f"{bin_dir}{os.pathsep}{env['PATH']}" + env["STUB_CALLS"] = str(calls) + proc = subprocess.run( + ["bash", str(SCRIPT)], + env=env, + capture_output=True, + text=True, + check=False, + ) + recorded = calls.read_text().split() + + self.assertEqual(proc.returncode, 1, proc.stderr) + self.assertIn("cannot render a minimal diagram", proc.stderr) + self.assertIn(LIBRARY_ERROR, proc.stderr) + self.assertNotIn("❌", proc.stdout) + self.assertEqual(recorded, ["mmdc"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/python/utils/install/lint/test_main.py b/tests/unit/python/utils/install/lint/test_main.py index c044c3afa9..6cfd4a48d8 100644 --- a/tests/unit/python/utils/install/lint/test_main.py +++ b/tests/unit/python/utils/install/lint/test_main.py @@ -34,12 +34,30 @@ class TestStampLogic(unittest.TestCase): with ( mock.patch.object(cli, "PROJECT_ROOT", root), mock.patch.object(cli.shutil, "which", return_value="/usr/bin/x"), + mock.patch.object( + cli.mermaid_cli, "browser_installed", return_value=True + ), mock.patch.object(cli, "_install_all") as inst, ): rc = cli.main([]) self.assertEqual(rc, 0) inst.assert_not_called() + def test_missing_mermaid_browser_invalidates_stamp(self) -> None: + with TemporaryDirectory() as tmp: + root = _make_repo_root(tmp, stamp_age_offset=60.0) + with ( + mock.patch.object(cli, "PROJECT_ROOT", root), + mock.patch.object(cli.shutil, "which", return_value="/usr/bin/x"), + mock.patch.object( + cli.mermaid_cli, "browser_installed", return_value=False + ), + mock.patch.object(cli, "_install_all") as inst, + ): + rc = cli.main([]) + self.assertEqual(rc, 0) + inst.assert_called_once() + def test_missing_tool_invalidates_stamp(self) -> None: with TemporaryDirectory() as tmp: root = _make_repo_root(tmp, stamp_age_offset=60.0) diff --git a/tests/unit/python/utils/install/lint/test_mermaid_cli.py b/tests/unit/python/utils/install/lint/test_mermaid_cli.py new file mode 100644 index 0000000000..3c827abe0d --- /dev/null +++ b/tests/unit/python/utils/install/lint/test_mermaid_cli.py @@ -0,0 +1,111 @@ +"""Unit tests for :mod:`utils.install.lint.mermaid_cli`.""" + +from __future__ import annotations + +import os +import subprocess +import unittest +import unittest.mock as mock +from pathlib import Path +from tempfile import TemporaryDirectory + +from utils.install.lint import mermaid_cli + + +def _done(returncode: int) -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess([], returncode, stdout="out\n", stderr="err\n") + + +class TestBrowserProvisioning(unittest.TestCase): + def provision(self, *, euid: int, results: list[int]) -> tuple[list, list[str]]: + """Run ``ensure()`` with mmdc present and a stubbed puppeteer. + + Args: + euid: effective uid the run pretends to have. + results: exit codes the successive puppeteer calls return. + + Returns: + The recorded subprocess calls and the warnings emitted. + """ + warnings: list[str] = [] + with ( + TemporaryDirectory() as cache, + mock.patch.dict(os.environ, {"PUPPETEER_CACHE_DIR": cache}), + mock.patch.object(mermaid_cli, "which", return_value=True), + mock.patch.object(mermaid_cli.os, "geteuid", return_value=euid), + mock.patch.object(mermaid_cli, "warn", side_effect=warnings.append), + mock.patch.object( + mermaid_cli.subprocess, + "run", + side_effect=[_done(code) for code in results], + ) as run, + ): + mermaid_cli.ensure() + return run.call_args_list, warnings + + def test_root_installs_the_browser_with_its_libraries(self) -> None: + calls, warnings = self.provision(euid=0, results=[0]) + self.assertEqual(len(calls), 1) + self.assertEqual(calls[0].args[0][-2:], ["chrome-headless-shell", "--install-deps"]) + self.assertEqual(warnings, []) + + def test_a_user_installs_the_browser_without_libraries(self) -> None: + calls, _ = self.provision(euid=1000, results=[0]) + self.assertNotIn("--install-deps", calls[0].args[0]) + + def test_the_browser_install_downloads_despite_the_mmdc_skip_flag(self) -> None: + with mock.patch.dict(os.environ, {"PUPPETEER_SKIP_DOWNLOAD": "1"}): + calls, _ = self.provision(euid=0, results=[0]) + self.assertNotIn("PUPPETEER_SKIP_DOWNLOAD", calls[0].kwargs["env"]) + + def test_a_failed_install_clears_the_browser_cache_and_retries_once(self) -> None: + with TemporaryDirectory() as cache: + folder = Path(cache) / "chrome-headless-shell" / "linux-1" + folder.mkdir(parents=True) + seen: list[bool] = [] + + def run(*_args, **_kwargs): + seen.append(folder.exists()) + return _done(1 if len(seen) == 1 else 0) + + with ( + mock.patch.dict(os.environ, {"PUPPETEER_CACHE_DIR": cache}), + mock.patch.object(mermaid_cli, "which", return_value=True), + mock.patch.object(mermaid_cli.subprocess, "run", side_effect=run), + ): + mermaid_cli.ensure() + self.assertEqual(seen, [True, False]) + + def test_a_second_failure_warns_with_puppeteers_output(self) -> None: + calls, warnings = self.provision(euid=0, results=[1, 1]) + self.assertEqual(len(calls), 2) + self.assertEqual(len(warnings), 1) + self.assertIn("out\nerr\n", warnings[0]) + + def test_a_timeout_warns_instead_of_raising(self) -> None: + with ( + mock.patch.object(mermaid_cli, "which", return_value=True), + mock.patch.object( + mermaid_cli.subprocess, + "run", + side_effect=subprocess.TimeoutExpired("npx", 900), + ), + mock.patch.object(mermaid_cli, "warn") as warn, + ): + mermaid_cli.ensure() + warn.assert_called_once() + + +class TestBrowserInstalled(unittest.TestCase): + def test_the_executable_in_the_puppeteer_cache_counts(self) -> None: + with TemporaryDirectory() as cache: + build = Path(cache) / "chrome-headless-shell" / "linux-1" / "linux64" + build.mkdir(parents=True) + with mock.patch.dict(os.environ, {"PUPPETEER_CACHE_DIR": cache}): + self.assertFalse(mermaid_cli.browser_installed()) + (build / "chrome-headless-shell").write_text("", encoding="utf-8") + self.assertTrue(mermaid_cli.browser_installed()) + + +if __name__ == "__main__": + unittest.main() diff --git a/utils/install/lint/__main__.py b/utils/install/lint/__main__.py index 11fb1fc56a..7bf56b6079 100644 --- a/utils/install/lint/__main__.py +++ b/utils/install/lint/__main__.py @@ -177,7 +177,10 @@ def _stamp_is_fresh(repo_root: Path) -> bool: return False if dep_path.stat().st_mtime > stamp_mtime: return False - return all(shutil.which(tool) is not None for tool in _STAMP_TOOLS) + return ( + all(shutil.which(tool) is not None for tool in _STAMP_TOOLS) + and mermaid_cli.browser_installed() + ) def _touch_stamp(repo_root: Path) -> None: diff --git a/utils/install/lint/mermaid_cli.py b/utils/install/lint/mermaid_cli.py index 099af21a4d..e84c788327 100644 --- a/utils/install/lint/mermaid_cli.py +++ b/utils/install/lint/mermaid_cli.py @@ -1,24 +1,28 @@ -"""Install @mermaid-js/mermaid-cli (mmdc) via npm global. +"""Install @mermaid-js/mermaid-cli (mmdc) and the chrome-headless-shell it renders with. -Best-effort: the puppeteer browser is not downloaded here -(``PUPPETEER_SKIP_DOWNLOAD``) so ``install-lint`` stays sandbox-safe. mmdc only -needs a browser to RENDER, which ``scripts/lint/mermaid.sh`` provisions on demand -(``npx puppeteer browsers install chrome-headless-shell``) right before it -renders. That browser ships as a zip, so ``unzip`` is ensured here (puppeteer's -extractor needs it, else it leaves an empty version dir and mmdc reports the -browser missing). A failed install warns instead of aborting ``install-lint``, -so sandboxes without npm/browser access still get the rest of the lint toolchain. +mmdc comes from npm with ``PUPPETEER_SKIP_DOWNLOAD`` set; the browser is +provisioned through puppeteer afterwards, as root with its shared libraries +(``--install-deps``). ``unzip`` is ensured first because puppeteer extracts the +browser with it. A failed install warns instead of aborting ``install-lint``, so +sandboxes without npm or browser access still get the rest of the lint +toolchain. """ from __future__ import annotations import contextlib import os +import shutil +import subprocess +from pathlib import Path from utils.install.npm import npm_install_global from utils.install.primitives import log, warn, which from utils.install.system_pkg import install_command_via_pkg +BROWSER = "chrome-headless-shell" +PROVISION_TIMEOUT_S = 900 + def _ensure_unzip() -> None: if which("unzip"): @@ -27,16 +31,58 @@ def _ensure_unzip() -> None: install_command_via_pkg("unzip") +def browser_cache() -> Path: + base = os.environ.get("PUPPETEER_CACHE_DIR") or Path.home() / ".cache" / "puppeteer" + return Path(base) / BROWSER + + +def browser_installed() -> bool: + return any(path.is_file() for path in browser_cache().glob(f"*/*/{BROWSER}")) + + +def _provision_browser() -> subprocess.CompletedProcess[str]: + argv = ["npx", "--yes", "puppeteer", "browsers", "install", BROWSER] + if os.geteuid() == 0: + argv.append("--install-deps") + env = {k: v for k, v in os.environ.items() if k != "PUPPETEER_SKIP_DOWNLOAD"} + return subprocess.run( + argv, + env=env, + capture_output=True, + text=True, + timeout=PROVISION_TIMEOUT_S, + check=False, + ) + + +def _ensure_browser() -> None: + log(f"Provisioning {BROWSER} for mmdc via puppeteer.") + try: + done = _provision_browser() + if done.returncode != 0: + shutil.rmtree(browser_cache(), ignore_errors=True) + done = _provision_browser() + except (OSError, subprocess.TimeoutExpired) as exc: + warn(f"{BROWSER} provisioning skipped (optional lint tool): {exc}") + return + if done.returncode != 0: + warn( + f"{BROWSER} provisioning failed (optional lint tool):\n" + f"{done.stdout}{done.stderr}" + ) + + def ensure() -> None: _ensure_unzip() - if which("mmdc"): - return - log("Missing command 'mmdc'. Installing @mermaid-js/mermaid-cli via npm.") - os.environ["PUPPETEER_SKIP_DOWNLOAD"] = "1" - try: - npm_install_global("@mermaid-js/mermaid-cli") - except Exception as exc: # noqa: BLE001 - best-effort optional lint tool - warn(f"mermaid-cli install skipped (optional lint tool): {exc}") - return if not which("mmdc"): - warn("Command 'mmdc' is still unavailable after installation.") + log("Missing command 'mmdc'. Installing @mermaid-js/mermaid-cli via npm.") + os.environ["PUPPETEER_SKIP_DOWNLOAD"] = "1" + try: + npm_install_global("@mermaid-js/mermaid-cli") + except Exception as exc: # noqa: BLE001 - best-effort optional lint tool + warn(f"mermaid-cli install skipped (optional lint tool): {exc}") + return + if not which("mmdc"): + warn("Command 'mmdc' is still unavailable after installation.") + return + _ensure_browser()