-
Notifications
You must be signed in to change notification settings - Fork 205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
deps: update tar-rs to handle very large uid/gid in image unpack #1387
Conversation
@adamqqqplay , a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/88468 |
Codecov Report
@@ Coverage Diff @@
## master #1387 +/- ##
==========================================
- Coverage 46.31% 46.31% -0.01%
==========================================
Files 122 122
Lines 38071 38071
Branches 38071 38071
==========================================
- Hits 17634 17632 -2
Misses 19513 19513
- Partials 924 926 +2 |
@@ -107,3 +107,6 @@ backend-s3 = ["nydus-storage/backend-s3"] | |||
|
|||
[workspace] | |||
members = ["api", "builder", "clib", "rafs", "storage", "service", "utils"] | |||
|
|||
[patch.crates-io] | |||
tar = { git = "https://github.com/nydusaccelerator/tar-rs.git" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a comment here explaining why we need to use the forked code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, comment added.
@adamqqqplay , The CI test is completed, please check result:
Congratulations, your test job passed! |
37d9e01
to
94b32a0
Compare
@adamqqqplay , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/88487 |
@adamqqqplay , The CI test is completed, please check result:
Congratulations, your test job passed! |
It's important to have a regression test for a tricky issue like this. But I think we can go without it for now. |
update tar-rs to support read large uid/gid from PAX extensions to fix very large UIDs/GIDs (>=2097151, limit of USTAR tar) lost in PAX style tar during unpack. Signed-off-by: Qinqi Qu <quqinqi@linux.alibaba.com>
94b32a0
to
98548ba
Compare
@adamqqqplay , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/88660 |
@liubogithub I added a test image at: ghcr.io/dragonflyoss/image-service/pax-uid-test to verify this is fixed. |
@adamqqqplay , The CI test is completed, please check result:
Congratulations, your test job passed! |
E2E is successfully tested here: https://github.com/adamqqqplay/image-service/actions/runs/5746313149/job/15575642277 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Relevant Issue (if applicable)
alexcrichton/tar-rs#332
Details
update tar-rs to support read large uid/gid from PAX extensions to fix very large UIDs/GIDs (>=2097151, limit of USTAR tar) lost in PAX style tar during unpack.
Types of changes
What types of changes does your PullRequest introduce? Put an
x
in all the boxes that apply:Checklist
Go over all the following points, and put an
x
in all the boxes that apply.