This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6064-6065705c16340c0be293212a71decfd9df4daae4 in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
commit 9ea4df346ccc3a4b5b44447e86840947a185fb55 Author: Andy Grove <[email protected]> AuthorDate: Mon Sep 21 02:24:11 2026 +0000 fix: restore the site's mermaid diagrams and make a dropped one fail CI (#6064) * fix: restore the site's mermaid diagrams and make a dropped one fail CI All three mermaid diagrams are absent from the published pages. #6021 moved rendering from the reader's browser to build-time SVG, which fixed #6020's CSP block, and mmdc has produced nothing since: the publish commit for that change removed the raw blocks and added no <object> and no _images/mermaid-*.svg. sphinxcontrib-mermaid downgrades a render failure to a warning and drops the node, so the deploy stays green and the page publishes with a hole in it. docs/README.md and the workflow comment both already said this was the failure mode; nothing enforced it, and the docs job runs only on push to main, so #6021 could not have been caught before merge either. Two candidate causes, both closed out here rather than guessed between: Chrome's setuid sandbox cannot start under the AppArmor policy Ubuntu ships from 23.10 onwards, and puppeteer's postinstall catches its own browser download failure and exits 0, which leaves a green install step with no browser at all. A puppeteer config with --no-sandbox covers the first; an explicit browsers install, allowed to fail the job, covers the second. The durable half is dev/ci/check-mermaid.py. It renders every fence with the arguments conf.py gives the build, so preflight fails on the pull request rather than after the merge, and --built asserts the built site carries a non-empty SVG per fence with a page referencing it, before the publish step runs. Checked against the currently published HTML, where it reports 3 fences and 0 SVGs. * ci: put mmdc's stderr in the annotation, not just a summary The first run of this check on a runner reproduced the failure -- all three diagrams -- but the annotation said only 'mmdc cannot render this diagram', and a job log is not always reachable. The annotation is the part of a failing job that always is, so it carries the whole error now. * ci: give preflight's runner Chrome's shared libraries The annotation from the previous run named the cause: the browser downloads onto ubuntu-slim and then cannot start, because the slim image carries none of Chrome's shared libraries. Install the ones chrome-headless-shell links against. --- .github/workflows/ci.yml | 27 +++++ .github/workflows/docs.yaml | 26 +++- dev/ci/check-ci-config.py | 13 ++ dev/ci/check-mermaid.py | 282 ++++++++++++++++++++++++++++++++++++++++++++ dev/ci/compute-changes.py | 4 + docs/README.md | 18 ++- docs/puppeteer-config.json | 3 + docs/source/conf.py | 22 +++- pom.xml | 2 + 9 files changed, 391 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2855633c8d..e4415959bd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -135,6 +135,33 @@ jobs: - name: Check markdown formatting run: prettier --check "**/*.md" + # The site draws its ```mermaid fences with mmdc at build time, and + # sphinxcontrib-mermaid turns any render failure into a warning and a + # dropped diagram, so the deploy stays green and the page publishes with + # a hole in it (issue #6062). The docs job runs only on push to main, so + # rendering the fences here is the only chance to catch that before it + # publishes. Kept identical to the same pair of steps in docs.yaml. + - name: Install mermaid-cli + run: | + set -eux + # ubuntu-slim carries none of Chrome's shared libraries, so the browser downloads + # fine and then dies with `libatk-1.0.so.0: cannot open shared object file`. These + # are what chrome-headless-shell actually links against; libgtk-3 pulls in most of + # the rest (atk, cairo, pango, gdk-pixbuf) as dependencies. + sudo apt-get update -qq + sudo apt-get install -y -qq --no-install-recommends \ + libgtk-3-0t64 libnss3 libasound2t64 libgbm1 libatk-bridge2.0-0t64 \ + libcups2t64 libxkbcommon0 libxdamage1 libxcomposite1 libxrandr2 libxfixes3 + npm install --prefix "$RUNNER_TEMP/mermaid" "$(python3 dev/ci/check-mermaid.py --cli-spec)" + # puppeteer's postinstall fetches the browser mmdc drives, but it catches its own + # download failures and exits 0, so a flaky fetch leaves a green install step and an + # mmdc that cannot launch. Fetch it again here, where a failure fails the job. + npm --prefix "$RUNNER_TEMP/mermaid" exec -- puppeteer browsers install chrome-headless-shell + echo "$RUNNER_TEMP/mermaid/node_modules/.bin" >> "$GITHUB_PATH" + + - name: Check mermaid diagrams render + run: python3 dev/ci/check-mermaid.py + - name: Check missing suites run: python3 dev/ci/check-suites.py diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml index 645f75e58d..991a86397c 100644 --- a/.github/workflows/docs.yaml +++ b/.github/workflows/docs.yaml @@ -55,10 +55,26 @@ jobs: node-version: '24' - name: Install mermaid-cli - # Draws the ```mermaid fences into SVG at build time. Without mmdc on PATH the build + # Draws the ```mermaid fences into SVG at build time. Without a working mmdc the build # still succeeds but silently drops every diagram, so it is installed unconditionally. # See mermaid_output_format in docs/source/conf.py for why the browser cannot draw them. - run: npm install -g @mermaid-js/[email protected] + # Kept identical to the same step in ci.yml's preflight; the version comes from the + # script so the pull-request check and this deploy cannot render with different ones. + run: | + set -eux + npm install --prefix "$RUNNER_TEMP/mermaid" "$(python3 dev/ci/check-mermaid.py --cli-spec)" + # mmdc draws each diagram by driving headless Chrome. puppeteer's postinstall fetches + # that browser, but it catches its own download failures and exits 0, so a flaky fetch + # leaves a green install step and an mmdc that cannot launch. Fetch it again here, + # where a failure is allowed to fail the job. A cache hit makes this a no-op. + npm --prefix "$RUNNER_TEMP/mermaid" exec -- puppeteer browsers install chrome-headless-shell + echo "$RUNNER_TEMP/mermaid/node_modules/.bin" >> "$GITHUB_PATH" + + - name: Check mermaid diagrams render + # Preflight ran this on the pull request, but it is the cheap half of the guard and this + # is the run that publishes. Failing here leaves the previous site in place rather than + # replacing a page with a diagram-shaped hole. + run: python3 dev/ci/check-mermaid.py - name: Install dependencies run: | @@ -74,6 +90,12 @@ jobs: cd docs ./build.sh + - name: Check the diagrams reached the pages + # The other half: mmdc can be healthy and the diagram still not reach the HTML, which is + # what sphinxcontrib-mermaid does with any render error it meets -- a warning, a dropped + # node, and a green build. This runs before the publish step for that reason. + run: python3 dev/ci/check-mermaid.py --built docs/build/html + - name: Copy & push the generated HTML run: | set -x diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index 2742a3b687..55edd5f5e2 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -133,6 +133,19 @@ ROUTING_CASES = [ # Spot checks that the additions above did not widen unrelated routes. (["docs/source/user-guide/overview.md"], {"docs"}), (["native/core/benches/parquet_read.rs"], {"benchmark"}), + # The mermaid guard is run by preflight, which is unconditional, and again + # by the docs deploy, which is not, so the deploy has to be routed. The + # build jobs come along because `dev/ci/**` already feeds them. + ( + ["dev/ci/check-mermaid.py"], + { + "docs", + "build_linux", + "build_linux_full", + "build_linux_all_profiles", + "build_macos", + }, + ), # The Delta gate script is read by nothing else; the contrib crate feeds # only the gate. The PyArrow pytest lives under spark/, so the Linux and # macOS builds see it too, but no Spark SQL or Iceberg suite does, and diff --git a/dev/ci/check-mermaid.py b/dev/ci/check-mermaid.py new file mode 100644 index 0000000000..50789d0b5d --- /dev/null +++ b/dev/ci/check-mermaid.py @@ -0,0 +1,282 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Guards the two halves of the site's ```mermaid pipeline, both of them silent. + +The site draws its diagrams at build time -- `mermaid_output_format = 'svg'` in +docs/source/conf.py, because the ASF Content-Security-Policy blocks the +client-side renderer (issue #6020). When mmdc is missing, cannot launch Chrome, +or chokes on a diagram, sphinxcontrib-mermaid logs a warning and drops that +diagram. The build stays green, the deploy goes ahead, and the only symptom is +a published page with a diagram-shaped hole in it, which nobody sees until they +open the page. + +That is not hypothetical: it is how all three of the site's diagrams came to be +missing between #6021, which introduced build-time rendering, and the change +that added this check. See issue #6062. + +Two modes, because there are two ways to lose a diagram: + + python3 dev/ci/check-mermaid.py + Render every fence under docs/source/ with mmdc, using the arguments + docs/source/conf.py gives the build. Catches a diagram mmdc rejects and + an mmdc that cannot run on this runner at all. Cheap enough for + preflight, so it fails on the pull request rather than after the merge. + Needs mmdc on PATH (`npm install -g @mermaid-js/mermaid-cli`). + + python3 dev/ci/check-mermaid.py --built docs/build/html + Assert the built site actually carries an SVG for every fence, and that + each one is referenced by a page. Catches the other half: a diagram that + renders fine but never reaches the HTML. Run before publishing. + +The pinned mermaid-cli version lives here too, and `--cli-spec` prints it, so +that preflight and the docs deploy install the same one from one place. Two +workflows pinning it separately is the drift that lets the pull-request check +pass while the deploy drops a diagram. + +Run from the repository root. +""" + +import argparse +import importlib.util +import re +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +DOCS_SOURCE = REPO_ROOT / "docs" / "source" +CONF_PY = DOCS_SOURCE / "conf.py" + +# The one place the mermaid-cli version is pinned; both workflows install +# `--cli-spec` rather than repeating it. See the module docstring. +MERMAID_CLI_PACKAGE = "@mermaid-js/mermaid-cli" +MERMAID_CLI_VERSION = "11.17.0" + +# An opening ```mermaid fence through to the closing fence at the same indent. +# Matches what myst_fence_as_directive hands to sphinxcontrib-mermaid. +FENCE = re.compile( + r"^(?P<indent>[ \t]*)```mermaid[ \t]*$(?P<body>.*?)^(?P=indent)```[ \t]*$", re.M | re.S +) + +# sphinxcontrib-mermaid names each output after a hash of the diagram source, +# so identical diagrams collapse onto one file. The check below compares +# distinct sources against distinct files for that reason. +BUILT_SVG = "mermaid-*.svg" + + +def load_conf(): + """Import docs/source/conf.py for the mmdc arguments the real build uses. + + Reading them rather than repeating them is the point: a check that renders + with different flags than the build can pass while the build drops the + diagram. conf.py is plain assignments plus function definitions, and its one + module-level call is exception-safe, so importing it is side-effect free. + """ + spec = importlib.util.spec_from_file_location("comet_docs_conf", CONF_PY) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def source_files(): + return sorted(DOCS_SOURCE.rglob("*.md")) + sorted(DOCS_SOURCE.rglob("*.rst")) + + +def fences(path): + """Yield (line number, diagram source) for each mermaid fence in `path`.""" + text = path.read_text(encoding="utf-8") + for match in FENCE.finditer(text): + yield text.count("\n", 0, match.start()) + 1, match.group("body") + + +def all_fences(): + """Yield (path, line number, diagram source) across the whole doc source.""" + for path in source_files(): + for line, code in fences(path): + yield path, line, code + + +def render(code, params, workdir): + """Render one diagram; return None on success or the failure text.""" + source = workdir / "diagram.mmd" + output = workdir / "diagram.svg" + source.write_text(code, encoding="utf-8") + if output.exists(): + output.unlink() + + result = subprocess.run( + ["mmdc", *params, "-i", str(source), "-o", str(output)], + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + return annotate( + (result.stderr or result.stdout or "").strip() or f"mmdc exited {result.returncode}" + ) + # mmdc has been seen to exit 0 having written nothing, which the build would + # turn into an <object> pointing at an empty file. + if not output.exists() or output.stat().st_size == 0: + return "mmdc exited 0 but wrote no SVG" + return None + + +def escape_annotation(message): + """Encode `message` for a GitHub workflow command. + + Annotations are the only part of a failing job that is readable without + fetching the log, so the whole mmdc error goes in one rather than just a + summary. Newlines and `%` have to be escaped or the command is truncated at + the first one. The limit is generous but not unbounded, hence the trim. + """ + trimmed = message.strip()[:3000] + return trimmed.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + + +def annotate(error): + """Append the fix for failures whose message does not suggest one.""" + if "Could not find" in error and "cache path" in error: + return ( + f"{error}\n" + f"No browser is installed for mmdc to drive. `npm install -g {MERMAID_CLI_PACKAGE}` " + f"is supposed to fetch one through puppeteer's postinstall, but that script catches " + f"its own download failures and exits 0, so the install step goes green without a " + f"browser. Run `npx puppeteer browsers install chrome-headless-shell` from the " + f"mermaid-cli install directory." + ) + return error + + +def check_renders(): + if shutil.which("mmdc") is None: + print( + "::error::mmdc is not on PATH. The docs build needs it to draw the " + "```mermaid fences; without it the build still succeeds and silently " + "publishes those pages with the diagrams missing. Install it with " + f"`npm install -g {MERMAID_CLI_PACKAGE}` (see docs/README.md)." + ) + return False + + # CI installs `--cli-spec`, so a mismatch only happens locally; say so + # rather than failing, since the local build is not what publishes. + installed = subprocess.run( + ["mmdc", "--version"], capture_output=True, text=True, check=False + ).stdout.strip() + if installed and installed != MERMAID_CLI_VERSION: + print( + f"mermaid: note, mmdc {installed} is on PATH but CI renders with " + f"{MERMAID_CLI_VERSION} (pinned in {Path(__file__).name})" + ) + + conf = load_conf() + if conf.mermaid_output_format == "raw": + print( + "::error::docs/source/conf.py sets mermaid_output_format = 'raw', which " + "renders in the reader's browser. The ASF Content-Security-Policy blocks " + "that script, so no diagram reaches a reader (issue #6020)." + ) + return False + params = list(conf.mermaid_params) + + failed = 0 + checked = 0 + with tempfile.TemporaryDirectory() as tmp: + workdir = Path(tmp) + for path, line, code in all_fences(): + checked += 1 + error = render(code, params, workdir) + if error: + failed += 1 + relative = path.relative_to(REPO_ROOT) + print(f"::error file={relative},line={line}::{escape_annotation(error)}") + print(f"mermaid: {relative}:{line} does not render:\n{error}\n") + + if failed: + print(f"mermaid: {failed} of {checked} diagrams failed to render") + return False + if not checked: + print("mermaid: no ```mermaid fences found under docs/source (FENCE no longer matches?)") + return False + print(f"mermaid: {checked} diagrams render") + return True + + +def check_built(built): + """The built site carries a non-empty SVG per distinct fence, each referenced.""" + failures = [] + if not built.is_dir(): + print(f"::error::{built} is not a directory; nothing was built there") + return False + + expected = {code for _, _, code in all_fences()} + svgs = sorted(built.rglob(BUILT_SVG)) + empty = [svg for svg in svgs if svg.stat().st_size == 0] + if empty: + failures.append( + f"{len(empty)} rendered diagram(s) are empty files: " + f"{', '.join(str(svg.relative_to(built)) for svg in empty)}" + ) + if len(svgs) < len(expected): + failures.append( + f"docs/source has {len(expected)} distinct ```mermaid fences but the " + f"build produced {len(svgs)} SVG(s) under {built}. sphinxcontrib-mermaid " + f"downgrades a render failure to a warning and drops the diagram, so the " + f"missing ones would publish as a hole in the page. Check the build log " + f"for mermaid warnings" + ) + + # A file nothing points at is as invisible as a missing one. + html = "\n".join(page.read_text(encoding="utf-8", errors="ignore") for page in built.rglob("*.html")) + unreferenced = [svg.name for svg in svgs if svg.name not in html] + if unreferenced: + failures.append( + f"rendered but referenced by no page: {', '.join(unreferenced)}" + ) + + for failure in failures: + print(f"::error::mermaid: {failure}") + if failures: + return False + print(f"mermaid: {len(svgs)} diagrams rendered into {built} and referenced") + return True + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--built", + type=Path, + help="check a built HTML tree (e.g. docs/build/html) instead of rendering the fences", + ) + parser.add_argument( + "--cli-spec", + action="store_true", + help="print the pinned mermaid-cli npm spec and exit, for `npm install -g`", + ) + args = parser.parse_args() + if args.cli_spec: + print(f"{MERMAID_CLI_PACKAGE}@{MERMAID_CLI_VERSION}") + return 0 + ok = check_built(args.built) if args.built else check_renders() + return 0 if ok else 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 98be40988c..ae29077db4 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -172,6 +172,10 @@ FILTERS = { ".asf.yaml", ".github/workflows/docs.yaml", "docs/**", + # The docs deploy renders and then verifies the site's mermaid diagrams with this + # script, so a change to it has to be exercised by a real build, not just by the + # preflight run that renders the fences. + "dev/ci/check-mermaid.py", # Generated docs (configs.md, per-version expression compatibility pages) are # built from these Scala sources by GenerateDocs, so changes to them must # republish the site even when no docs/ file is touched. diff --git a/docs/README.md b/docs/README.md index cc5e5ae1f2..cda2e56a6e 100644 --- a/docs/README.md +++ b/docs/README.md @@ -29,10 +29,26 @@ inside a Python virtualenv. - Python - `pip install -r requirements.txt` -- Node, and `npm install -g @mermaid-js/mermaid-cli` for the `mmdc` command +- Node, and `npm install -g "$(python3 ../dev/ci/check-mermaid.py --cli-spec)"` for the `mmdc` command `mmdc` draws the ` ```mermaid ` fences into SVG when the docs are built. Without it on `PATH` the build still succeeds, but logs a warning and leaves those diagrams out of the pages. +That is silent all the way to the published site, so two CI checks guard it, both of which you +can run yourself: + +```bash +python3 ../dev/ci/check-mermaid.py # every fence renders under mmdc +python3 ../dev/ci/check-mermaid.py --built build/html # every fence reached a page +``` + +`mmdc` draws each diagram by driving headless Chrome. Installing mermaid-cli is supposed to fetch +that browser through puppeteer's `postinstall`, but that script catches its own download failures +and exits 0, so `Could not find chrome-headless-shell` from the check above means the install went +green without one. Fetch it explicitly with `npx puppeteer browsers install chrome-headless-shell`. + +Chrome's sandbox also cannot start under the AppArmor policy Ubuntu ships from 23.10 onwards. +`puppeteer-config.json` turns it off, and `mermaid_params` in `source/conf.py` passes that config +to every render. ## Build & Preview diff --git a/docs/puppeteer-config.json b/docs/puppeteer-config.json new file mode 100644 index 0000000000..2274c80af9 --- /dev/null +++ b/docs/puppeteer-config.json @@ -0,0 +1,3 @@ +{ + "args": ["--no-sandbox", "--disable-setuid-sandbox"] +} diff --git a/docs/source/conf.py b/docs/source/conf.py index c1c85ea011..698eeba4cf 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -31,6 +31,12 @@ # import sys # sys.path.insert(0, os.path.abspath('.')) +from pathlib import Path + +# docs/, whether this file is read from docs/source/ (a direct sphinx-build) or +# from the docs/temp/ copy that docs/build.sh generates. +DOCS_DIR = Path(__file__).resolve().parent.parent + # -- Project information ----------------------------------------------------- project = 'Apache DataFusion Comet' @@ -79,9 +85,19 @@ myst_fence_as_directive = ['mermaid'] # build still succeeds but logs a warning and drops the diagrams. See docs/README.md. mermaid_output_format = 'svg' -# Render on a transparent background so one SVG suits both the light and dark site themes; -# mmdc otherwise bakes in a white background. -mermaid_params = ['-b', 'transparent'] +# -b transparent: render on a transparent background so one SVG suits both the light and dark +# site themes; mmdc otherwise bakes in a white background. +# +# -p puppeteer-config.json: mmdc draws each diagram by driving headless Chrome through puppeteer. +# Chrome's setuid sandbox needs unprivileged user namespaces, which Ubuntu restricts by AppArmor +# policy from 23.10 onwards, so on an ubuntu-24.04 runner Chrome can fail to launch at all. The +# config file turns that sandbox off; the CI container is already the isolation boundary. +# +# Any such failure is invisible without help: mmdc exits non-zero, sphinxcontrib-mermaid turns +# that into a warning and drops the diagram, and the build publishes a page with a hole in it. +# That is issue #6062, and dev/ci/check-mermaid.py is what makes it loud. It reads the arguments +# below rather than repeating them, so the check renders exactly as the build does. +mermaid_params = ['-b', 'transparent', '-p', str(DOCS_DIR / 'puppeteer-config.json')] # Add any paths that contain templates here, relative to this directory. templates_path = ['_templates'] diff --git a/pom.xml b/pom.xml index 6766979652..3288c69fa7 100644 --- a/pom.xml +++ b/pom.xml @@ -1156,6 +1156,8 @@ under the License. <exclude>tpcds-sf-1/**</exclude> <exclude>tpch/**</exclude> <exclude>docs/*.txt</exclude> + <!-- JSON has no comment syntax, so no license header can go in it. --> + <exclude>docs/puppeteer-config.json</exclude> <exclude>docs/logos/*.png</exclude> <exclude>docs/logos/*.svg</exclude> <exclude>docs/source/_static/images/**</exclude> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
