Restore symlink mtime on Linux after extracting an entry - #381
Merged
Merged
Conversation
The symlink path in `FileManager.setAttributes(_:ofItemAtURL:traverseLink:)` has been an Apple-only no-op since the file's `#if` was added: every non-Darwin platform took the `return`, dropping both stored permission bits *and* the stored modification date for symlink entries. Permission bits genuinely don't apply on Linux — the kernel ignores them on symlinks no matter what `lchmod` does — so leaving that half of the codepath off matches reality. The modification date is a different story. `lutimes(2)` writes to the symlink itself rather than chasing the link, has shipped in Glibc since 2.6, and the existing `setSymlinkModificationDate` helper already calls it correctly. The only reason it wasn't being invoked on Linux is that the call site was Apple-gated. Add a Linux branch that calls `setSymlinkModificationDate` (and only that) so extracting a ZIP archive on Linux preserves stored symlink mtimes — same fidelity Apple platforms already get. Bionic and Windows fall through to the existing no-op path; Bionic ships `lutimes` only behind `__INTRODUCED_IN(26)`, and Windows has no equivalent symlink-targeted utimes call, so leave them alone here. Comment on the `#else` branch is updated to reflect what's actually left out (Bionic + Windows, not "non-Darwin POSIX" generally).
weichsel
approved these changes
May 10, 2026
| // Since non-Darwin POSIX platforms ignore permissions on symlinks and swift-corelibs-foundation | ||
| // currently doesn't support setting the modification date, this codepath is currently a no-op | ||
| // on these platforms. | ||
| // Bionic and Windows lack a fully equivalent symlink-targeted |
Owner
There was a problem hiding this comment.
I'd just say "Other platforms" here (vs. Windows & Bionic)
| // since 2.6. swift-corelibs-foundation's `setAttributes` doesn't | ||
| // round-trip `.modificationDate` for symlinks today, so we go | ||
| // straight to the libc syscall here to preserve mtime parity with | ||
| // Apple platforms when extracting. |
Owner
There was a problem hiding this comment.
Changes look good.
Can you please move the
testInvalidSymlinkCompressionMethodErrorConditions
testSymlinkModificationDateTransferErrorConditions
tests from the darwinOnlyTests to the allTests list. They are also relevant on Linux now.
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
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.
Summary
Linux extraction quietly drops the modification date of every symlink entry today. The fix is to call the existing
setSymlinkModificationDatehelper from the Linux branch instead of taking the blanket Apple-only no-op.Background
FileManager.setAttributes(_:ofItemAtURL:traverseLink:)splits on platform:The comment is half right:
lchmod(2)does, so leaving the permission half off matches reality.lutimes(2)writes to the symlink itself rather than chasing the target, and Glibc has shipped it since 2.6. The existingsetSymlinkModificationDatealready calls it correctly. The only reason it wasn't running on Linux is that the entire call site was Apple-gated. The "swift-corelibs-foundation doesn't support …" half of the comment is true forFileManager.setAttributes(_:ofItemAtPath:), but ZIPFoundation is going straight to libc anyway, so it doesn't depend on that.So Linux extraction loses symlink mtimes on every archive, which sometimes matters for tooling that walks extracted trees by timestamp.
Change
Add a
#elseif os(Linux)branch to the call site that calls onlysetSymlinkModificationDate. No definition changes —setSymlinkModificationDateitself was already correct and accessible, just unused.Bionic falls through to the no-op because its
lutimesis__INTRODUCED_IN(26)(not safe to lean on at low NDK targets) and Windows has no equivalent symlink-targeted utimes call. Both can be revisited in a follow-up — out of scope here.Validation
Built clean on macOS (Swift 6.3, Xcode 26) —
Build complete!. Fullswift testrun shows the same 2 pre-existing failures the unmodifieddevelopmentbranch has (testCreateZIP64ArchiveWithLargeSize— aVersionenum mirror-string format mismatch unrelated to anything here); all other 129 tests pass.I don't have a Linux box wired up to run the test target on, but the code path is straightforward —
setSymlinkModificationDatealready had test coverage on Apple platforms that exercises exactly thislutimescall. Happy to add a Linux-only XCTest if you'd prefer (theattributesround-trip would just need to assert on the extracted symlink'sDateinstead of returning early).Out of scope
utimensat(AT_FDCWD, path, ×, AT_SYMLINK_NOFOLLOW)is the obvious workaround for Android once a maintainer is happy bumping the supported NDK floor; Windows would need aSetFileInformationByHandlecall against an opened reparse-point handle.Related: weichsel/ZIPFoundation#380 is my Android compile fix. This PR is independent and doesn't depend on it landing first.