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

parsing header.Name (filepath) for subfolders #7

Merged
merged 5 commits into from
Jul 11, 2016

Conversation

ThomasDotCodes
Copy link
Contributor

While trying to unrar an archive with subfolders, the call to header.IsDir evaluates as false because the folder structure does not exist yet. This is a less than ideal way to parse the path for subfolders and create them as needed before header.IsDir is checked against.

While trying to unrar an archive with subfolders, the call to header.IsDir evaluates as false because the folder structure does not exist yet. This is a less than ideal way to parse the path for subfolders and create them as needed before header.IsDir is checked against.
@@ -37,6 +38,23 @@ func Unrar(source, destination string) error {
} else if err != nil {
return err
}

pathComponents := strings.Split(header.Name, "/")
Copy link
Owner

@mholt mholt Jul 8, 2016

Choose a reason for hiding this comment

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

You could do filepath.Dir(header.Name) and remove that if pi == len(pathComponents) - 1 check below.

Copy link
Owner

Choose a reason for hiding this comment

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

So is "/" always used in rar files, even on Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have been testing on windows exclusively so far, so I can confirm that the forward-slash "/" is used on windows. Haven't tested on other platforms yet.

@mholt
Copy link
Owner

mholt commented Jul 8, 2016

Thanks for finding a fix! You beat me to it. 😄

// check to see if the path exists already
if stat, err := os.Stat(destination + path); err != nil || !stat.IsDir() {
// make the directory
mkdir(destination + path)
Copy link
Owner

Choose a reason for hiding this comment

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

This function will make all parent folders necessary to make the folder at destination+path so it should only need be called once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is that header only gets set in the for loop, and this needs to be called after header is set, but before header.isDir is evaluated.

@ThomasDotCodes
Copy link
Contributor Author

Made another update that prevents subfolder creation from being called multiple times. Still not 100% ideal solution, since user might want to unrar the same file twice, and folders may have been deleted in between calls to unrar. Open to suggestions on better implementation.

@ThomasDotCodes
Copy link
Contributor Author

ThomasDotCodes commented Jul 8, 2016

Updated again with a simpler (better) way of making sure makeSubfolders() is only called once per call to Unrar()

@mholt
Copy link
Owner

mholt commented Jul 9, 2016

Hey, nice improvements!

I think I was able to fix it simply with this just before the writeNewFile line (although this could be in an else for the IsDir check):

err = mkdir(filepath.Dir(filepath.Join(destination, header.Name)))
if err != nil {
    return err
}

Although I'm not 100% sure what the rar file you sent me is supposed to look like since I didn't ask for its zip equivalent. But I saw strange behavior before my change and it looks good now. Can you see if this works for you? If so I prefer it because it's much simpler. If this works, you can still get the credit by updating your PR.

@ThomasDotCodes
Copy link
Contributor Author

I can confirm that does work. Updated my branch accordingly.

@mholt mholt merged commit d0c7c0a into mholt:master Jul 11, 2016
@mholt
Copy link
Owner

mholt commented Jul 11, 2016

Cool, thanks for your help!

@ThomasDotCodes ThomasDotCodes deleted the unrar-subfolders-patch branch July 13, 2016 17:41
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.

None yet

2 participants