mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-06 02:54:26 +00:00
fix(lint): provision the mermaid browser in install-lint, not in the parallel lint run
make lint runs install-lint once and then dispatches its fifteen targets in parallel. scripts/lint/mermaid.sh provisioned chrome-headless-shell inside that parallel phase with `npx puppeteer browsers install --install-deps`, whose `apt-get satisfy` carries no lock timeout. In a fresh stack container another target held the apt lock, the shared libraries stayed missing, the script only warned, and all 289 Markdown files then failed on libnspr4.so (#795). utils/install/lint/mermaid_cli.py now provisions the browser right after mmdc, inside the serial install-lint step, as root with --install-deps and as a user without it, because puppeteer refuses to install libraries without root. A failed install deletes the chrome-headless-shell cache folder and retries once under a 900 second timeout per attempt; a cut-off download otherwise leaves a folder without its executable, and puppeteer refuses every later install of that build. The install-lint stamp counts as fresh only while the browser executable exists, so a recreated container provisions it again. mermaid.sh no longer calls apt-get or npx. It renders one minimal diagram first and, when that fails, exits with mmdc's output instead of reporting every Markdown file. Verified: tests/unit/python/scripts/lint/test_mermaid.py stubs mmdc, npx and apt-get. With a browser that cannot load libnspr4.so the script exits 1 with that line, before any Markdown file renders, and mmdc is the only tool it calls. Against main's mermaid.sh the same test fails: it calls apt-get and npx and reports the files one by one. The installer and stamp cases pass, shellcheck is clean. Not verified: a full make lint in a recreated stack container. The race reproduced in one of two runs in the issue. feature/store-i18n changed the provisioning in mermaid.sh with2f5f52bd1aanddc67bce5f0(timeout, deleting a cut-off browser). Both behaviours moved into mermaid_cli.py here, so its merge takes this version of mermaid.sh. Closes #795 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
789c1a1b63
commit
c8cb7fda8c
7 files changed
+269
-34
No files matched your search
+9
-14
@@ -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}"
|
||||
|
||||
|
||||
Whitespace-only changes.
@@ -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()
|
||||
@@ -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)
|
||||
|
||||
@@ -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()
|
||||
@@ -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:
|
||||
|
||||
@@ -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()
|
||||
Reference in new issue
Block a user