From 53f65a97448656172da141b9eb461a93bdbd3cd1 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 26 Jul 2026 18:10:33 +0200 Subject: [PATCH 1/2] feat(python): ship the libmagic database inside the wheel `pyproject.toml` disabled libmagic for wheel builds on the premise that its runtime data "cannot be bundled into the package". That was never true: - the wheel links statically (`BUILD_SHARED_LIBS=OFF`), so libmagic and its zlib/xz/bzip2 dependencies end up inside `_core..so` with nothing for auditwheel to vendor; - `magic.mgc` is 8.5 MB raw but deflates to ~0.4 MB, and wheels are zip-deflated; - `python/CMakeLists.txt` already installs the whole build data directory as `pyodr/data`, so `ODR_BUNDLE_ASSETS=ON` is enough to place the database there. What was actually missing is the runtime fix-up. The compiled-in default from `project_info::libmagic_database_path()` is the relative string `"share/magic.mgc"`, meaningless inside a site-packages install, and `pyodr/__init__.py` only ever repointed the core data path. Give the database the same three-step resolution (configured path, then `ODR_LIBMAGIC_DATABASE_PATH`, then the bundled copy), keyed on `isfile` rather than `isdir`. Verified against an installed wheel: `libmagic_database_path()` resolves inside site-packages and `pyodr.mimetype()` on a minimal `.odt` returns `application/vnd.oasis.opendocument.text` instead of the `application/zip` that odr's own 12-byte sniffing yields. The wheel grew by 444 KB compressed. The new test skips when the build has no database, so in-tree runs without bundled assets stay green. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YU1LRdUxTFmM5m5eCgb5Lb --- .github/workflows/python.yml | 10 ++++++---- pyproject.toml | 8 +++++++- python/AGENTS.md | 3 ++- python/README.md | 22 +++++++++++++++------- python/pyodr/__init__.py | 32 ++++++++++++++++++++++++++++---- python/tests/test_meta.py | 21 +++++++++++++++++++++ 6 files changed, 79 insertions(+), 17 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 51c9f4d85..021413e4a 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -41,7 +41,7 @@ jobs: wheels: runs-on: ${{ matrix.os }} # Builds the reduced dependency set that `pyproject.toml` pins (no - # pdf2htmlEX/wvWare/libmagic), so it cannot share a cache with `build_test`. + # pdf2htmlEX/wvWare), so it cannot share a cache with `build_test`. env: CACHE_FLAVOR: wheel strategy: @@ -104,15 +104,17 @@ jobs: restore-keys: | ccache-${{ env.CACHE_FLAVOR }}-${{ matrix.host_profile }}-${{ env.CCACHE_KEY_SUFFIX }}- - # Match the dependency set that pyproject.toml enables (no pdf2htmlEX, - # wvWare, libmagic — their runtime data cannot ship inside the wheel). + # Match the dependency set that pyproject.toml enables. pdf2htmlEX and + # wvWare stay off (their runtime data cannot ship inside the wheel); + # libmagic stays on and `bundle_assets` bridges its database path into the + # build so `magic.mgc` lands in `pyodr/data`. - name: conan install run: > conan install . -o '&:with_python=True' -o '&:with_pdf2htmlEX=False' -o '&:with_wvWare=False' - -o '&:with_libmagic=False' + -o '&:bundle_assets=True' --profile:host '${{ matrix.host_profile }}' --profile:build '${{ matrix.build_profile }}' --build missing diff --git a/pyproject.toml b/pyproject.toml index 65f510e63..45398d167 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -65,7 +65,13 @@ BUILD_SHARED_LIBS = "OFF" # bundled into the package (PDF rendering falls back to the built-in parser). ODR_WITH_PDF2HTMLEX = "OFF" ODR_WITH_WVWARE = "OFF" -ODR_WITH_LIBMAGIC = "OFF" +# libmagic's database is just a data file, so it can ship: `ODR_BUNDLE_ASSETS` +# copies `magic.mgc` next to the odr.js assets, `python/CMakeLists.txt` installs +# that directory as `pyodr/data`, and `pyodr/__init__.py` points `GlobalParams` +# at the installed copy. Costs ~0.4 MB of wheel (the database deflates to that +# from 8.5 MB) and buys real MIME detection instead of odr's 12-byte sniffing. +ODR_WITH_LIBMAGIC = "ON" +ODR_BUNDLE_ASSETS = "ON" # Section must exist for setuptools-scm to pick up its pyproject config. [tool.setuptools_scm] diff --git a/python/AGENTS.md b/python/AGENTS.md index f81e6ab76..ed54e8740 100644 --- a/python/AGENTS.md +++ b/python/AGENTS.md @@ -26,6 +26,7 @@ pybind11 bindings for the public C++ API (`src/odr/*.hpp`), packaged as - C++ sources follow the repo clang-format; python is formatted with `black`. - Tests must stay hermetic: build inputs inline in `tests/conftest.py`; HTML-rendering tests take the `core_data_path` fixture (skips when assets are - missing). + missing), and tests needing libmagic skip on `_libmagic_database()` in + `tests/test_meta.py` (empty when the build has no database). - Build/test loop: see `python/README.md`; CI lives in `.github/workflows/python.yml`. diff --git a/python/README.md b/python/README.md index 9cfd6ee2b..36f6cd410 100644 --- a/python/README.md +++ b/python/README.md @@ -33,18 +33,26 @@ PYTHONPATH=build/python ODR_CORE_DATA_PATH=build/data python -m pytest python/te `pip install .` from the repository root builds a wheel via scikit-build-core (see the root `pyproject.toml`); run `conan install` first and point `CMAKE_ARGS` at the generated `conan_toolchain.cmake` so the C++ dependencies -resolve. Wheels build without pdf2htmlEX/wvWare/libmagic (their runtime data -cannot ship inside the wheel), so match those options: +resolve. Wheels build without pdf2htmlEX/wvWare (their runtime data cannot ship +inside the wheel) but with libmagic, whose database is bundled — so match those +options: ```bash conan install . -o '&:with_python=True' -o '&:with_pdf2htmlEX=False' \ - -o '&:with_wvWare=False' -o '&:with_libmagic=False' --build missing + -o '&:with_wvWare=False' -o '&:bundle_assets=True' --build missing CMAKE_ARGS="-DCMAKE_TOOLCHAIN_FILE=$PWD/conan_toolchain.cmake" pip install . ``` ## Runtime data -Rendering uses shipped assets (CSS/JS). Wheels bundle them under `pyodr/data` -and pick them up automatically; for in-tree builds set the environment variable -`ODR_CORE_DATA_PATH` (the tests read it) or call -`pyodr.GlobalParams.set_odr_core_data_path(...)`. +Rendering uses shipped assets (CSS/JS), and MIME detection uses libmagic's +compiled database (`magic.mgc`). Wheels bundle both under `pyodr/data` and pick +them up automatically. For in-tree builds set `ODR_CORE_DATA_PATH` (the tests +read it) and, if needed, `ODR_LIBMAGIC_DATABASE_PATH`, or call +`pyodr.GlobalParams.set_odr_core_data_path(...)` / +`set_libmagic_database_path(...)`. + +Neither is fatal when missing: without the assets, rendering fails on the +individual resource; without the database, odrcore tries the system database and +then falls back to its own magic sniffing (which sees an `.odt` as +`application/zip` rather than the ODF type). diff --git a/python/pyodr/__init__.py b/python/pyodr/__init__.py index 06bd58335..c06845039 100644 --- a/python/pyodr/__init__.py +++ b/python/pyodr/__init__.py @@ -21,7 +21,11 @@ __version__ = _core.version() -def _init_data_paths() -> None: +# Runtime assets installed into the package by `python/CMakeLists.txt`. +_BUNDLED_DATA_PATH = Path(__file__).resolve().parent / "data" + + +def _init_odr_core_data_path() -> None: # Rendering needs the shipped odr.js/css assets. Resolution order: an # already-configured valid path wins, then the ODR_CORE_DATA_PATH # environment variable, then the assets bundled with the package. The @@ -34,9 +38,29 @@ def _init_data_paths() -> None: if env_path: _core.GlobalParams.set_odr_core_data_path(env_path) return - data_path = Path(__file__).resolve().parent / "data" - if data_path.is_dir(): - _core.GlobalParams.set_odr_core_data_path(str(data_path)) + if _BUNDLED_DATA_PATH.is_dir(): + _core.GlobalParams.set_odr_core_data_path(str(_BUNDLED_DATA_PATH)) + + +def _init_libmagic_database_path() -> None: + # Same resolution order for libmagic's compiled database, which is a file + # rather than a directory. Leaving it unresolved is not fatal: odrcore then + # tries the system database and finally falls back to its own sniffing. + current = _core.GlobalParams.libmagic_database_path() + if current and os.path.isfile(current): + return + env_path = os.environ.get("ODR_LIBMAGIC_DATABASE_PATH") + if env_path: + _core.GlobalParams.set_libmagic_database_path(env_path) + return + database_path = _BUNDLED_DATA_PATH / "magic.mgc" + if database_path.is_file(): + _core.GlobalParams.set_libmagic_database_path(str(database_path)) + + +def _init_data_paths() -> None: + _init_odr_core_data_path() + _init_libmagic_database_path() _init_data_paths() diff --git a/python/tests/test_meta.py b/python/tests/test_meta.py index d2722f7a3..081508a55 100644 --- a/python/tests/test_meta.py +++ b/python/tests/test_meta.py @@ -1,6 +1,16 @@ +import os + +import pytest + import pyodr +def _libmagic_database() -> str: + """The resolved libmagic database, or "" when this build has none.""" + path = pyodr.GlobalParams.libmagic_database_path() + return path if path and os.path.isfile(path) else "" + + def test_version(): # A dev build carries no project version; only assert consistency. assert isinstance(pyodr.version(), str) @@ -67,3 +77,14 @@ def test_decoder_engine(): def test_global_params(): assert isinstance(pyodr.GlobalParams.odr_core_data_path(), str) + assert isinstance(pyodr.GlobalParams.libmagic_database_path(), str) + + +@pytest.mark.skipif( + not _libmagic_database(), + reason="built without libmagic, or its database was not bundled", +) +def test_mimetype_uses_libmagic(odt_path): + # odr's own sniffing only reaches the ZIP container ("application/zip"); + # recognising the ODF mimetype entry stored inside it is what libmagic adds. + assert pyodr.mimetype(str(odt_path)) == "application/vnd.oasis.opendocument.text" From 3304982b8ea6ac9abf8806259416c97c70ee69fd Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 26 Jul 2026 18:18:29 +0200 Subject: [PATCH 2/2] fix(python): ignore install-relative data paths when resolving The compiled-in defaults from `project_info` are install-relative guesses ("share", "share/magic.mgc"). Testing them with `isdir`/`isfile` resolved them against the process working directory, so launching from a directory that happens to contain a `share/` tree made initialization return early and silently ignore the assets shipped in the package. Only an absolute path that exists now counts as already configured; anything else falls through to the environment variable and then to the bundled copy, as intended. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DBknDRREKdLmcBKaGgBm4t --- python/pyodr/__init__.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/python/pyodr/__init__.py b/python/pyodr/__init__.py index c06845039..18160528c 100644 --- a/python/pyodr/__init__.py +++ b/python/pyodr/__init__.py @@ -25,14 +25,19 @@ _BUNDLED_DATA_PATH = Path(__file__).resolve().parent / "data" +def _is_configured(path: str) -> bool: + # The compiled-in defaults are install-relative guesses ("share", + # "share/magic.mgc"). Resolving those against the process working directory + # would make the outcome depend on where the interpreter was launched, so + # only an absolute path that exists counts as already configured. + return bool(path) and os.path.isabs(path) and os.path.exists(path) + + def _init_odr_core_data_path() -> None: # Rendering needs the shipped odr.js/css assets. Resolution order: an - # already-configured valid path wins, then the ODR_CORE_DATA_PATH - # environment variable, then the assets bundled with the package. The - # built-in default is an install-relative guess ("share"), so only an - # existing directory counts as configured. - current = _core.GlobalParams.odr_core_data_path() - if current and os.path.isdir(current): + # already-configured path wins, then the ODR_CORE_DATA_PATH environment + # variable, then the assets bundled with the package. + if _is_configured(_core.GlobalParams.odr_core_data_path()): return env_path = os.environ.get("ODR_CORE_DATA_PATH") if env_path: @@ -46,8 +51,7 @@ def _init_libmagic_database_path() -> None: # Same resolution order for libmagic's compiled database, which is a file # rather than a directory. Leaving it unresolved is not fatal: odrcore then # tries the system database and finally falls back to its own sniffing. - current = _core.GlobalParams.libmagic_database_path() - if current and os.path.isfile(current): + if _is_configured(_core.GlobalParams.libmagic_database_path()): return env_path = os.environ.get("ODR_LIBMAGIC_DATABASE_PATH") if env_path: