mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-09-17 08:36:25 +00:00
verify_layout.py's skipped: message blamed only the xpdf-based pdftotext that
Git for Windows puts ahead of Poppler in PATH. A real Poppler can abort too:
26.0x before 26.05 crashes -bbox/-bbox-layout/-htmlmeta on a 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.
Named both causes in the raised message, the code comment above it, and the
module docstring. Behavior is unchanged: either cause still degrades to
skipped: exit 2, not exit 1, since a broken extractor is still not a broken
document.
Added test_poppler_abort_on_empty_info_string_names_that_cause_too, mocking
subprocess.run to raise CalledProcessError the way a real Poppler 26.0x abort
does (exit 1, a libc++abi out_of_range trace on stderr) rather than the
xpdf case's exit 99 - the two failures are the same exception type with
different exit codes and stderr text, so the message has to actually
distinguish them, not just catch the one class. Negative control: this test
fails against the pre-fix message with "'Poppler aborted' not found in
'...a pdftotext without -bbox is usually the xpdf build...'".
Thanks to @main-sounds-audio for the report and the isolated repro (which
field, which Poppler modes, five runs of five) that pinned this to Poppler's
own upstream regression (issue #1699, fixed in 26.05.0) rather than an
extraction-library swap.
Fixes #451
This commit is contained in:
@@ -55,6 +55,18 @@ per-file diff commands.
|
|||||||
|
|
||||||
### Fixed
|
### 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) -
|
- **The template-placeholder guard in `test_setup_command.py` now skips on forks** (#463) -
|
||||||
`TemplatesStillCarryThePlaceholders` asserts that `05-cv-templates.md` and
|
`TemplatesStillCarryThePlaceholders` asserts that `05-cv-templates.md` and
|
||||||
`06-cover-letter-templates.md` still contain `[FIRST_NAME]`, `[LAST_NAME]`, `[YOUR_EMAIL]`,
|
`06-cover-letter-templates.md` still contain `[FIRST_NAME]`, `[LAST_NAME]`, `[YOUR_EMAIL]`,
|
||||||
|
|||||||
@@ -126,6 +126,28 @@ class TestExtractorFailure(unittest.TestCase):
|
|||||||
with self.assertRaisesRegex(RuntimeError, "bounding boxes"):
|
with self.assertRaisesRegex(RuntimeError, "bounding boxes"):
|
||||||
parse_pdf(Path("cv/main_example.pdf"))
|
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):
|
def test_missing_poppler_raises_a_skippable_error(self):
|
||||||
with patch("tools.verify_layout.shutil.which", return_value=None):
|
with patch("tools.verify_layout.shutil.which", return_value=None):
|
||||||
with self.assertRaisesRegex(RuntimeError, "not found"):
|
with self.assertRaisesRegex(RuntimeError, "not found"):
|
||||||
|
|||||||
+20
-7
@@ -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
|
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
|
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.
|
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
|
Without it, or with a broken extractor, the check reports `skipped:` and exits 2 rather
|
||||||
xpdf-based `pdftotext` that shadows Poppler in PATH and rejects the flag), the check
|
than inventing a layout failure. A broken extractor has two distinct causes: an xpdf-based
|
||||||
reports `skipped:` and exits 2 rather than inventing a layout failure. Line height serves
|
`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
|
as a font-size proxy to spot section headings; left edge (xMin) separates bullet lines
|
||||||
from entry headers.
|
from entry headers.
|
||||||
|
|
||||||
@@ -166,14 +171,22 @@ def parse_pdf(path: Path) -> list[Page]:
|
|||||||
check=True,
|
check=True,
|
||||||
).stdout
|
).stdout
|
||||||
except subprocess.CalledProcessError as exc:
|
except subprocess.CalledProcessError as exc:
|
||||||
# An xpdf-based pdftotext has no -bbox and exits 99. That is a broken extractor,
|
# A broken extractor has two distinct causes, not one: an xpdf-based pdftotext
|
||||||
# not a broken document, so it degrades to the skip path instead of exit 1.
|
# 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()
|
stderr_lines = (exc.stderr or "").strip().splitlines()
|
||||||
detail = stderr_lines[0] if stderr_lines else f"exit {exc.returncode}"
|
detail = stderr_lines[0] if stderr_lines else f"exit {exc.returncode}"
|
||||||
raise RuntimeError(
|
raise RuntimeError(
|
||||||
f"pdftotext could not produce bounding boxes for {path} ({detail}); "
|
f"pdftotext could not produce bounding boxes for {path} ({detail}); "
|
||||||
"a pdftotext without -bbox is usually the xpdf build that Git for Windows "
|
"either the pdftotext first in PATH is an xpdf build with no -bbox flag "
|
||||||
"puts ahead of Poppler in PATH"
|
"(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
|
) from exc
|
||||||
pages = []
|
pages = []
|
||||||
for _w, h, body in PAGE_RE.findall(out):
|
for _w, h, body in PAGE_RE.findall(out):
|
||||||
|
|||||||
Reference in New Issue
Block a user