Skip to content
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

Replace archive/tar fork with use of archive/tar #175

Merged
merged 1 commit into from
Sep 8, 2020
Merged

Replace archive/tar fork with use of archive/tar #175

merged 1 commit into from
Sep 8, 2020

Conversation

TBBle
Copy link
Contributor

@TBBle TBBle commented Jul 16, 2020

Since Go 1.6, archive/tar has gained public support for custom PAX Records, so there's no need to maintain a fork of this library anymore.

We do end up with a fork of the PAX timestamp parsing/formatting functions, but they are much less likely to change.

Copying the PAX time-parsing/formatting headers, and use of modTime as a fallback for creationTime inspired by containerd's reimplementation of this functionality, but recreated by hand.

Since Go 1.6, archive/tar has gained public support for custom PAX
Records, so there's no need to maintain a fork of this library anymore.

We do end up with a fork of the PAX timestamp parsing/formatting
functions, but they are much less likely to change.

Signed-off-by: Paul "TBBle" Hampson <[email protected]>
@TBBle
Copy link
Contributor Author

TBBle commented Jul 16, 2020

As it happens, this change will make it feasible to write sparse blocks to the stream in GNU tar PAX sparse format 1.0, which archive/tar will happily decompress for us. I'm not sure how valuable that would be though, particularly if gziping the tarballs anyway.

Edit: I just noticed that the OCI spec forbids sparse files anyway.

@TBBle
Copy link
Contributor Author

TBBle commented Jul 27, 2020

It looks like this fix (or further hacks on the forked archive/tar) is needed to fix support for files larger than 8g when importing a layer, as we hit golang/go#15573. See microsoft/Windows-Containers#43 (comment) for reproduction case using hcsshim and moby/moby#40444 for a reproduction in Docker Desktop for Windows.

@thaJeztah
Copy link
Contributor

@ambarve @kevpar PTAL

@TBBle
Copy link
Contributor Author

TBBle commented Aug 28, 2020

Is there anything I can do to get this advanced, @ambarve, @kevpar? I know it says '37 files', but that's 34 deletions, one addition, and only two changed files, most of which was search-and-replace. I'd really like to land this, so we can start vendoring it into downstream users and resolve some of the referenced issues.

@ambarve
Copy link
Contributor

ambarve commented Sep 6, 2020

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants