Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
gregbenz
added a commit
that referenced
this pull request
Sep 28, 2026
The installed gain-map and compressed-ICC headers declare five helpers whose implementations live in extras, leaving normal libjxl consumers with undefined symbols. Move the existing implementations into libjxl and separate readers from writers so jxl_dec does not acquire encoder dependencies. Update the CMake, Bazel, and GN source lists without changing public signatures or codec behavior. Add an installed C consumer that links using only libjxl pkg-config metadata and checks bundle/ICC serialization, plus a decoder-only CTest consumer that reads fixed synthetic bytes without encoder linkage. Refresh the Linux loader cache before the existing installed-examples CI step. Fixes libjxl#4865. Validation: the installed consumer fails with all five symbols missing on unmodified upstream on macOS and Linux; the decoder-only consumer fails with both reader symbols missing. Patched shared/static consumers pass on macOS, Linux, and Windows. The decoder regression passes CTest in macOS shared, static-libjxl, and lean builds and runs directly on Linux. Existing focused bundle/ICC tests and formatting/source-list checks pass. The identical library/test changes passed 114 checks in #1, including Bazel, WASM, Windows, macOS, Linux, cross-testing, and conformance. That validation branch also contains unrelated mingw32 and Intel SDE CI repairs excluded here. Windows pkg-config and GN builds were not tested; local static lcms testing on macOS needed an explicit lcms dependency due to existing package metadata.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CI validation in this fork before upstream submission.
The installed gain-map and compressed-ICC headers declare functions that applications cannot link through libjxl. Their implementations currently live in extras.
This moves the five existing helpers into libjxl without changing their signatures or behavior. Read and write implementations are split so decoder-only builds do not pull in encoder code. The generated CMake, Bazel, and GN source lists are updated accordingly.
A new C example links through the libjxl pkg-config package alone, reproduces the link failure before this change, and checks bundle and ICC roundtrips afterward. It runs in the existing installed-examples CI step. Linux now refreshes the loader cache after installation so the example can find libjxl's shared-library dependencies.
A separate CTest consumer links only to the build-tree jxl_dec target and reads fixed synthetic bundle and ICC bytes. It calls no writer helpers, so encoder linkage cannot hide missing decoder symbols.
CI maintenance changes use the already-pinned GoogleTest submodule for mingw32, where the system package is unavailable, and replace the unavailable Intel SDE download with version 10.13.1. Both cross-test and conformance workflows use the updated download. The SDE archive is checked against Intel's published SHA-256. Existing jobs and test coverage are retained.
Related upstream issue: libjxl#4865.
Testing
The new decoder-only test fails to link against clean upstream with exactly the two reader symbols missing. It passes through CTest in macOS shared, lean decoder-only, and static-libjxl builds, and runs directly against the Linux decoder library. Linux CTest configuration in the cached local image lacked PNG development files.
Shared and static consumers pass on macOS arm64, Linux arm64, and Windows x64. The shared consumer fails with all five symbols missing on unmodified upstream on macOS and Linux.
Decoder-only consumers pass on macOS and Windows with tools, boxes, and JPEG transcoding disabled. Only the two reader helpers are exported, with no Brotli dependency.
Existing gain-map and compressed-ICC tests pass on macOS, and cjxl/djxl build successfully.
The Linux loader failure was reproduced in an isolated container. The same C executable passes after refreshing the loader cache.
Bundled GoogleTest selection was verified with the repository's CMake logic and pinned submodule. The new SDE download matches Intel's published checksum and recognizes the existing
-sproption. The prior CI run passed mingw32 and a native cross-test shard using SDE. Conformance validation with the updated download is pending.The macOS static lcms build needs lcms linked explicitly because its existing package metadata omits that dependency. Windows linkage was checked with MSVC and explicit libraries; pkg-config integration was not tested there. Bazel and GN builds were not run locally.