From 6063c8201eb9afa24110b8c44764f78a9bc2cd77 Mon Sep 17 00:00:00 2001 From: Julien Schueller Date: Fri, 28 Mar 2025 09:17:26 +0100 Subject: [PATCH 1/2] imgmath: Avoid parallel workers depth write race Ensure parallel workers do not try to write the image depth multiple times to achieve reproducible builds. Typically my svg files ended up like this: ``` ... ``` So images were not the same and sometimes the depth was incorrect too. Introduces a new dependency to the filelock module. --- pyproject.toml | 1 + sphinx/ext/imgmath.py | 82 ++++++++++++++++++++++++++----------------- 2 files changed, 51 insertions(+), 32 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 92f00d3a759..f9153e5b4e2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,6 +84,7 @@ dependencies = [ "roman-numerals-py>=1.0.0", "packaging>=23.0", "colorama>=0.4.6; sys_platform == 'win32'", + "filelock>=3.0", ] dynamic = ["version"] diff --git a/sphinx/ext/imgmath.py b/sphinx/ext/imgmath.py index 5b58db7b084..efb2a613427 100644 --- a/sphinx/ext/imgmath.py +++ b/sphinx/ext/imgmath.py @@ -17,6 +17,7 @@ from subprocess import CalledProcessError from typing import TYPE_CHECKING +import filelock from docutils import nodes import sphinx @@ -29,6 +30,8 @@ from sphinx.util.template import LaTeXRenderer if TYPE_CHECKING: + from typing import Any + from docutils.nodes import Element from sphinx.application import Sphinx @@ -82,7 +85,7 @@ def read_svg_depth(filename: str | os.PathLike[str]) -> int | None: def write_svg_depth(filename: Path, depth: int) -> None: """Write the depth to SVG file as a comment at end of file""" with open(filename, 'a', encoding='utf-8') as f: - f.write('\n' % depth) + f.write(f'\n') def generate_latex_macro( @@ -266,36 +269,46 @@ def render_math( ) generated_path = self.builder.outdir / self.builder.imagedir / 'math' / filename generated_path.parent.mkdir(parents=True, exist_ok=True) - if generated_path.is_file(): - if image_format == 'png': - depth = read_png_depth(generated_path) - elif image_format == 'svg': - depth = read_svg_depth(generated_path) - return generated_path, depth - - # if latex or dvipng (dvisvgm) has failed once, don't bother to try again - latex_failed = hasattr(self.builder, '_imgmath_warned_latex') - trans_failed = hasattr(self.builder, '_imgmath_warned_image_translator') - if latex_failed or trans_failed: - return None, None - - # .tex -> .dvi - try: - dvipath = compile_math(latex, self.builder) - except InvokeError: - self.builder._imgmath_warned_latex = True # type: ignore[attr-defined] - return None, None - - # .dvi -> .png/.svg - try: - if image_format == 'png': - depth = convert_dvi_to_png(dvipath, self.builder, generated_path) - elif image_format == 'svg': - depth = convert_dvi_to_svg(dvipath, self.builder, generated_path) - except InvokeError: - self.builder._imgmath_warned_image_translator = True # type: ignore[attr-defined] - return None, None + # ensure parallel workers do not try to write the image depth + # multiple times to achieve reproducible builds + lock: Any = contextlib.nullcontext() + if self.builder.parallel_ok: + lockfile = generated_path.with_suffix(generated_path.suffix + '.lock') + lock = filelock.FileLock(lockfile) + + with lock: + if not generated_path.is_file(): + # if latex or dvipng (dvisvgm) has failed once, don't bother to try again + latex_failed = hasattr(self.builder, '_imgmath_warned_latex') + trans_failed = hasattr(self.builder, '_imgmath_warned_image_translator') + if latex_failed or trans_failed: + return None, None + + # .tex -> .dvi + try: + dvipath = compile_math(latex, self.builder) + except InvokeError: + self.builder._imgmath_warned_latex = True # type: ignore[attr-defined] + return None, None + + # .dvi -> .png/.svg + try: + if image_format == 'png': + depth = convert_dvi_to_png(dvipath, self.builder, generated_path) + elif image_format == 'svg': + depth = convert_dvi_to_svg(dvipath, self.builder, generated_path) + except InvokeError: + self.builder._imgmath_warned_image_translator = True # type: ignore[attr-defined] + return None, None + + return generated_path, depth + + # at this point it has been created + if image_format == 'png': + depth = read_png_depth(generated_path) + elif image_format == 'svg': + depth = read_svg_depth(generated_path) return generated_path, depth @@ -319,11 +332,16 @@ def clean_up_files(app: Sphinx, exc: Exception) -> None: with contextlib.suppress(Exception): shutil.rmtree(app.builder._imgmath_tempdir) + math_outdir = app.builder.outdir / app.builder.imagedir / 'math' if app.builder.config.imgmath_embed: # in embed mode, the images are still generated in the math output dir # to be shared across workers, but are not useful to the final document with contextlib.suppress(Exception): - shutil.rmtree(app.builder.outdir / app.builder.imagedir / 'math') + shutil.rmtree(math_outdir) + else: + # cleanup lock files when using parallel workers + for lockfile in math_outdir.glob('*.lock'): + Path.unlink(lockfile) def get_tooltip(self: HTML5Translator, node: Element) -> str: @@ -383,7 +401,7 @@ def html_visit_displaymath(self: HTML5Translator, node: nodes.math_block) -> Non self.body.append('

') if node['number']: number = get_node_equation_number(self, node) - self.body.append('(%s)' % number) + self.body.append(f'({number})') self.add_permalink_ref(node, _('Link to this equation')) self.body.append('') From 6a8e614c3c27a40334538d009d4a690830179418 Mon Sep 17 00:00:00 2001 From: Julien Schueller Date: Sun, 27 Apr 2025 09:40:31 +0200 Subject: [PATCH 2/2] imgmath: Make lockfile optional Instead of mandatory --- pyproject.toml | 4 +++- sphinx/ext/imgmath.py | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index f9153e5b4e2..04b887dc1fb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,7 +84,6 @@ dependencies = [ "roman-numerals-py>=1.0.0", "packaging>=23.0", "colorama>=0.4.6; sys_platform == 'win32'", - "filelock>=3.0", ] dynamic = ["version"] @@ -108,6 +107,9 @@ lint = [ "pypi-attestations==0.0.25", "betterproto==2.0.0b6", ] +package = [ +"filelock>=3.0", +] test = [ "pytest>=8.0", "pytest-xdist[psutil]>=3.4", diff --git a/sphinx/ext/imgmath.py b/sphinx/ext/imgmath.py index efb2a613427..aa53aa74167 100644 --- a/sphinx/ext/imgmath.py +++ b/sphinx/ext/imgmath.py @@ -17,7 +17,6 @@ from subprocess import CalledProcessError from typing import TYPE_CHECKING -import filelock from docutils import nodes import sphinx @@ -275,7 +274,12 @@ def render_math( lock: Any = contextlib.nullcontext() if self.builder.parallel_ok: lockfile = generated_path.with_suffix(generated_path.suffix + '.lock') - lock = filelock.FileLock(lockfile) + try: + import filelock # type: ignore[import-not-found] + + lock = filelock.FileLock(lockfile) + except ImportError: + pass with lock: if not generated_path.is_file():