Preserve symlinks when archiving build contexts#2
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f5af02e03
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a98e694771
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4499f054e
ℹ️ 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".
| let archivedPathsByHostPath = entryInfo.reduce(into: [URL: [URL]]()) { result, info in | ||
| result[info.pathOnHost.standardizedFileURL, default: []].append(info.pathInArchive) | ||
| } |
There was a problem hiding this comment.
Include the context root in absolute-symlink rewrites
Fresh evidence here is that archivedPathsByHostPath is reduced only from entryInfo, and entryInfo never contains source itself when archiving a directory. A symlink like sub/self -> /path/to/context therefore still falls through _rewriteArchivedAbsoluteSymlinkTarget, is emitted with the client-absolute target, and breaks again in staged/remote tar builds after extraction instead of pointing back at the extracted root (.. in this example).
Useful? React with 👍 / 👎.
275885b to
f120d30
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f120d30e07
ℹ️ 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".
| let targetPath = URL(fileURLWithPath: symlinkTarget) | ||
| .standardizedFileURL | ||
| .resolvingSymlinksInPath() | ||
| .path |
There was a problem hiding this comment.
Preserve archived symlinks instead of resolving through them
If an absolute symlink points to another symlink that is also in the archive (for example, link -> /context/alias and alias -> target.txt), calling resolvingSymlinksInPath() here collapses the target to the final file before lookup. Archiver.uncompress will then recreate link pointing straight at target.txt, so the extracted build context no longer preserves the original symlink topology. That changes behavior for builds that copy or inspect symlinks rather than dereferencing them.
Useful? React with 👍 / 👎.
Problem
While validating fresh image builds for the new Compose-style plugin against
resq-fullstack, we reproduced a core build failure before BuildKit ever got to the Dockerfile logic:container buildcould fail while archiving the build context.The root cause is that the archiver treated non-regular filesystem entries like regular files. For symbolic links and directories, it could still derive a payload size from filesystem metadata and then try to stream file bytes for an entry type that should only have a header.
Real repositories hit this easily because build contexts often contain symlinks from virtualenvs, package managers, or worktree layouts. In that state, fresh builds fail in the client archiving step with errors like
unable to write data to the archive.Why this belongs in core
This is a plain
container buildbug. It is not Compose-specific; the plugin just made it easier to reproduce on a realistic workload.What this changes
0Validation
swift test --filter ArchiverTests