Replace ZIPFoundation with libarchive - #53
Merged
Conversation
Swaps the ZIPFoundation dependency — pinned to an untagged `development` HEAD for a Windows `#import` fix (weichsel/ZIPFoundation#380) — for marcprux/swift-archive 3.8.9, which vendors libarchive as C sources behind a thin Swift wrapper. The `GzipSupport` trait is enabled: it is off upstream by default, and without zlib libarchive cannot inflate DEFLATE entries, i.e. every real `.docx`/`.epub`. Reading (DOCX parts, DOCX media, the iWork `.iwa` container) goes through swift-archive's `ArchiveReader`. libarchive is a sequential reader with no lookup by name, so each call streams the container once and keeps the entries it wants instead of seeking per part. Writing `.docx` and `.epub` goes through the new SwiftTextZip target, which drives the raw `archive_*` C API rather than `ArchiveWriter`. `ArchiveEntry.apply()` always sets uid/gid/mtime, and libarchive emits a `ux` extra block for uid/gid and a `UT` block for any timestamp. Setting none of the three buys two things no public `ArchiveWriter` API can: * A bare `mimetype` header. EPUB's OCF container forbids any extra field there — the rule that keeps `application/epub+zip` at the fixed offset 38 reading systems sniff. * Reproducible output. libarchive derives the MS-DOS timestamp with `localtime()`, where ZIPFoundation used `gmtime()`, so stamping would make archives differ between machines in different timezones. Entries now date to the 1980 ZIP epoch everywhere; the real date still lives in the OPF's `dcterms:modified`. `.docx` gains this too — it was previously stamped with `Date()` and never reproducible. iWork writing stays on the hand-rolled `StoredZipWriter`: `.pages` needs stored entries with sizes and CRC in the local header, which libarchive's always-streaming ZIP writer cannot emit. Since SwiftTextZip is shared by two independently-gated modules, it lists the Archive product twice under DOCX and under EPUB — Swift 6.2's SwiftPM requires ALL traits in one `.when(traits:)`, so OR semantics have to be spelled as separate dependencies. Platform floor rises to macOS 13 / iOS 15 / tvOS 15 / watchOS 10, from macOS 12 / iOS 13 / tvOS 13 / watchOS 6: SwiftPM requires a package to be at least as new as any product it links, and swift-archive's own floor is macOS 13 / iOS 15. `platforms:` is package-wide, so the pure-Swift targets inherit it even though they don't link libarchive. Verified: 496 tests pass; epubcheck reports 0 errors / 0 warnings on generated EPUBs; Python's zipfile `testzip()` passes on generated `.docx`/`.epub`; `textutil` (Apple's own OOXML reader) parses the `.docx`; `.docx` output is byte-identical under TZ=UTC and TZ=Pacific/Auckland and EPUB entry headers likewise; real-world `.pages` and `.docx` read correctly; iOS builds; the portable subset and the HTML-only trait subset still resolve without libarchive. Output shrinks ~9%: real zlib beats the fixed-Huffman encoder the interim in-house writer would have used. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 278e08b0f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`write(_:to:)` built the archive beside the destination, then read the finished file back into a `Data` and wrote every byte a second time. On an image-heavy `.docx`/`.epub` that doubles peak memory on top of the entry data the callers already hold. libarchive streams straight to disk, so the staging file is already the final artifact — swap it in with `replaceItemAt` (or `moveItem` when there is nothing to replace). Staging as a sibling of the destination keeps it on the same volume, making that a rename rather than a copy, and it stays atomic either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Requirements section said "Swift 5.9+", but the manifest declares `swift-tools-version:6.1` — a 5.9 toolchain can't parse it at all, so the claim was never true. State 6.1+, and note that CI builds on 6.3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`.when(traits:)` is any-of as of the 6.3 tools version, so `SwiftTextZip` no longer needs the same product listed twice to mean "DOCX or EPUB" — one condition covers both. Drops the 6.2-era commentary from the other trait-gated targets too, which was explaining a workaround that no longer applies. Safe for every CI job: macOS and iOS run Xcode latest-stable (Swift 6.3.3 as of the last green run on main), Linux is swift:6.3-jammy, and Windows and Android pin 6.3.1. The macOS job's comment claiming latest-stable is 6.2.x was stale; corrected. Also restores Package.resolved. A local `SWIFTTEXT_PORTABLE_ONLY=1 swift test` rewrites the lockfile down to the portable dependency set, and re-resolving from that pruned state had silently bumped swift-argument-parser 1.7.0 → 1.8.2 and XMLKit 1.0.0 → 1.0.1. The lockfile now differs from main by the ZIPFoundation → swift-archive swap alone. Verified: 496 tests; portable subset (153 tests); SwiftLint --strict; iOS build; and each trait subset resolves correctly — DOCX alone and EPUB alone each pull libarchive in and compile SwiftTextZip, while HTML alone resolves neither swift-archive nor argument-parser. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Bumped to Checked before bumping: the macOS job's comment claiming Also restores Re-verified: 496 tests, portable subset (153), SwiftLint 🤖 Posted by Claude Code |
The file was copied from another project and never adapted: it told agents
to run `swift test` for "BrickCore" changes and to build
`SwiftLEGO.xcodeproj` with the `SwiftLEGO` scheme. SwiftText has no Xcode
project at all, so that command could never have worked here.
Its "ship for iOS 26 and newer" line was SwiftLEGO's floor too. Taken at
face value it is wrong by eleven major versions — the package has always
targeted low deployment targets on purpose (macOS 12 / iOS 13 before the
libarchive migration, macOS 13 / iOS 15 now), with Linux, Windows and
Android CI jobs. An automated reviewer already cited that line against
this PR, so it was actively misleading readers, not just stale.
Dropped the pluralization rule as well: it prescribes SwiftUI's
`Text("^[\(n) part](inflect: true)")`, and there is no SwiftUI anywhere in
this package.
Kept what is true (structured concurrency; Swift Testing only — verified:
zero XCTest imports, 63 files using Swift Testing) and added what an agent
here actually needs: the real deployment floors and why they are set, the
cross-platform constraint on Apple-only frameworks, the 6.3 toolchain
requirement, the trait-gating convention for dependencies, and the
`SWIFTTEXT_PORTABLE_ONLY` subset — including that it rewrites
Package.resolved, which silently bumped two dependencies earlier in this
branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #46 (the Zip-container half; the PDF-deflate options in that issue are untouched).
Why
Package.swiftpinned ZIPFoundation to an untaggeddevelopmentHEAD — the only commit carrying the Windows#import→#includefix (weichsel/ZIPFoundation#380), without which the DOCX/Pages readers can't build on Windows. Upstream still hasn't tagged a release with it. This swaps in marcprux/swift-archive 3.8.9, which vendors libarchive as C sources behind a thin Swift wrapper, so there's no system libarchive to provision and we're back on a version tag.GzipSupportis enabled explicitly — it's off by default upstream, and without zlib libarchive can't inflate DEFLATE entries, i.e. every real.docx/.epub.What changed
Reading — DOCX parts, DOCX media, the iWork
.iwacontainer — goes throughArchiveReader. libarchive is a sequential reader with no lookup by name, so each call streams the container once and keeps the entries it wants rather than seeking per part.Writing
.docxand.epubgoes through the newSwiftTextZiptarget, which drives the rawarchive_*C API rather thanArchiveWriter.ArchiveEntry.apply()unconditionally sets uid/gid/mtime, and libarchive emits auxextra block for uid/gid and aUTblock for any timestamp. Setting none of the three buys two things no publicArchiveWriterAPI can express:mimetypeheader. EPUB's OCF container forbids any extra field on that entry — the rule that keepsapplication/epub+zipat the fixed offset 38 reading systems sniff.localtime(), where ZIPFoundation usedgmtime(). Stamping would make archives differ byte-for-byte between machines in different timezones, and pre-compensating the mtime would just move that machine-dependence into theUTfield. Entries now date to the 1980 ZIP epoch everywhere; the real date still lives in the OPF'sdcterms:modified, which is what readers surface..docxgains this too — it was previously stamped with ZIPFoundation'sDate()default and was never reproducible.iWork writing stays on the hand-rolled
StoredZipWriter..pagesneeds stored entries with sizes and CRC in the local header plus Apple's exact per-entry metadata; libarchive always writes streaming entries with a trailing data descriptor (ZIP_ENTRY_FLAG_LENGTH_AT_ENDis unconditional — it never knows the CRC when the local header goes out). That file is untouched by this PR.Output got ~9% smaller: real zlib compresses better than anything hand-rolled.
Reviewer notes
platforms:is package-wide, so the pure-Swift targets inherit it even though they don't link libarchive. README's Requirements section now states this explicitly.The doubled product entry in
SwiftTextZipis deliberate:The target is shared by two independently-gated modules, and Swift 6.2's SwiftPM requires all traits in one
.when(traits:)to be enabled (6.3 changed this to any-of). Spelling OR as separate dependencies keeps 6.2 working. Verified this doesn't leak: a consumer withtraits: ["HTML"]still resolves only swift-markdown/cmark/XMLKit, so the README's lean-resolution claim holds.Known and accepted: libarchive's zip reader has
#ifdef DEBUGtracing, and SwiftPM passes-DDEBUG=1to C targets in debug builds, so a handful ofHeader id 0x…lines go to stderr per archive that carries extra fields (Apple's.pagesdo; our own output doesn't). Release builds are silent, stdout is unaffected, and text output is byte-identical either way. Fixing it needs a one-line#undef DEBUGin swift-archive'sconfig_spm.h— can't be done downstream, sincecSettingsdon't apply to a dependency's target.CI: Linux now installs
zlib1g-dev+libssl-dev(libarchive links-lzand-lcryptothere). Windows already provisioned vcpkg zlib asz.lib, which is exactly whatCArchive's.linkedLibrary("z")wants, so that job needed only comment updates.Verification
--strictclean; iOSxcodebuildsucceeds; portable subset (SWIFTTEXT_PORTABLE_ONLY=1) builds and runs its 153 testsmimetypestored,extraLen0, media type at offset 38zipfile.testzip()(CRC-checks every entry) passes on generated.docxand.epubtextutil— Apple's own OOXML reader — parses the generated.docx.docxbyte-identical underTZ=UTCvsTZ=Pacific/Auckland; EPUB entry headers likewise. New test walks the central directory asserting no timestamps and no extra fields, so this can't silently regress.pagesand.docxread correctly through the release CLI🤖 Generated with Claude Code