chore(deps): update dependency thirdparty/mupdf to v1.28.0 - #687
Conversation
ec5c165 to
9ffab10
Compare
Edited/Blocked NotificationRenovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR. You can manually request rebase by checking the rebase/retry box above. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
WalkthroughMuPDF is upgraded from 1.27.2 to 1.28.0 across submodule, build, and exported version references. Shared MuPDF Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/build-deps/src/build/native.rs (1)
556-576: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe two "cache not complete" tests don't isolate their claimed condition.
native_cache_complete_false_when_marker_stale_but_libmupdf_a_existsandnative_cache_complete_false_when_artifacts_missingnever set up a libwebp directory or marker. Sincenative_cache_complete_atshort-circuits onmarkers::is_built(git_root, &libwebp_dir, "thirdparty/libwebp")first, both assertions pass simply because libwebp isn't built — not because of the specific mupdf marker/artifact condition each test name claims to exercise. If the mupdf-specific logic being tested were broken, these tests would still pass.Set up a valid libwebp marker (as done in
native_cache_complete_true_when_fresh) in both tests so only the intended mupdf condition varies.♻️ Suggested fix
#[test] fn native_cache_complete_false_when_marker_stale_but_libmupdf_a_exists() { let git_root = workspace_root(); let artifact_root = artifact_tempdir(); let fixture = NativeMupdfFixture::new(artifact_root.path()); + let libwebp_dir = build_root(artifact_root.path()).join("libwebp"); + std::fs::create_dir_all(&libwebp_dir).unwrap(); + markers::mark_built(git_root, &libwebp_dir, "libwebp", "thirdparty/libwebp").unwrap(); std::fs::write(markers::built_marker_path(&fixture.mupdf_dir), FAKE_SHA).unwrap(); assert!(!native_cache_complete_at(git_root, artifact_root.path())); } #[test] fn native_cache_complete_false_when_artifacts_missing() { let git_root = workspace_root(); let artifact_root = artifact_tempdir(); let fixture = NativeMupdfFixture::new(artifact_root.path()); + let libwebp_dir = build_root(artifact_root.path()).join("libwebp"); + std::fs::create_dir_all(&libwebp_dir).unwrap(); + markers::mark_built(git_root, &libwebp_dir, "libwebp", "thirdparty/libwebp").unwrap(); markers::mark_built(git_root, &fixture.mupdf_dir, "mupdf", "thirdparty/mupdf").unwrap(); std::fs::remove_file(fixture.mupdf_dir.join("build/release/libmupdf.a")).unwrap(); assert!(!native_cache_complete_at(git_root, artifact_root.path())); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/build-deps/src/build/native.rs` around lines 556 - 576, Update native_cache_complete_false_when_marker_stale_but_libmupdf_a_exists and native_cache_complete_false_when_artifacts_missing to create a valid libwebp build marker using the same setup as native_cache_complete_true_when_fresh before asserting. Keep each test’s mupdf-specific condition as the only varying factor.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/build-deps/src/build/native.rs`:
- Around line 117-134: Update native_cache_complete_at to verify libwebp’s built
artifacts in addition to its marker: require src/.libs/libwebp.a and the
src/.libs and src/demux/.libs directories under libwebp_dir. Preserve the
existing MuPDF checks and return true only when all required artifacts exist.
---
Nitpick comments:
In `@crates/build-deps/src/build/native.rs`:
- Around line 556-576: Update
native_cache_complete_false_when_marker_stale_but_libmupdf_a_exists and
native_cache_complete_false_when_artifacts_missing to create a valid libwebp
build marker using the same setup as native_cache_complete_true_when_fresh
before asserting. Keep each test’s mupdf-specific condition as the only varying
factor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: d1d34da9-1b0d-4a13-8615-d2d59b8c8f44
📒 Files selected for processing (10)
.gitmodulesbuild-scripts/mupdf/kobo.patchcrates/build-deps/src/build/kobo.rscrates/build-deps/src/build/kobo/mupdf.rscrates/build-deps/src/build/mupdf.rscrates/build-deps/src/build/native.rscrates/build-deps/src/markers.rscrates/build-deps/src/versions.rscrates/core/src/document/mupdf_sys.rsthirdparty/mupdf
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
crates/core/**/*.rs
📄 CodeRabbit inference engine (crates/core/REVIEW.md)
crates/core/**/*.rs: Do not use"string literal".to_string()orformat!("literal")for user-visible text.
Thefl!macro must be used at every call site for user-visible text.
crates/core/**/*.rs: Allhandle_eventandrendermethods in view components must use the specified OpenTelemetry#[cfg_attr(feature = "tracing", tracing::instrument(...))]instrumentation.
Instrumentationskip()names must exactly match the method parameter names, including underscore prefixes.
Verify tracing instrumentation withcargo check --features tracing.
A view'srender()is called only when it has no children or is a background view; container views with children are not rendered directly.
For container decoration, use a background view for content rendered before children, or a dedicated child view for overlays rendered in child order.
Rustdoc examples should prefer fully compilable examples, thenno_run, and useignoreonly for private orpub(crate)items unreachable from the test harness, with an explanatory comment.
Use#to hide Rustdoc example boilerplate and verify examples withcargo test --doc.
Use typed SQLx macros:sqlx::query!,sqlx::query_as!, andsqlx::query_scalar!; do not use untyped query functions.
Use.flatten()onquery_scalar!results for nullable columns producingOption<Option<T>>.
Untyped SQLx queries are permitted for dynamic SQL only when unit tests cover every dynamic path and a comment explains why a typed macro cannot be used.
All user-visible strings must usefl!withuse crate::fl;; do not hardcode labels, buttons, placeholders, notifications, or other user-facing text.
Pass Fluent variable values at call sites usingfl!("id", var = value).
Apply localization toSettingKindlabels,Event::OpenNamedInputlabels, menu entry text, button labels, notification text, and all other user-visible strings.
Files:
crates/core/src/document/mupdf_sys.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: The emulator code path should panic on errors, while the app code path should handle errors gracefully.
Prefer?overunwrap()orexpect()in library and app Rust code.
Usethiserrorfor custom error types andanyhowfor ad-hoc errors.
Prefer iterators over index-based loops.
Use&strinstead ofStringin function parameters when ownership is not needed.
Prefer borrowing over cloning.
Inline expressions directly into struct fields instead of introducing intermediate bindings solely for a struct literal.
Avoidunsafeunless it is required and documented.
Avoid prematurecollect()calls and keep iterators lazy.
Ensure Rust code compiles without warnings.
Use the most restrictive visibility that works: prefer privatemodoverpub mod,pub(crate)overpub, and expose items publicly only when non-documentation code outside the parent module uses them.
Before accepting newpubitems, check for external non-documentation usage with imports or fully qualified paths.
Prefer newtype wrappers over raw primitives when values represent domain concepts.
ImplementDisplay,FromStr, and other appropriate standard traits on newtypes to keep them ergonomic.
Match on enum variants instead of comparing strings.
Comments should explain why, not what; avoid inline comments, commented-out code, changelog comments, and divider comments. ContextualTODO,FIXME,HACK, andNOTEannotations are allowed.
Use structured fields with thetracingcrate; never put log data in formatted strings, use no module prefixes, and do not mix structured fields with format arguments.
Usedebugfor development detail,infofor important runtime events,warnfor recoverable issues, anderrorfor failures requiring attention.
Return domain types directly fromsqlxqueries instead of parsing primitives afterward.
Implementsqlx::Type,Encode, andDecodefor owned newtypes used in SQLite query results, delegating to the inner primitive.
Use sqlx t...
Files:
crates/core/src/document/mupdf_sys.rscrates/build-deps/src/versions.rscrates/build-deps/src/build/kobo/mupdf.rscrates/build-deps/src/build/mupdf.rscrates/build-deps/src/build/kobo.rscrates/build-deps/src/markers.rscrates/build-deps/src/build/native.rs
**/*.{rs,toml,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
All feature combinations run on
ubuntu-latest; onlydefaultandtestfeatures produce build artifacts.
Files:
crates/core/src/document/mupdf_sys.rscrates/build-deps/src/versions.rscrates/build-deps/src/build/kobo/mupdf.rscrates/build-deps/src/build/mupdf.rscrates/build-deps/src/build/kobo.rscrates/build-deps/src/markers.rscrates/build-deps/src/build/native.rs
**/*.{rs,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
When testing strings rendered through
fl!, construct expected strings withfl!as well.
Files:
crates/core/src/document/mupdf_sys.rscrates/build-deps/src/versions.rscrates/build-deps/src/build/kobo/mupdf.rscrates/build-deps/src/build/mupdf.rscrates/build-deps/src/build/kobo.rscrates/build-deps/src/markers.rscrates/build-deps/src/build/native.rs
🧠 Learnings (1)
📚 Learning: 2026-05-23T14:57:43.820Z
Learnt from: OGKevin
Repo: OGKevin/cadmus PR: 478
File: crates/core/src/view/settings_editor/refresh_rate_by_kind_editor.rs:91-91
Timestamp: 2026-05-23T14:57:43.820Z
Learning: When reviewing Rust code that calls `ToggleableKeyboard::new(parent_rect: Rectangle, number_mode: bool)` (from `crates/core/src/view/toggleable_keyboard.rs`), treat the second argument as `number_mode` (numeric/number-layout mode), not as an initial visibility flag. In the constructor implementation, `visible` is initialized to `false` and the keyboard starts hidden; only the numeric layout changes based on `number_mode` (e.g., for numeric inputs like `refresh rate u8`). Therefore, do not flag `ToggleableKeyboard::new(rect, true)` as “making the keyboard start visible.”
Applied to files:
crates/core/src/document/mupdf_sys.rs
🔇 Additional comments (12)
crates/build-deps/src/build/kobo/mupdf.rs (1)
26-30: LGTM!crates/build-deps/src/build/kobo.rs (2)
166-191: LGTM!
280-352: LGTM!.gitmodules (1)
48-48: LGTM!thirdparty/mupdf (1)
1-1: Verify the submodule commit matches MuPDF 1.28.0.This diff exposes only the new commit pointer, so confirm that
205b8cf43551279d1215e88fe2845c5d595bade9is the intended upstream MuPDF v1.28.0 revision and contains no unintended changes before merging.crates/build-deps/src/versions.rs (1)
16-16: LGTM!crates/core/src/document/mupdf_sys.rs (1)
25-25: LGTM!build-scripts/mupdf/kobo.patch (1)
1-40: LGTM! Adding-std=gnu99to the KoboCFLAGSis a reasonable fix given the older cross-toolchain, and the-DFZ_ENABLE_*defines here don't clash with the native-sideXCFLAGS_NATIVE/XCFLAGS_SHAREDcomposition since the Kobo path only injects-DHAVE_WEBP=1viaXCFLAGS(percrates/build-deps/src/build/kobo/mupdf.rs).crates/build-deps/src/build/mupdf.rs (1)
1-9: LGTM! Clean extraction of sharedmake libsargs/flags with idiomatic iterator usage.Also applies to: 26-52
crates/build-deps/src/build/native.rs (1)
21-23: LGTM! The rest of the marker-aware rebuild flow (ensure_native_artifacts,remove_build_dir,XCFLAGS_NATIVE/make_libs_invocationwiring inbuild_mupdf_native) looks correct and consistent with the shared build helpers.Also applies to: 84-99, 136-143, 361-410
crates/build-deps/src/markers.rs (2)
56-80: LGTM!marker_matches_commitcleanly captures the "non-empty and equal" semantics, andis_builtdelegates to it correctly.
119-175: LGTM! Solid coverage of match/mismatch/empty-marker cases.
Change-Id: 4e2aefa515535b542854d434284124d7 Change-Id-Short: vlxplkpuyuuw
This PR contains the following updates:
1.27.2→1.28.0Release Notes
ArtifexSoftware/mupdf (thirdparty/mupdf)
v1.28.0Compare Source
Configuration
📅 Schedule: (UTC)
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
🔕 Ignore: Close this PR and you won't be reminded about these updates again.
This PR was generated by Mend Renovate. View the repository job log.