build: normalize input archives before Darwin libtool merge#23
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR adds a pre-normalization step to Key points:
Confidence Score: 5/5Safe to merge; the fix is correct and the only remaining finding is a P2 style suggestion about adding the -D flag to match the PR description's stated intent. The change is small (29 lines), well-scoped to Darwin builds only, and has been validated end-to-end by the author. The shell invocation is idiomatic and handles spaces correctly. The single P2 observation (missing -D on ranlib) does not affect correctness — only build reproducibility. src/build/LibtoolStep.zig — minor: ranlib -D flag omitted vs. PR description intent. Important Files Changed
Sequence DiagramsequenceDiagram
participant ZigBuild as Zig Build
participant NormStep as RunStep(ranlib i)
participant LibtoolRun as RunStep(libtool)
participant FS as Build Cache
ZigBuild->>NormStep: for each source .a (index i)
NormStep->>FS: /bin/cp source to i-libghostty-fat.a
NormStep->>FS: /usr/bin/ranlib i-libghostty-fat.a
NormStep-->>LibtoolRun: normalized .a (LazyPath)
LibtoolRun->>FS: libtool -static -o libghostty-fat.a all normalized inputs
LibtoolRun-->>ZigBuild: output libghostty-fat.a
Reviews (1): Last reviewed commit: "build: normalize input archives before D..." | Re-trigger Greptile |
| run_step.addArgs(&.{ | ||
| "/bin/sh", | ||
| "-c", | ||
| "/bin/cp \"$1\" \"$2\" && /usr/bin/ranlib \"$2\"", |
There was a problem hiding this comment.
Missing
-D flag for deterministic output
The PR description explicitly states the goal is to use ranlib -D to produce a deterministic archive ("deterministic (-D strips timestamps)"), but the implementation omits the flag. Without -D, ranlib embeds the current wall-clock time into the archive's symbol table. While Zig's input-hashed build cache won't spuriously re-run this step because of non-deterministic output, the resulting archives won't be reproducible across different machines or timestamps — which matters for content-addressed caches, supply-chain tooling, or downstream consumers that verify archive hashes.
| "/bin/cp \"$1\" \"$2\" && /usr/bin/ranlib \"$2\"", | |
| "/bin/cp \"$1\" \"$2\" && /usr/bin/ranlib -D \"$2\"", |
This is the same change as ghostty-org#11999 and is required for the build.
Background
Zig 0.15.2 can produce macOS
.aarchives where some 64-bit Mach-O members are only 4-byte aligned inside the archive. Recent Applelibtool -staticdoes not handle that layout correctly: it emits "not 8-byte aligned" warnings and, in the failing case, silently drops those members when creating the combined static library.In Ghostty, this happened in the Darwin libtool merge step that builds
libghostty-fat.a. The x86_64 inputlibghostty.astill contained the expectedlibghostty_zcu.oand about 97 exported_ghostty_symbols, but afterlibtool -staticthe output archive contained only 4 SIMD symbols becauselibghostty_zcu.ohad been discarded.Fix
Normalize each input archive by copying it and running
ranlib -Don the copy before passing it tolibtool. This rewrites the archive into a layout Apple's linker tools accept without flattening members through the filesystem or changing the archive's semantic contents.Why This Approach
ar x→ar rcscan fix the warnings but is a broader, riskier transformation (duplicate member basenames can collide, non-.omembers can be lost, member order can change).ranlib -Drewrites the archive in place, preserves all archive members, and is deterministic (-Dstrips timestamps).Validation
After applying the normalization step, a clean
zig buildsucceeded and the final macOS xcframework archive contained all expected_ghostty_symbols in both the x86_64 and arm64 slices.Summary by cubic
Normalize input
.aarchives before the Darwinlibtool -staticmerge to prevent dropped 64‑bit Mach‑O objects and preserve expected_ghostty_symbols inlibghostty-fat.a.This fixes build failures seen with recent Xcode
libtoolwhen using Zig 0.15.2 archives.libtoolmisalignment handling by copying each input archive and running/usr/bin/ranlibon the copy before merging.normalizeArchiveinsrc/build/LibtoolStep.zig; cleanzig buildnow preserves expected symbols for both x86_64 and arm64.Written for commit bd75907. Summary will update on new commits.
Summary by CodeRabbit