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

Fix recursive zip extraction. #20

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

AnneKitsune
Copy link

Hello there!

Really cool crate, I like it a lot! I'll probably open more pull requests as I go.

Copy link
Owner

@jaemk jaemk left a comment

Choose a reason for hiding this comment

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

awesome thanks! Just a few small thing

io::copy(&mut file, &mut output)?;
let outpath = into_dir.join(file.sanitized_name());

if (&*file.name()).ends_with('/') {
Copy link
Owner

Choose a reason for hiding this comment

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

can you use outpath.is_dir() instead to be windows compatible

Copy link
Author

Choose a reason for hiding this comment

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

That was copy pasted from zip-rs example, but yes I can.

Copy link
Author

Choose a reason for hiding this comment

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

Doing that causes the program to think everything is a file. I think this is mostly due to zip-rs handling folders as files and only making the difference using the ending /.

Copy link
Owner

Choose a reason for hiding this comment

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

hm, that's unfortunate... Do you know if that's also the case on windows? Could you maybe setup a test zip file with nested dirs and add windows-cfg'd/linux-cfg'd tests that check this?

Copy link
Author

Choose a reason for hiding this comment

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

I think the reason for the is_dir to not work is because of the sanitized_name call, which might be removing the trailing slash.

I'm a bit(very) short on time currently, so I don't want to spend a lot of time on this bug fix writing unit tests. Sorry!

src/lib.rs Outdated
let outpath = into_dir.join(file.sanitized_name());

if (&*file.name()).ends_with('/') {
fs::create_dir_all(&outpath).unwrap();
Copy link
Owner

Choose a reason for hiding this comment

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

can you use the ? operator instead of unwraps

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

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