Support concatenated gzip (mgzip/pigz) layers in ztoc#1871
Conversation
| @@ -228,6 +228,134 @@ func TestDecompressWithGzipHeaders(t *testing.T) { | |||
| } | |||
There was a problem hiding this comment.
needs a e2e test using pigz
There was a problem hiding this comment.
I'll try to change TestDecompressConcatenatedGzip to use pigz (from os.exec) instead of using the current BuildConcatenatedGzipTar method for compression.
There was a problem hiding this comment.
We can add integration test perhaps to test container with actual pigz tests, in some places we might be using some images from ecr public which we can populate with desired test images as needed.
There was a problem hiding this comment.
I think I can get a minimal integration test working, which created a basic docker image locally with pigz compression and uses that to verify that ztoc get-file work fine as well as fuse mounts are present.
I think for now that should be enough and in future we can add some custom ecr image in soci-workshop-examples repository. This would probably needed to be done by some maintainer with access :)
I will update the PR soon.
There was a problem hiding this comment.
@Shubhranshu153 I have update the unit test to use pigz for compression as well as added a minimal integration test which generates a pigz based image manually, indexes it and verifies the content of it.
Also I have manually ran and verified the working of the image which was resulting in 0 span ztoc creation in #1834
There was a problem hiding this comment.
we can create a image with pigz in soci-workshop-example and have that test.
Its ok to have feature extension pending for future pr but ideally should have the required test for a new feature.
|
Overall looks correct to me. Reading more on gzip to see if i am missing something. Would like some e2e tests here though as we dont know if it works correctly through the both creation and execution lifecycle with a pigz compressor. |
7d69a74 to
680c963
Compare
Signed-off-by: Praful Gupta <prafulgupta6@gmail.com>
680c963 to
8ce5f10
Compare
|
Hey @Shubhranshu153 just nudging regarding this :) is there something else I should be supporting with here? 😄 |
|
I'll take a look myself as well — sorry this slipped under my radar |
Issue #, if available:
Fixes #1834. The C-level zinfo code only processed the first gzip member, causing all ztoc entries to map to span 0 for mgzip-compressed layers. This adds handling for multiple concatenated gzip members in all three functions: index generation, file-based extraction, and buffer-based extraction.
Description of changes:
generate_zinfo_from_fpby detecting additional data after Z_STREAM_END and resetting inflate state for the next memberextract_data_from_fp,extract_data_from_buffer) by manually skipping the 8-byte gzip trailer on the first member boundary (raw inflate mode), then switching to gzip mode (windowBits=47) which auto-consumes subsequent trailersTesting performed:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.