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

[Bugfix] Switch tar crate to make tar.gz decompression work #28

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kageiit
Copy link

@kageiit kageiit commented May 25, 2024

Fixes #27

@kageiit
Copy link
Author

kageiit commented May 25, 2024

happy to add tests. The fixtures currently come from some temp repo with no instructions. Im not sure what is a good way to generate a test fixture for the PAX 1.0 file format - https://www.gnu.org/software/tar/manual/html_node/PAX-1.html

@thoughtpolice
Copy link

thoughtpolice commented Aug 22, 2024

I just ran into #27. Is there anything preventing this from being merged? This is a pretty important fix since it's somewhat impossible to tell ahead of time what tarfiles will work and what won't.

/cc @bigfootjon I suppose?

@bolinfest
Copy link
Contributor

I suspect tests are the long pole here? /cc @zertosh

Also, the fixtures still point to a personal repo (which is something we needed to do when this repo was still private prior to launch so it didn't break the internal Meta CI), but ideally that would also be changed now, e.g.:

GITHUB_REPO = "https://github.com/zertosh/dotslash_fixtures"

.arg("https://github.com/zertosh/dotslash_fixtures/raw/462625c6bf2671439dce66bd5bc40b05f2ed8819/pack.tar.gz")

@facebook-github-bot
Copy link
Contributor

@zertosh has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@bigfootjon
Copy link
Member

@kageiit can you rebase this PR?

@facebook-github-bot
Copy link
Contributor

@zertosh has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@zertosh
Copy link
Member

zertosh commented Sep 11, 2024

I am conflicted here. I can repro the problem, and I can also repro that switching to binstall_tar fixes at least the ruff extracting issue. BUT, I don't have a lot of confidence in binstall_tar. It looks like they just grabbed the latest alexcrichton/tar-rs and applied (blindly?) many outstanding PRs. I don't know about the quality or how vetted anything has been. I also tried alexcrichton/tar-rs#298 by itself and it addresses the ruff issue too.

@bolinfest, wdyt?

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

Successfully merging this pull request may close these issues.

Some tar.gz archives dont decompress correctly
6 participants