Harden the `.. d2::` directive render path
Three robustness fixes surfaced by the PR #460 self-review, - a render that's *attempted and fails* (a `d2` bin is present but errors on the source) now raises a docutils error instead of silently serving the stale committed SVG — so `sphinx-build -W` fails on a broken `.d2` edit. The no-binary case stays a quiet committed-SVG fallback. - render into a sibling temp-file then `os.replace()` so a failed/torn render can never clobber a good committed SVG. - guard against 2 distinct `.d2` sources colliding on a single `_diagrams/<stem>.svg` within a build. `render_svg()` now returns a `RenderResult` tristate that `run()` maps to error / raw-source-literal / image. (this patch was generated in some part by [`claude-code`][claude-code-gh]) [claude-code-gh]: https://github.com/anthropics/claude-code
parent
8661336cf6
commit
3a5cfde023
|
|
@ -16,6 +16,14 @@ Rendering policy,
|
||||||
emitted as a literal block so no content is ever
|
emitted as a literal block so no content is ever
|
||||||
silently dropped.
|
silently dropped.
|
||||||
|
|
||||||
|
A render that is *attempted and fails* (a ``d2`` bin is
|
||||||
|
present but errors on the source) is NOT silently
|
||||||
|
degraded to the stale committed SVG: it raises a
|
||||||
|
docutils error (so ``sphinx-build -W`` fails the
|
||||||
|
build). The render is also atomic — a failed/torn
|
||||||
|
render can never clobber a good committed SVG — so the
|
||||||
|
last-good diagram survives a bad edit.
|
||||||
|
|
||||||
Binary discovery order,
|
Binary discovery order,
|
||||||
|
|
||||||
- the ``D2_BIN`` env var; may contain args which are
|
- the ``D2_BIN`` env var; may contain args which are
|
||||||
|
|
@ -37,11 +45,13 @@ Usage,
|
||||||
'''
|
'''
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import enum
|
||||||
import os
|
import os
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
import shlex
|
import shlex
|
||||||
import shutil
|
import shutil
|
||||||
import subprocess as sp
|
import subprocess as sp
|
||||||
|
import tempfile
|
||||||
|
|
||||||
from docutils import nodes
|
from docutils import nodes
|
||||||
from docutils.parsers.rst import directives
|
from docutils.parsers.rst import directives
|
||||||
|
|
@ -55,6 +65,22 @@ log = logging.getLogger(__name__)
|
||||||
# git-committed, fallback SVG outputs.
|
# git-committed, fallback SVG outputs.
|
||||||
_outdir: str = '_diagrams'
|
_outdir: str = '_diagrams'
|
||||||
|
|
||||||
|
# per-build map of {output-svg-name: source-relpath} used
|
||||||
|
# to detect 2 distinct `.d2` sources colliding on a single
|
||||||
|
# output stem; reset on each `builder-inited` (see `setup`).
|
||||||
|
_seen_outputs: dict[str, str] = {}
|
||||||
|
|
||||||
|
|
||||||
|
class RenderResult(enum.Enum):
|
||||||
|
'''
|
||||||
|
Outcome of a `render_svg()` call.
|
||||||
|
|
||||||
|
'''
|
||||||
|
OK = 'ok' # fresh SVG exists on disk
|
||||||
|
FELL_BACK = 'fell_back' # no bin; serving committed SVG
|
||||||
|
NO_OUTPUT = 'no_output' # no bin AND no committed SVG
|
||||||
|
FAILED = 'failed' # bin present, render errored
|
||||||
|
|
||||||
|
|
||||||
def find_d2(
|
def find_d2(
|
||||||
app: Sphinx,
|
app: Sphinx,
|
||||||
|
|
@ -77,37 +103,54 @@ def render_svg(
|
||||||
app: Sphinx,
|
app: Sphinx,
|
||||||
src: Path,
|
src: Path,
|
||||||
out: Path,
|
out: Path,
|
||||||
) -> bool:
|
) -> RenderResult:
|
||||||
'''
|
'''
|
||||||
Maybe (re)render `src` -> `out`, returning
|
Maybe (re)render `src` -> `out` and report the
|
||||||
`True` when an up-to-date SVG exists after the
|
outcome as a `RenderResult`.
|
||||||
call.
|
|
||||||
|
A render is performed only when `out` is missing or
|
||||||
|
older than `src` AND a `d2` binary is available. The
|
||||||
|
write is atomic (temp-file + `os.replace`) so a
|
||||||
|
failed or torn render never clobbers an existing
|
||||||
|
(committed) SVG.
|
||||||
|
|
||||||
'''
|
'''
|
||||||
stale: bool = (
|
fresh: bool = (
|
||||||
not out.exists()
|
out.exists()
|
||||||
or
|
and
|
||||||
src.stat().st_mtime > out.stat().st_mtime
|
src.stat().st_mtime <= out.stat().st_mtime
|
||||||
)
|
)
|
||||||
if not stale:
|
if fresh:
|
||||||
return True
|
return RenderResult.OK
|
||||||
d2cmd: list[str]|None = find_d2(app)
|
d2cmd: list[str]|None = find_d2(app)
|
||||||
if d2cmd is None:
|
if d2cmd is None:
|
||||||
|
# no binary: fall back to whatever is committed,
|
||||||
|
# else signal the caller to emit the raw source.
|
||||||
if out.exists():
|
if out.exists():
|
||||||
log.info(
|
log.info(
|
||||||
f'no d2 binary; using committed svg '
|
f'no d2 binary; using committed svg '
|
||||||
f'for {src.name}'
|
f'for {src.name}'
|
||||||
)
|
)
|
||||||
return True
|
return RenderResult.FELL_BACK
|
||||||
return False
|
return RenderResult.NO_OUTPUT
|
||||||
out.parent.mkdir(
|
out.parent.mkdir(
|
||||||
parents=True,
|
parents=True,
|
||||||
exist_ok=True,
|
exist_ok=True,
|
||||||
)
|
)
|
||||||
|
# render into a sibling temp-file first so a bad
|
||||||
|
# `d2` exit (or a crash mid-write) leaves any prior
|
||||||
|
# committed SVG fully intact.
|
||||||
|
fd, tmpname = tempfile.mkstemp(
|
||||||
|
dir=str(out.parent),
|
||||||
|
prefix=f'.{out.stem}.',
|
||||||
|
suffix='.svg.tmp',
|
||||||
|
)
|
||||||
|
os.close(fd)
|
||||||
|
tmp = Path(tmpname)
|
||||||
argv: list[str] = (
|
argv: list[str] = (
|
||||||
d2cmd
|
d2cmd
|
||||||
+ list(app.config.d2_args)
|
+ list(app.config.d2_args)
|
||||||
+ [str(src), str(out)]
|
+ [str(src), str(tmp)]
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
proc = sp.run(
|
proc = sp.run(
|
||||||
|
|
@ -120,18 +163,22 @@ def render_svg(
|
||||||
OSError,
|
OSError,
|
||||||
sp.TimeoutExpired,
|
sp.TimeoutExpired,
|
||||||
) as err:
|
) as err:
|
||||||
|
tmp.unlink(missing_ok=True)
|
||||||
log.warning(
|
log.warning(
|
||||||
f'd2 invocation failed for {src.name}: '
|
f'd2 invocation failed for {src.name}: '
|
||||||
f'{err}'
|
f'{err}'
|
||||||
)
|
)
|
||||||
return out.exists()
|
return RenderResult.FAILED
|
||||||
if proc.returncode != 0:
|
if proc.returncode != 0:
|
||||||
|
tmp.unlink(missing_ok=True)
|
||||||
log.warning(
|
log.warning(
|
||||||
f'd2 render error for {src.name}:\n'
|
f'd2 render error for {src.name}:\n'
|
||||||
f'{proc.stderr}'
|
f'{proc.stderr}'
|
||||||
)
|
)
|
||||||
return out.exists()
|
return RenderResult.FAILED
|
||||||
return True
|
# atomic swap-in of the freshly rendered SVG.
|
||||||
|
os.replace(tmp, out)
|
||||||
|
return RenderResult.OK
|
||||||
|
|
||||||
|
|
||||||
class D2Diagram(SphinxDirective):
|
class D2Diagram(SphinxDirective):
|
||||||
|
|
@ -169,18 +216,54 @@ class D2Diagram(SphinxDirective):
|
||||||
/ _outdir
|
/ _outdir
|
||||||
/ f'{src.stem}.svg'
|
/ f'{src.stem}.svg'
|
||||||
)
|
)
|
||||||
if not render_svg(self.env.app, src, out):
|
# collision guard: two distinct sources must not
|
||||||
# last resort: emit the raw d2 source.
|
# map onto the same output stem within a build.
|
||||||
|
prior: str|None = _seen_outputs.get(out.name)
|
||||||
|
if (
|
||||||
|
prior is not None
|
||||||
|
and
|
||||||
|
prior != relsrc
|
||||||
|
):
|
||||||
|
err = self.state_machine.reporter.error(
|
||||||
|
f'd2 output collision: {relsrc!r} and '
|
||||||
|
f'{prior!r} both render to '
|
||||||
|
f'{_outdir}/{out.name}; rename one '
|
||||||
|
f'`.d2` stem',
|
||||||
|
line=self.lineno,
|
||||||
|
)
|
||||||
|
return [err]
|
||||||
|
_seen_outputs[out.name] = relsrc
|
||||||
|
|
||||||
|
result: RenderResult = render_svg(
|
||||||
|
self.env.app,
|
||||||
|
src,
|
||||||
|
out,
|
||||||
|
)
|
||||||
|
if result is RenderResult.FAILED:
|
||||||
|
# loud, build-failing (under `-W`) signal; the
|
||||||
|
# prior committed SVG, if any, is untouched.
|
||||||
|
err = self.state_machine.reporter.error(
|
||||||
|
f'd2 render failed for {relsrc} (see '
|
||||||
|
f'build log); the committed svg, if any, '
|
||||||
|
f'was left untouched',
|
||||||
|
line=self.lineno,
|
||||||
|
)
|
||||||
|
return [err]
|
||||||
|
if result is RenderResult.NO_OUTPUT:
|
||||||
|
# last resort: emit the raw d2 source so no
|
||||||
|
# content is ever silently dropped.
|
||||||
log.warning(
|
log.warning(
|
||||||
f'no svg available for {relsrc}; '
|
f'no svg available for {relsrc}; '
|
||||||
f'emitting d2 source as literal block'
|
f'emitting d2 source as literal block'
|
||||||
)
|
)
|
||||||
|
src_text: str = src.read_text()
|
||||||
literal = nodes.literal_block(
|
literal = nodes.literal_block(
|
||||||
src.read_text(),
|
src_text,
|
||||||
src.read_text(),
|
src_text,
|
||||||
)
|
)
|
||||||
literal['language'] = 'text'
|
literal['language'] = 'text'
|
||||||
return [literal]
|
return [literal]
|
||||||
|
# OK | FELL_BACK -> embed the SVG figure.
|
||||||
img = nodes.image(
|
img = nodes.image(
|
||||||
uri=f'/{_outdir}/{out.name}',
|
uri=f'/{_outdir}/{out.name}',
|
||||||
alt=self.options.get(
|
alt=self.options.get(
|
||||||
|
|
@ -224,12 +307,29 @@ class D2Diagram(SphinxDirective):
|
||||||
return [fig]
|
return [fig]
|
||||||
|
|
||||||
|
|
||||||
|
def _reset_seen(
|
||||||
|
app: Sphinx,
|
||||||
|
) -> None:
|
||||||
|
'''
|
||||||
|
Clear the per-build output-collision map at the
|
||||||
|
start of each build.
|
||||||
|
|
||||||
|
'''
|
||||||
|
_seen_outputs.clear()
|
||||||
|
|
||||||
|
|
||||||
def setup(app: Sphinx) -> dict:
|
def setup(app: Sphinx) -> dict:
|
||||||
app.add_config_value('d2_bin', None, 'env')
|
app.add_config_value('d2_bin', None, 'env')
|
||||||
app.add_config_value('d2_args', [], 'env')
|
app.add_config_value('d2_args', [], 'env')
|
||||||
app.add_directive('d2', D2Diagram)
|
app.add_directive('d2', D2Diagram)
|
||||||
|
app.connect('builder-inited', _reset_seen)
|
||||||
return {
|
return {
|
||||||
'version': '0.1.0',
|
'version': '0.1.0',
|
||||||
|
# NB: run() writes the rendered SVG during the
|
||||||
|
# READ phase; the temp-file + `os.replace` swap
|
||||||
|
# keeps that atomic so concurrent renders of the
|
||||||
|
# same diagram (under `sphinx-build -j`) can't
|
||||||
|
# tear the output file.
|
||||||
'parallel_read_safe': True,
|
'parallel_read_safe': True,
|
||||||
'parallel_write_safe': True,
|
'parallel_write_safe': True,
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue