Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/apple.yml
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ jobs:

# The iOS *device* slice is only ever link checked; nothing runs it. The
# simulator run is the analogue of android's instrumented job and the only one
# that sees what a device sees — that `+load` fired, that `NSBundle` found
# `magic.mgc`, that a temp directory is writable inside an app container.
# that sees what a device sees — that `+load` fired, that `NSBundle` found the
# renderer assets, that a temp directory is writable inside an app container.
test:
needs: xcframework
runs-on: macos-15
Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/build_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ jobs:
-DCMAKE_CXX_FLAGS="-Werror"
-DCMAKE_INSTALL_PREFIX=install
-DODR_TEST=ON
-DODR_WITH_LIBMAGIC=ON
-DODR_BUNDLE_ASSETS=ON
${{ matrix.bindings && '-DODR_JNI=ON -DODR_PYTHON=ON' || '' }}

Expand All @@ -138,7 +137,6 @@ jobs:
-DCMAKE_BUILD_TYPE=Release
-DCMAKE_INSTALL_PREFIX=install
-DODR_TEST=ON
-DODR_WITH_LIBMAGIC=OFF

- name: build
run: cmake --build build --config Release
Expand Down
5 changes: 2 additions & 3 deletions .github/workflows/python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,8 @@ jobs:
restore-keys: |
ccache-${{ env.CACHE_FLAVOR }}-${{ matrix.host_profile }}-${{ env.CCACHE_KEY_SUFFIX }}-

# Match the dependency set that pyproject.toml enables. libmagic stays on
# and `bundle_assets` bridges its database path into the build so
# `magic.mgc` lands in `pyodr/data`.
# Match the dependency set that pyproject.toml enables. `bundle_assets`
# is what stages the renderer's css/js so they land in `pyodr/data`.
- name: conan install
run: >
conan install .
Expand Down
11 changes: 6 additions & 5 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ bytes ─▶ magic/open_strategy ─▶ DecodedFile ─▶ Document ─▶ Eleme
DecoderEngine) format) elements) walks public API)
```

1. **Detect** — `internal/magic.cpp` (+ `internal/libmagic`) sniffs the file;
1. **Detect** — `internal/magic.cpp` sniffs the head of the file;
`internal/open_strategy.cpp` picks a `FileType` + `DecoderEngine` and builds
the matching `abstract::DecodedFile`.
the matching `abstract::DecodedFile`. `odr::mimetype` composes the two, so a
zip is named by what is inside it.
2. **Decode** — a document file yields an `abstract::Document`.
3. **Element tree** — a `Document` exposes a root `ElementIdentifier` plus an
`abstract::ElementAdapter`. Public value-semantics handles (`Element`, `Slide`,
Expand Down Expand Up @@ -88,9 +89,9 @@ cmake --build cmake-build-relwithdebinfo --target translate # CLI: file → HTM
when really necessary.
- **Run the test binary from the build dir** so output stays out of the repo tree.
- **For debugging, prefer the `translate` CLI** on a single file over the suite.
- CMake options (`CMakeLists.txt`): `ODR_TEST`, `ODR_CLI`, `ODR_WITH_LIBMAGIC`,
`ODR_PYTHON`, `ODR_JNI`, `ODR_APPLE`, `ODR_CLANG_TIDY`. A new `.cpp` must be
added to `ODR_SOURCE_FILES`.
- CMake options (`CMakeLists.txt`): `ODR_TEST`, `ODR_CLI`, `ODR_PYTHON`,
`ODR_JNI`, `ODR_APPLE`, `ODR_CLANG_TIDY`. A new `.cpp` must be added to
`ODR_SOURCE_FILES`.
- **Test data is fetched, not vendored**, and opt in: `-DODR_TEST_FETCH_DATA=ON`
makes `cmake/test_data.cmake` clone the repositories pinned in
`test/data.cmake` into `test/data/` (they were submodules until then; read
Expand Down
61 changes: 9 additions & 52 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ option(ODR_TEST_FETCH_DATA "Fetch the test data pinned in test/data.cmake" OFF)
option(ODR_CLI "enable command line interface" ON)
option(ODR_WITH_HTTP_SERVER "Build the HTTP server (requires cpp-httplib)" ON)
option(ODR_CLANG_TIDY "Run clang-tidy static analysis" OFF)
option(ODR_WITH_LIBMAGIC "Build with libmagic" ON)
# Removed, and accepted only so a consumer still passing it keeps configuring.
# `odr::mimetype` is our own detection now: it reads inside zip and cfb
# containers, which libmagic cannot, and it costs neither a dependency nor the
# 8 MB `magic.mgc` database every consumer had to ship and point us at.
option(ODR_WITH_LIBMAGIC "Removed, does nothing (deprecated)" OFF)
option(ODR_BUNDLE_ASSETS "Bundle assets during build and install" OFF)
option(ODR_PYTHON "Build Python bindings" OFF)
option(ODR_JNI "Build JNI bindings" OFF)
Expand Down Expand Up @@ -274,30 +278,6 @@ set(ODR_BUILD_ODR_DATA_PATH "${CMAKE_CURRENT_BINARY_DIR}/data")
file(COPY "${odr.js_SOURCE_DIR}/" DESTINATION "${ODR_BUILD_ODR_DATA_PATH}")
set(ODR_INSTALL_ODR_DATA_PATH "${CMAKE_INSTALL_DATADIR}")

# Validate an injected data-file path before it is used to bundle assets.
#
# These paths (LIBMAGIC_DATABASE_PATH, ...) are consumed but
# never discovered by CMake — they must be injected from outside via `-D...=`;
# our conanfile.py bridges them from the dependency runenv. A consumer that
# builds odrcore without that bridge left the variable empty, and
# `file(COPY "${empty}/" ...)` degraded to `file(COPY "/" ...)`, i.e. an attempt
# to recursively copy the whole root filesystem (see issue #599). This guard
# fails fast with an actionable message instead of copying "/".
#
# path value of the externally-injected path variable (may be empty)
# name name of that variable, for the diagnostic
function(odr_require_data_path path name)
if (NOT path)
message(FATAL_ERROR
"ODR_BUNDLE_ASSETS is ON but ${name} is not set. Provide it "
"explicitly (e.g. -D${name}=<path>) or build through the conan "
"recipe, or disable bundling with -DODR_BUNDLE_ASSETS=OFF.")
endif ()
if (NOT EXISTS "${path}")
message(FATAL_ERROR "${name} points to a path that does not exist: '${path}'.")
endif ()
endfunction()

if (ODR_WITH_HTTP_SERVER)
find_package(httplib REQUIRED)
target_sources(odr
Expand All @@ -314,33 +294,10 @@ if (ODR_WITH_HTTP_SERVER)
)
endif ()
if (ODR_WITH_LIBMAGIC)
find_package(libmagic REQUIRED)
target_sources(odr
PRIVATE
"src/odr/internal/libmagic/libmagic.cpp"
)
target_link_libraries(odr
PRIVATE
libmagic::libmagic
)
target_compile_definitions(odr
PRIVATE
ODR_WITH_LIBMAGIC
)

if (ODR_BUNDLE_ASSETS)
odr_require_data_path("${LIBMAGIC_DATABASE_PATH}" LIBMAGIC_DATABASE_PATH)

set(ODR_BUILD_LIBMAGIC_DATABASE_PATH "${ODR_BUILD_ODR_DATA_PATH}/magic.mgc")

file(COPY_FILE "${LIBMAGIC_DATABASE_PATH}" "${ODR_BUILD_LIBMAGIC_DATABASE_PATH}")

set(ODR_INSTALL_LIBMAGIC_DATABASE_PATH "${ODR_INSTALL_ODR_DATA_PATH}/magic.mgc")
else ()
set(ODR_BUILD_LIBMAGIC_DATABASE_PATH "${LIBMAGIC_DATABASE_PATH}")

set(ODR_INSTALL_LIBMAGIC_DATABASE_PATH "${LIBMAGIC_DATABASE_PATH}")
endif ()
message(DEPRECATION
"ODR_WITH_LIBMAGIC no longer does anything: libmagic is gone and "
"`odr::mimetype` is our own detection. Drop the option; "
"`GlobalParams::libmagic_database_path` is inert too.")
endif ()

configure_file("src/odr/internal/project_info.cpp.in" "src/odr/internal/project_info.cpp")
Expand Down
5 changes: 2 additions & 3 deletions android/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,5 @@ calls it.
finds the incompatibilities first.
- The manifest stays permission-free; permissions the server needs are the
app's call, and documented in `README.md` instead.
- Assets keep the `core/odrcore` + `core/libmagic` layout of the droid conan
deployer, so an app can move between the two packagings without touching its
extraction code.
- Assets keep the `core/odrcore` layout of the droid conan deployer, so an app
can move between the two packagings without touching its extraction code.
1 change: 0 additions & 1 deletion android/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ odr-core-android.aar
├── jni/<abi>/libodr_jni.so the bindings with the core linked in
├── jni/<abi>/libc++_shared.so the c++ runtime they were built against
├── assets/core/odrcore/* css/js of the html renderer
├── assets/core/libmagic/magic.mgc libmagic database
└── proguard.txt keeps the classes JNI resolves by name
```

Expand Down
8 changes: 4 additions & 4 deletions android/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ android {
targetCompatibility = JavaVersion.VERSION_17
}

// the instrumented apk carries the bindings and the 8 MB libmagic database,
// and pushing that onto a cold emulator outlasts ddmlib's default timeout,
// which surfaces as a ShellCommandUnresponsiveException rather than as a
// failing test
// the instrumented apk carries the bindings and the renderer assets, and
// pushing that onto a cold emulator outlasts ddmlib's default timeout, which
// surfaces as a ShellCommandUnresponsiveException rather than as a failing
// test
installation { timeOutInMs = 10 * 60 * 1000 }

sourceSets {
Expand Down
6 changes: 1 addition & 5 deletions android/build_native.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
prebuilt/jniLibs/<abi>/libodr_jni.so the bindings, core linked in
prebuilt/jniLibs/<abi>/libc++_shared.so from the NDK, see below
prebuilt/assets/core/odrcore/* css/js of the html renderer
prebuilt/assets/core/libmagic/magic.mgc libmagic database

`libc++_shared.so` has to be shipped because the android profiles build against
the shared c++ runtime and nothing else in a consuming app pulls it in; the
Expand Down Expand Up @@ -99,7 +98,6 @@ def build(architecture: str, conan: str, build_profile: str, output: Path) -> No
"-DODR_JNI=ON",
"-DODR_CLI=OFF",
"-DODR_TEST=OFF",
"-DODR_WITH_LIBMAGIC=ON",
"-DODR_WITH_HTTP_SERVER=ON",
"-DODR_BUNDLE_ASSETS=ON"])
Comment thread
andiwand marked this conversation as resolved.
run(["cmake", "--build", cmake_dir, "--target", "odr_jni",
Expand All @@ -117,9 +115,7 @@ def build(architecture: str, conan: str, build_profile: str, output: Path) -> No
data = cmake_dir / "data"
assets = output / "assets" / "core"
shutil.rmtree(assets, ignore_errors=True)
shutil.copytree(data, assets / "odrcore", ignore=shutil.ignore_patterns("magic.mgc"))
(assets / "libmagic").mkdir(parents=True, exist_ok=True)
shutil.copy2(data / "magic.mgc", assets / "libmagic" / "magic.mgc")
shutil.copytree(data, assets / "odrcore")


def main() -> int:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ class OdrAndroidTest {
assertTrue("$data is not a directory", data.isDirectory)
assertTrue(File(data, "document.css").isFile)
assertTrue(File(data, "document.js").isFile)

val magic = File(GlobalParams.libmagicDatabasePath())
assertTrue("$magic is not a file", magic.isFile)
}

@Test
Expand All @@ -46,12 +43,12 @@ class OdrAndroidTest {
}

@Test
fun detectsTypeWithTheBundledMagicDatabase() {
fun detectsTypeInsideTheContainer() {
val directory = TestSupport.tempDir("magic")
val odt = TestFiles.odtFile(directory)

// goes through libmagic, so it only works when the database extracted above
// is the one the native library actually opened
// naming the odt rather than the zip holding it means detection opened the
// container
assertEquals("application/vnd.oasis.opendocument.text", Odr.mimetype(odt.toString()))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import java.io.FileOutputStream
import java.io.IOException

/**
* Makes the library usable on android: the renderer reads its CSS/JS and the libmagic database as
* plain files, and an APK holds them as assets, which are not files.
* Makes the library usable on android: the renderer reads its CSS/JS as plain files, and an APK
* holds them as assets, which are not files.
*
* Call [init] once before anything else touches the library:
* ```
Expand All @@ -22,7 +22,7 @@ object OdrAndroid {
/** Asset directory this AAR ships its runtime data under. */
private const val ASSETS = "core"

/** The libmagic database alone is 8 MB, so the default 8 KB would be a lot of syscalls. */
/** The default 8 KB would be a lot of syscalls for assets this size. */
private const val BUFFER_SIZE = 64 * 1024

private var initialized = false
Expand Down Expand Up @@ -56,7 +56,6 @@ object OdrAndroid {
}

GlobalParams.setOdrCoreDataPath(File(root, "odrcore").absolutePath)
GlobalParams.setLibmagicDatabasePath(File(root, "libmagic/magic.mgc").absolutePath)
initialized = true
}

Expand Down
16 changes: 8 additions & 8 deletions apple/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,17 @@ it — a simulator binary mistagged `IOS` is the classic
| `macos-arm64_x86_64` | `apple-macos-armv8` + `apple-macos-x86_64` |

`assemble` also fails the build if a framework is missing its headers, module
map, `magic.mgc` or the plist's platform keys — the analogue of
map, renderer resources or the plist's platform keys — the analogue of
`android/build.gradle.kts`'s `checkNative`, and for the same reason: those all
publish happily and then fail at the consumer.

## Why a dynamic framework

`+load` in `src/OdrCoreBootstrap.mm` is what points odrcore at the css/js and
`magic.mgc` in this bundle, so an app never has to. In a **static** framework
nothing references that translation unit, the linker drops it, and the
bootstrap never runs — and a SwiftPM binary target gives the consumer no way to
pass `-ObjC`/`-force_load` to get it back. Two lesser reasons: `.binaryTarget`
`+load` in `src/OdrCoreBootstrap.mm` is what points odrcore at the css/js in
this bundle, so an app never has to. In a **static** framework nothing
references that translation unit, the linker drops it, and the bootstrap never
runs — and a SwiftPM binary target gives the consumer no way to pass
`-ObjC`/`-force_load` to get it back. Two lesser reasons: `.binaryTarget`
has no `resources:`, so the assets have to live in the bundle; and an undefined
symbol becomes a link error here instead of a crash at the consumer.

Expand Down Expand Up @@ -130,8 +130,8 @@ which would leave the tag serving the previous version's binary.
The iOS *device* slice is only ever link-checked — nothing runs it. The
simulator suite is the analogue of android's instrumented job and the only
place that sees what a device sees: that `+load` fired, that `NSBundle` found
`magic.mgc`, that `temp_directory_path()` is writable inside an app container.
A new binding is only covered once something in `tests/` calls it.
the renderer assets, that `temp_directory_path()` is writable inside an app
container. A new binding is only covered once something in `tests/` calls it.

`tests/Fixtures/mixed-layout.odt` is 9 KB of `odt/` from OpenDocument.test,
carried here because `test/data/` is fetched by `cmake/test_data.cmake` and a
Expand Down
7 changes: 3 additions & 4 deletions apple/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ if (BUILD_SHARED_LIBS)
endif ()
if (NOT ODR_BUNDLE_ASSETS)
message(FATAL_ERROR
"ODR_APPLE needs ODR_BUNDLE_ASSETS=ON: the renderer's css/js and "
"the libmagic database ship as bundle resources")
"ODR_APPLE needs ODR_BUNDLE_ASSETS=ON: the renderer's css/js ship "
"as bundle resources")
endif ()

enable_language(OBJCXX)
Expand Down Expand Up @@ -64,8 +64,7 @@ set(ODR_APPLE_PUBLIC_HEADERS
"include/OdrCoreObjC/ODRTable.h"
)

# The renderer data the root build already staged, plus the libmagic database
# `ODR_BUNDLE_ASSETS` copied in beside it.
# The renderer's css/js, as the root build staged them.
file(GLOB ODR_APPLE_RESOURCES "${ODR_BUILD_ODR_DATA_PATH}/*")
if (NOT ODR_APPLE_RESOURCES)
message(FATAL_ERROR "no resources in ${ODR_BUILD_ODR_DATA_PATH}")
Expand Down
4 changes: 2 additions & 2 deletions apple/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ for view in service.views {
}
```

Nothing needs configuring first. The framework points odrcore at the css, JS and
libmagic database it carries before `main` runs — that is what the `+load` in
Nothing needs configuring first. The framework points odrcore at the css and JS
it carries before `main` runs — that is what the `+load` in
`OdrCoreBootstrap.mm` is for. Override it with `GlobalParams` from
`application(_:didFinishLaunchingWithOptions:)` if you relocated the resources.

Expand Down
2 changes: 0 additions & 2 deletions apple/build_xcframework.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ def build(profile: str, conan: str, build_profile: str) -> None:
"-DODR_TEST=OFF",
"-DODR_JNI=OFF",
"-DODR_PYTHON=OFF",
"-DODR_WITH_LIBMAGIC=ON",
"-DODR_WITH_HTTP_SERVER=ON",
"-DODR_BUNDLE_ASSETS=ON"])
Comment thread
andiwand marked this conversation as resolved.
run(["cmake", "--build", cmake_dir, "--target", "odr_apple",
Expand Down Expand Up @@ -159,7 +158,6 @@ def assert_contents(framework: Path) -> None:
required = [
root / "Headers" / f"{FRAMEWORK}.h",
root / "Modules" / "module.modulemap",
resources / "magic.mgc",
resources / "document.css",
]
missing = [path for path in required if not path.exists()]
Expand Down
2 changes: 1 addition & 1 deletion apple/exported_symbols.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# The framework's entire public surface is its ObjC classes, so export those
# and nothing else. Everything linked in — cryptopp, libmagic, all of `odr::` —
# and nothing else. Everything linked in — cryptopp, pugixml, all of `odr::` —
# becomes private-extern, which keeps an app that links its own copy of any of
# them from binding against ours.
#
Expand Down
19 changes: 19 additions & 0 deletions apple/include/OdrCoreObjC/ODRFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,23 @@ typedef NS_ENUM(NSInteger, ODRFileType) {

ODRFileTypeTruetypeFont,
ODRFileTypeOpentypeFont,

ODRFileTypeWebp,
ODRFileTypeTaggedImageFileFormat,
ODRFileTypeHighEfficiencyImageFormat,
ODRFileTypeAv1ImageFileFormat,

ODRFileTypeMpegAudio,
ODRFileTypeMpeg4Audio,
ODRFileTypeOggAudio,
ODRFileTypeWaveformAudio,
ODRFileTypeFreeLosslessAudioCodec,

ODRFileTypeMpeg4Video,
ODRFileTypeQuicktimeVideo,
ODRFileTypeThirdGenerationPartnershipVideo,
ODRFileTypeMatroskaVideo,
ODRFileTypeAudioVideoInterleave,
} NS_SWIFT_NAME(FileType);

typedef NS_ENUM(NSInteger, ODRFileCategory) {
Expand All @@ -65,6 +82,8 @@ typedef NS_ENUM(NSInteger, ODRFileCategory) {
ODRFileCategoryArchive,
ODRFileCategoryDocument,
ODRFileCategoryFont,
ODRFileCategoryAudio,
ODRFileCategoryVideo,
} NS_SWIFT_NAME(FileCategory);

typedef NS_ENUM(NSInteger, ODRFileLocation) {
Expand Down
Loading
Loading