diff --git a/app/tools/ocr.py b/app/tools/ocr.py index 74ce18f..b8dd0ac 100644 --- a/app/tools/ocr.py +++ b/app/tools/ocr.py @@ -49,29 +49,56 @@ def _find_tesseract() -> str | None: def _find_tessdata(tess_exe: str | None) -> str | None: """Locate the tessdata directory across platforms. - Windows / macOS Homebrew put tessdata next to the binary. Debian / - Ubuntu put it under /usr/share/tesseract-ocr//tessdata, - and the binary itself often has a default path baked in that - points at the wrong version (Ubuntu 24.04 ships v5 but the - default still points at .../4.00/tessdata, see issue #27). - Without TESSDATA_PREFIX set explicitly, OCR fails with + Precedence (first match wins): + + 1. ``/tessdata`` adjacent to the binary — Windows and any + install that bundles the data next to the executable. + 2. The versioned Linux layout + ``/usr/share/tesseract-ocr//tessdata``, reverse-sorted so + the newest version wins. Checked *before* the relative prefix + derivation (step 3) on purpose: the binary often has a stale + default baked in (Ubuntu 24.04 ships v5 but still points at + .../4.00/tessdata, see issue #27), and a flat ``/usr/share/tessdata`` + — which step 3 would derive from ``/usr/bin/tesseract`` — must never + shadow a newer versioned directory. + 3. ``/share/tessdata`` derived relative to the binary + (``/bin/tesseract`` -> ``/share/tessdata``). This + covers Homebrew (Intel ``/usr/local``, Apple Silicon + ``/opt/homebrew``) and non-standard install prefixes; on those + systems the versioned glob in step 2 finds nothing so we land here. + 4. Fixed fallbacks for older Debian, manual installs, Homebrew and + snap layouts. + + Without TESSDATA_PREFIX set explicitly, OCR would otherwise fail with 'Error opening data file .../4.00/tessdata/eng.traineddata'.""" import sys + # 1. tessdata adjacent to the binary (Windows and bundled installs). if tess_exe: adjacent = os.path.join(os.path.dirname(tess_exe), "tessdata") if os.path.isdir(adjacent): return adjacent + # 2. Versioned Linux layout, newest first. Kept ahead of the relative + # prefix derivation so a stale /usr/share/tessdata never shadows a + # newer /usr/share/tesseract-ocr//tessdata (issue #27). if sys.platform.startswith(("linux", "darwin")): import glob - # Sort descending so the latest version wins (5 over 4.00 on - # Ubuntu 24.04 where both folders may coexist). for p in sorted(glob.glob("/usr/share/tesseract-ocr/*/tessdata"), reverse=True): if os.path.isdir(p): return p - # Older Debian, manual installs, snap fallbacks. + # 3. Homebrew (Intel /usr/local, Apple Silicon /opt/homebrew) and + # custom prefixes: /bin/tesseract -> /share/tessdata. + if tess_exe: + prefixed = os.path.join(os.path.dirname(os.path.dirname(tess_exe)), + "share", "tessdata") + if os.path.isdir(prefixed): + return prefixed + # 4. Older Debian, manual installs, Homebrew (Intel + Apple Silicon), + # snap fallbacks. + if sys.platform.startswith(("linux", "darwin")): for p in ("/usr/share/tessdata", "/usr/local/share/tessdata", + "/opt/homebrew/share/tessdata", "/snap/tesseract/current/usr/share/tesseract-ocr/tessdata"): if os.path.isdir(p): return p diff --git a/tests/test_ocr_tessdata.py b/tests/test_ocr_tessdata.py new file mode 100644 index 0000000..8d7681f --- /dev/null +++ b/tests/test_ocr_tessdata.py @@ -0,0 +1,247 @@ +"""Regression tests for ``app.tools.ocr._find_tessdata``. + +The bug: on macOS Homebrew the ``tesseract`` binary lives in +``/bin`` but the language data lives in +``/share/tessdata`` — ``/usr/local`` on Intel and +``/opt/homebrew`` on Apple Silicon. The old ``_find_tessdata`` only +looked for a ``tessdata`` folder *adjacent* to the binary and, as a +fixed fallback, ``/usr/local/share/tessdata`` (Intel only). On an +Apple Silicon Mac (e.g. a MacBook Air M5) it therefore returned +``None`` and the app never set ``TESSDATA_PREFIX``, so extra language +packs such as Hebrew were silently invisible. + +The fix is additive: + +1. Derive ``/share/tessdata`` relative to the binary + (``/bin/tesseract`` -> ``/share/tessdata``). This + covers Intel, Apple Silicon *and* non-standard install prefixes in + one generic step. +2. Add ``/opt/homebrew/share/tessdata`` to the fixed + ``linux``/``darwin`` fallback list, for the case where the binary is + resolved via ``PATH`` and the derived prefix does not match. + +The precedence enforced by ``_find_tessdata`` (first match wins) is: + +1. ``/tessdata`` adjacent to the binary (Windows / bundled). +2. Versioned Linux ``/usr/share/tesseract-ocr//tessdata``, + reverse-sorted so the newest version wins. +3. The relative ``/share/tessdata`` derivation from step (1) + above — deliberately *after* the versioned lookup so a stale flat + ``/usr/share/tessdata`` (which is what the derivation yields for + ``/usr/bin/tesseract``) never shadows a newer versioned directory + (issue #27, PR #146 review). +4. Fixed ``/usr/share``, ``/usr/local/share``, ``/opt/homebrew/share`` + and snap fallbacks. + +Nothing about the existing Windows-adjacent or Linux versioned +behaviour changes. + +These tests never touch the real filesystem or the real ``sys.platform`` +— everything is monkeypatched so they run identically on the Windows and +Linux CI runners (there is no macOS runner). +""" + +from __future__ import annotations + +import glob as _glob +import os +import sys +from pathlib import Path + +# Make the project root importable so ``from app.tools...`` works. +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +# _find_tessdata is a plain function and needs no GUI, but importing the +# module pulls in PySide6 widget classes; force the offscreen platform so +# a headless runner never tries to open a display. +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +from app.tools.ocr import _find_tessdata # noqa: E402 + + +# --------------------------------------------------------------------------- +# helpers +# --------------------------------------------------------------------------- +def _norm(path): + """Collapse ``os.sep`` differences so a test can be written with forward + slashes yet still match paths that ``os.path.join`` built with ``\\`` on + Windows. ``None`` passes through so a regression that returns ``None`` + fails as a plain assertion rather than an ``AttributeError``.""" + return None if path is None else path.replace("\\", "/") + + +def _fake_isdir(existing): + """Return an ``os.path.isdir`` replacement that reports *only* the given + (separator-normalised) directories as existing.""" + wanted = {_norm(p) for p in existing} + + def _isdir(path): + return _norm(path) in wanted + + return _isdir + + +def _patch_fs(monkeypatch, existing, *, platform=None, glob_results=None): + """Install a fake ``os.path.isdir`` (and optionally ``glob.glob`` / + ``sys.platform``) covering exactly *existing*.""" + monkeypatch.setattr(os.path, "isdir", _fake_isdir(existing)) + if platform is not None: + monkeypatch.setattr(sys, "platform", platform) + if glob_results is not None: + monkeypatch.setattr(_glob, "glob", lambda pattern: list(glob_results)) + + +# --------------------------------------------------------------------------- +# prefix-relative derivation (step 3: runs after the adjacent probe AND the +# versioned Linux glob, so the versioned glob is patched empty to prove we +# fall through to the derivation, not to a real CI-runner tessdata dir) +# --------------------------------------------------------------------------- +def test_apple_silicon_homebrew(monkeypatch): + # /bin/tesseract -> /share/tessdata on /opt/homebrew. + # Only the share/tessdata dir exists; the adjacent bin/tessdata does not + # and macOS has no versioned /usr/share/tesseract-ocr layout. + _patch_fs( + monkeypatch, + {"/opt/homebrew/share/tessdata"}, + platform="darwin", + glob_results=[], + ) + result = _find_tessdata("/opt/homebrew/bin/tesseract") + assert _norm(result) == "/opt/homebrew/share/tessdata" + + +def test_intel_homebrew_regression(monkeypatch): + # /usr/local (Intel Homebrew) must keep working via the same derivation. + _patch_fs( + monkeypatch, + {"/usr/local/share/tessdata"}, + platform="darwin", + glob_results=[], + ) + result = _find_tessdata("/usr/local/bin/tesseract") + assert _norm(result) == "/usr/local/share/tessdata" + + +def test_custom_prefix(monkeypatch): + # A non-standard prefix proves the derivation is generic, not hard-coded. + _patch_fs( + monkeypatch, + {"/opt/custom/share/tessdata"}, + platform="linux", + glob_results=[], + ) + result = _find_tessdata("/opt/custom/bin/tesseract") + assert _norm(result) == "/opt/custom/share/tessdata" + + +# --------------------------------------------------------------------------- +# adjacent tessdata (Windows-style install) still wins first +# --------------------------------------------------------------------------- +def test_adjacent_bin_tessdata_takes_precedence(monkeypatch): + # When BOTH the adjacent bin/tessdata and the derived share/tessdata + # exist, the adjacent one (Windows layout) must be returned first. + _patch_fs(monkeypatch, { + "/opt/homebrew/bin/tessdata", + "/opt/homebrew/share/tessdata", + }) + result = _find_tessdata("/opt/homebrew/bin/tesseract") + assert _norm(result) == "/opt/homebrew/bin/tessdata" + + +def test_windows_adjacent(monkeypatch): + # A real Windows install path: tessdata sits next to tesseract.exe. + # Compute the adjacent path exactly as the function does so the test is + # OS-agnostic (ntpath vs posixpath split \\ differently). + tess_exe = r"C:\Program Files\Tesseract-OCR\tesseract.exe" + adjacent = os.path.join(os.path.dirname(tess_exe), "tessdata") + _patch_fs(monkeypatch, {adjacent}) + result = _find_tessdata(tess_exe) + assert _norm(result) == _norm(adjacent) + + +# --------------------------------------------------------------------------- +# fixed macOS fallback (binary found via PATH, derived prefix does not match) +# --------------------------------------------------------------------------- +def test_apple_silicon_fixed_fallback(monkeypatch): + # tesseract resolved from an unrelated location -> derived + # /weird/share/tessdata is absent, so the explicit + # /opt/homebrew/share/tessdata fallback must catch it. + _patch_fs( + monkeypatch, + {"/opt/homebrew/share/tessdata"}, + platform="darwin", + glob_results=[], + ) + result = _find_tessdata("/weird/place/tesseract") + assert _norm(result) == "/opt/homebrew/share/tessdata" + + +# --------------------------------------------------------------------------- +# Linux regressions (unchanged behaviour) +# --------------------------------------------------------------------------- +def test_linux_versioned_tessdata(monkeypatch): + # Debian/Ubuntu layout: /usr/share/tesseract-ocr//tessdata. + _patch_fs( + monkeypatch, + {"/usr/share/tesseract-ocr/5/tessdata"}, + platform="linux", + glob_results=["/usr/share/tesseract-ocr/5/tessdata"], + ) + result = _find_tessdata("/usr/bin/tesseract") + assert _norm(result) == "/usr/share/tesseract-ocr/5/tessdata" + + +def test_linux_reverse_sort_prefers_latest(monkeypatch): + # When 4.00 and 5 coexist (Ubuntu 24.04), the reverse sort must pick 5. + _patch_fs( + monkeypatch, + { + "/usr/share/tesseract-ocr/4.00/tessdata", + "/usr/share/tesseract-ocr/5/tessdata", + }, + platform="linux", + # Deliberately unsorted to prove the function sorts, not glob. + glob_results=[ + "/usr/share/tesseract-ocr/4.00/tessdata", + "/usr/share/tesseract-ocr/5/tessdata", + ], + ) + result = _find_tessdata("/usr/bin/tesseract") + assert _norm(result) == "/usr/share/tesseract-ocr/5/tessdata" + + +def test_linux_versioned_wins_over_flat_usr_share(monkeypatch): + # PR #146 review regression: for /usr/bin/tesseract the relative prefix + # derivation yields the flat /usr/share/tessdata. If that derivation ran + # before the versioned lookup (the bug), a stale /usr/share/tessdata + # would shadow a newer /usr/share/tesseract-ocr/5/tessdata. Both dirs + # exist here; the versioned lookup runs first, so 5 must win (issue #27). + # + # This test FAILS against the pre-fix ordering (which returned + # /usr/share/tessdata) and PASSES once the versioned glob is checked + # before the relative derivation. + _patch_fs( + monkeypatch, + { + "/usr/share/tessdata", + "/usr/share/tesseract-ocr/5/tessdata", + }, + platform="linux", + glob_results=["/usr/share/tesseract-ocr/5/tessdata"], + ) + result = _find_tessdata("/usr/bin/tesseract") + assert _norm(result) == "/usr/share/tesseract-ocr/5/tessdata" + + +# --------------------------------------------------------------------------- +# nothing found +# --------------------------------------------------------------------------- +def test_nothing_found_returns_none(monkeypatch): + _patch_fs(monkeypatch, set(), platform="darwin", glob_results=[]) + assert _find_tessdata("/opt/homebrew/bin/tesseract") is None + + +def test_none_binary_returns_none(monkeypatch): + # No binary and no system paths -> None (never raises on tess_exe=None). + _patch_fs(monkeypatch, set(), platform="linux", glob_results=[]) + assert _find_tessdata(None) is None