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

Add 256 filename support (prefix + name) and add support for space terminated numbers and non-null terminated names #10

Conversation

schnoberts1
Copy link
Contributor

... as discussed previously. As before, maybe this isn't idiomatic. Also, I'm not sure I like how I've done the loop to skip over directories. I wonder if it would be better to skip over everything but regular files. It'd make the logic easier and if the assumption that all tar headers have correct size fields is true it would mean we skip things we don't care about rather than stop. That being said, perhaps that is undesirable as in your code we know that we can't parse the tar file completely.

Very much a first cut and looking for opinions.

@schnoberts1 schnoberts1 force-pushed the add-support-for-non-null-terminated-strings-and-256-length-filenames branch from 5f4747c to 46770c5 Compare August 31, 2023 14:34
@schnoberts1 schnoberts1 force-pushed the add-support-for-non-null-terminated-strings-and-256-length-filenames branch from 8e6bb88 to 29e3a28 Compare August 31, 2023 14:58
Copy link
Owner

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Will have a closer look soon.

examples/alloc_feature.rs Show resolved Hide resolved
src/archive.rs Outdated Show resolved Hide resolved
src/archive.rs Outdated Show resolved Hide resolved
@schnoberts1
Copy link
Contributor Author

I noticed a few of the checks failed. The Linux ones just cancelled, the windows one fails to check out the repo due to:

Error: error: unable to create file tests/01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234/ABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJ: Filename too long

I can remove this and simply leave the test tar file if you like.

…rmally these aren't needed as the .tar files are used in the source. The tests themselves note what files they depend on.
…grading them past

the Rust 1.60 compiler. Example from an indirectly dependent crate on Linux:

error: package `rustix v0.38.11` cannot be built because it requires rustc 1.63 or newer, while the currently active rustc version is 1.60.0
@phip1611
Copy link
Owner

phip1611 commented Sep 11, 2023

Hi! Sorry, I'm kind of busy. I hope, I can look on this soon.

@schnoberts1
Copy link
Contributor Author

schnoberts1 commented Sep 11, 2023 via email

@phip1611 phip1611 merged commit 14097e0 into phip1611:main May 3, 2024
@phip1611
Copy link
Owner

phip1611 commented May 3, 2024

Thank you so much for your contribution, @schnoberts1. Sorry, I totally forgot about it. Great work!

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.

2 participants