diff --git a/CHANGELOG.md b/CHANGELOG.md index 68629a0..7bc3b73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,18 @@ per-file diff commands. ### Fixed +- **`tools/verify_layout.py`'s `skipped:` message named only one cause of a broken + extractor when there are two** (#451) - it blamed the xpdf-based `pdftotext` Git for + Windows puts ahead of Poppler in PATH (no `-bbox` flag, exits 99), but a real Poppler + can abort `-bbox`/`-bbox-layout`/`-htmlmeta` too: Poppler 26.0x before 26.05 crashes on + any PDF whose Info dictionary carries an empty string in any field, and `hyperref` + writes exactly that for every field it does not set. A `lualatex`/`pdflatex` document + built with `hyperref` and no `\hypersetup{pdftitle=...}` - an ordinary `/add-template` + CV template, not a malformed one - hits this with a working Poppler installed, and the + old message sent the reader to check their PATH when nothing was wrong with it. The + message now names both causes; behavior is unchanged, degrading to `skipped:` exit 2 + either way, since a broken extractor is still not a broken document. + - **The template-placeholder guard in `test_setup_command.py` now skips on forks** (#463) - `TemplatesStillCarryThePlaceholders` asserts that `05-cv-templates.md` and `06-cover-letter-templates.md` still contain `[FIRST_NAME]`, `[LAST_NAME]`, `[YOUR_EMAIL]`, diff --git a/tests/test_verify_layout.py b/tests/test_verify_layout.py index 06df97f..31ce49d 100644 --- a/tests/test_verify_layout.py +++ b/tests/test_verify_layout.py @@ -126,6 +126,28 @@ class TestExtractorFailure(unittest.TestCase): with self.assertRaisesRegex(RuntimeError, "bounding boxes"): parse_pdf(Path("cv/main_example.pdf")) + def test_poppler_abort_on_empty_info_string_names_that_cause_too(self): + """Poppler 26.0x before 26.05 aborts -bbox on an empty Info-dict string, e.g. + the empty /Title hyperref writes when pdftitle is unset (#451). That crash is + not the xpdf-shadowing case - it has no -bbox flag and exits 99 - so the + message must name both, not just the one the exit code happens to match. + """ + failure = subprocess.CalledProcessError( + 1, + "pdftotext", + stderr="libc++abi: terminating due to uncaught exception of type " + "std::out_of_range: basic_string", + ) + with patch("tools.verify_layout.shutil.which", return_value="/usr/bin/pdftotext"), patch( + "tools.verify_layout.subprocess.run", side_effect=failure + ): + with self.assertRaisesRegex(RuntimeError, "bounding boxes") as ctx: + parse_pdf(Path("cv/main_example.pdf")) + message = str(ctx.exception) + self.assertIn("xpdf", message) + self.assertIn("Poppler aborted", message) + self.assertIn("hyperref", message) + def test_missing_poppler_raises_a_skippable_error(self): with patch("tools.verify_layout.shutil.which", return_value=None): with self.assertRaisesRegex(RuntimeError, "not found"): diff --git a/tools/verify_layout.py b/tools/verify_layout.py index d0b10d6..0a330aa 100644 --- a/tools/verify_layout.py +++ b/tools/verify_layout.py @@ -26,9 +26,14 @@ that, and CI runs it. Two implementations of one rule drift. Geometry comes from Poppler word bounding boxes (`pdftotext -bbox`). Poppler is optional repo-wide - since #369 `verify_pdf.py` prefers pypdf and falls back to Poppler - but word bounding boxes have no pypdf equivalent, so this is the one step that still wants it. -Without it, or with an extractor that cannot do `-bbox` (Git-for-Windows ships an -xpdf-based `pdftotext` that shadows Poppler in PATH and rejects the flag), the check -reports `skipped:` and exits 2 rather than inventing a layout failure. Line height serves +Without it, or with a broken extractor, the check reports `skipped:` and exits 2 rather +than inventing a layout failure. A broken extractor has two distinct causes: an xpdf-based +`pdftotext` (Git for Windows ships one ahead of Poppler in PATH) has no `-bbox` flag at +all, and Poppler 26.0x before 26.05 aborts `-bbox`/`-bbox-layout`/`-htmlmeta` on a PDF +whose Info dictionary carries an empty string in any field - which `hyperref` writes for +every field it does not set, so any `lualatex`/`pdflatex` document built with `hyperref` +and no `\hypersetup{pdftitle=...}` triggers a real Poppler crashing on a legal PDF (#451). +Line height serves as a font-size proxy to spot section headings; left edge (xMin) separates bullet lines from entry headers. @@ -166,14 +171,22 @@ def parse_pdf(path: Path) -> list[Page]: check=True, ).stdout except subprocess.CalledProcessError as exc: - # An xpdf-based pdftotext has no -bbox and exits 99. That is a broken extractor, - # not a broken document, so it degrades to the skip path instead of exit 1. + # A broken extractor has two distinct causes, not one: an xpdf-based pdftotext + # has no -bbox at all and exits 99 (Git for Windows puts one ahead of Poppler + # in PATH); a real Poppler before 26.05 aborts -bbox on a PDF whose Info dict + # has an empty string in any field - which hyperref writes for every field it + # does not set, so a lualatex/pdflatex document with hyperref and no pdftitle + # triggers this even with a working Poppler (#451). Either way this is a + # broken extractor, not a broken document, so it degrades to the skip path + # instead of exit 1. stderr_lines = (exc.stderr or "").strip().splitlines() detail = stderr_lines[0] if stderr_lines else f"exit {exc.returncode}" raise RuntimeError( f"pdftotext could not produce bounding boxes for {path} ({detail}); " - "a pdftotext without -bbox is usually the xpdf build that Git for Windows " - "puts ahead of Poppler in PATH" + "either the pdftotext first in PATH is an xpdf build with no -bbox flag " + "(Git for Windows ships one ahead of Poppler), or Poppler aborted on this " + "document - Poppler 26.0x before 26.05 aborts on a PDF whose Info " + "dictionary carries an empty string, as hyperref writes when pdftitle is unset" ) from exc pages = [] for _w, h, body in PAGE_RE.findall(out):