You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While building out a Docker Compose-style plugin and validating it against our own real development stack, we ran into two classes of problems in the build-context path:
correctness issues in archive generation, especially around symlinks
very high client-side overhead while preparing and streaming build contexts
This PR keeps the earlier archive/symlink correctness fixes and adds a follow-up performance pass in the same codepath.
Problems addressed
Archive correctness
The previous implementation had several issues around symlink handling and archive identity:
symlink targets were not fully reflected in the advertised archive digest
absolute symlinks pointing back into the archived source tree could remain client-absolute after unpack
symlink targets that traversed a symlinked ancestor could fail to rewrite to the archived target
These issues could break staged/remote builds or make archive digests unstable for cache/integrity purposes.
Build-context performance
The build-context sync path was also doing unnecessary work on large repositories:
BuildFSSync computed the included file set, then Archiver.compress re-enumerated the source tree again
archive creation used relatively expensive Foundation metadata and stream APIs per file
path normalization work in BuildFSSync added avoidable overhead during context preparation
In practice, this showed up as very slow [internal] load build context times while testing against a real repo.
What changed
Correctness
preserve symlink semantics during archive/unarchive
include effective symlink target information in the archive digest
rewrite archived absolute symlinks to archive-relative paths when their targets are also present in the archive
canonicalize absolute symlink targets through symlinked ancestors before archive lookup
Performance
let BuildFSSync hand Archiver the already-computed entry list instead of forcing a second full-tree archive walk
replace heavier per-file Foundation metadata/stream access with lighter POSIX-based file status and reads
reduce unnecessary URL/path normalization in the build-context path
@mazdak Are these fixes and optimizations generally useful? If so, why can't they be applied to the archiver implementation in the containerization library, as opposed creating a separate implementation just for container?
Are these fixes and optimizations generally useful? If so, why can't they be applied to the archiver implementation in the containerization library, as opposed creating a separate implementation just for container?
Yes, they are indeed generally useful. Let me see about adding them to the Containerization lib and I'll circle back
Thanks for looking into this. Another thing that would be helpful would be to provide concrete examples of what was wrong for each of the "archive correctness" points and what the fix will do instead. For anything that fails in container today due to these issues, does the same failure exist in Docker, Podman, or colima?
Thanks for looking into this. Another thing that would be helpful would be to provide concrete examples of what was wrong for each of the "archive correctness" points and what the fix will do instead. For anything that fails in container today due to these issues, does the same failure exist in Docker, Podman, or colima?
It's a been a coupe of weeks, so I am going to dig and find out what the issue was. But no, this problem does not exist in docker.
I have now made two PRs and can probably close this one:
Thanks for looking into this. Another thing that would be helpful would be to provide concrete examples of what was wrong for each of the "archive correctness" points and what the fix will do instead. For anything that fails in container today due to these issues, does the same failure exist in Docker, Podman, or colima?
I also just added more context on the original issue in #1391
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
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.
Type of Change
Motivation and Context
While building out a Docker Compose-style plugin and validating it against our own real development stack, we ran into two classes of problems in the build-context path:
This PR keeps the earlier archive/symlink correctness fixes and adds a follow-up performance pass in the same codepath.
Problems addressed
Archive correctness
The previous implementation had several issues around symlink handling and archive identity:
These issues could break staged/remote builds or make archive digests unstable for cache/integrity purposes.
Build-context performance
The build-context sync path was also doing unnecessary work on large repositories:
BuildFSSynccomputed the included file set, thenArchiver.compressre-enumerated the source tree againBuildFSSyncadded avoidable overhead during context preparationIn practice, this showed up as very slow
[internal] load build contexttimes while testing against a real repo.What changed
Correctness
Performance
BuildFSSynchandArchiverthe already-computed entry list instead of forcing a second full-tree archive walkTesting