Various features I need for bootc-delta - #75
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #75 +/- ##
==========================================
+ Coverage 59.51% 68.94% +9.42%
==========================================
Files 10 10
Lines 1035 1111 +76
==========================================
+ Hits 616 766 +150
+ Misses 305 232 -73
+ Partials 114 113 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for Docker/OCI-style whiteout files and adds functionality to ignore specific source path prefixes during delta analysis. It refactors the analysis logic into a reusable SourceAnalysis structure, facilitating concurrent operations and pre-computation. A new flag for a custom temporary directory was also added. Feedback was provided regarding an inefficiency in how opaque whiteouts are processed, suggesting a single-pass approach to improve performance when handling multiple whiteouts.
We want the default to not be /tmp, because that is often a tmpfs and delta files can be large. NOTE: On win32, we can't use the hardcoded value Signed-off-by: Alexander Larsson <alexl@redhat.com>
We can't just use strings.HasPrefix(), because we want an ignored prefix of /foo to ignore /foo/bar, but not /foobar. Signed-off-by: Alexander Larsson <alexl@redhat.com>
This adds a new AnalyzeSources() function that gathers all the information for a set of sources, and DiffWithSources() that does the diff against those sources. With this setup you can do diffs against multiple tar files with the same set of sources without repeating the source analysis. This is useful for the case of diffing multi-layer OCI images. The old Diff() API is still there, and calls AnalyzeSources() in the background. Signed-off-by: Alexander Larsson <alexl@redhat.com>
If this is specified, any files with this prefix will not be used as a delta source, which is useful in case the source files set is partial at apply time. In particular, in my work with bootc deltas, this is useful for layer files inside /sysroot/ostree, as they are not all stored on the bootc system. Signed-off-by: Alexander Larsson <alexl@redhat.com>
If the tar files passed in are docker/oci style layers, then it makes sense to allow applying the whiteouts in them. That way, if a file is removed in a layer, and we have only the complete image mounted as a source of delta info (which is what we have in the bootc case), then we won't use this removed file as a delta source. Signed-off-by: Alexander Larsson <alexl@redhat.com>
e0b6e6b to
8436733
Compare
This creates multi-file diffs, including with whiteouts and tries them both with and without whiteouts. Signed-off-by: Alexander Larsson <alexl@redhat.com>
348c824 to
e8b8330
Compare
djach7
left a comment
djach7
left a comment
There was a problem hiding this comment.
LGTM, one small question about a validation opportunity. Not a blocker, though.
Signed-off-by: Alexander Larsson <alexl@redhat.com>
With containers/tar-diff#75 merged we can now rely on the standard repo.
|
Why was this squashed when merged? I carefully made individually extracted independent commits, so that git log and blame can show what changed and for what reason. Now its all just a bunch of somewhat unrelated changes in one huge change? |
3 participants
I got some feedback on my bootc-delta work and had to change the approach a bit (see containers/oci-delta#1). Now I diff against the full container image, not just the ostree objects.
This means we now have to filter out /sysroot/ostree (rather than before to only look at that).
Also, since we're diffing against the resulting image rather than the individual layers we need to handle docker layer whiteouts.
Also, for better performance on many layers, I split the API up so that we can share the source analysis between different layers.