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

Ensure that intermediate directories exist when unpacking an entry #1098

Merged
merged 1 commit into from
May 17, 2017

Conversation

ranma42
Copy link
Contributor

@ranma42 ranma42 commented May 8, 2017

The unpack function assumes that the directory in which the file is
being extracted exists, while most tar tools will automatically
create the intermediate directories if they are missing.

This would have avoided #1092.

@brson
Copy link
Contributor

brson commented May 8, 2017

@bors r+

Note though that we still cannot remove the directory entries from the tarballs produced by Rust, because they need to be compatible with older rustups. I'm not sure how long a suitable deprecation period is.

@bors
Copy link
Contributor

bors commented May 8, 2017

📌 Commit 5ae90c4 has been approved by brson

@bors
Copy link
Contributor

bors commented May 8, 2017

⌛ Testing commit 5ae90c4 with merge 4f98cc0...

bors added a commit that referenced this pull request May 8, 2017
Update tar and use new safe unpacking function

The `unpack_in` function automatically handles the creation of the
directories and validates the path name, but it is only available
since version 0.4.11 of the `tar` crate.

This is the rustup side of fixing #1092.
@ranma42
Copy link
Contributor Author

ranma42 commented May 8, 2017

Yes, I have no plans for removing them and the Rust port of rust-installer will keep them as well for the time being. Removing them was just convenient when sorting the paths.

@bors
Copy link
Contributor

bors commented May 8, 2017

💔 Test failed - status-appveyor

@brson
Copy link
Contributor

brson commented May 8, 2017

Legit errors.

@ranma42 ranma42 changed the title Update tar and use new safe unpacking function Ensure that intermediate directories exist when unpacking an entry May 9, 2017
@ranma42
Copy link
Contributor Author

ranma42 commented May 9, 2017

It is easier to fix without updating tar. Updated title and first post to get reasonable commit messages upon merging.

The `unpack` function assumes that the directory in which the file is
being extracted exists, while most `tar` tools will automatically
create the intermediate directories if they are missing.

This would have avoided rust-lang#1092.
@brson
Copy link
Contributor

brson commented May 9, 2017

@bors r+

I can imagine this repeated syscall wanting to be optimized someday, but today I'm not too worried. rustup generally needs to be optimized.

@bors
Copy link
Contributor

bors commented May 9, 2017

📌 Commit ab40997 has been approved by brson

@bors
Copy link
Contributor

bors commented May 10, 2017

⌛ Testing commit ab40997 with merge 6a5f6f2...

bors added a commit that referenced this pull request May 10, 2017
Ensure that intermediate directories exist when unpacking an entry

The `unpack` function assumes that the directory in which the file is
being extracted exists, while most `tar` tools will automatically
create the intermediate directories if they are missing.

This would have avoided #1092.
@bors
Copy link
Contributor

bors commented May 10, 2017

💔 Test failed - status-appveyor

@ranma42
Copy link
Contributor Author

ranma42 commented May 11, 2017

Is the failure spurious? The non-merge AppVeyor build succeeded

@Diggsey
Copy link
Contributor

Diggsey commented May 17, 2017

Yes - @bors r=brson retry

@bors
Copy link
Contributor

bors commented May 17, 2017

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented May 17, 2017

📌 Commit ab40997 has been approved by brson

@bors
Copy link
Contributor

bors commented May 17, 2017

⌛ Testing commit ab40997 with merge 3605b7e...

bors added a commit that referenced this pull request May 17, 2017
Ensure that intermediate directories exist when unpacking an entry

The `unpack` function assumes that the directory in which the file is
being extracted exists, while most `tar` tools will automatically
create the intermediate directories if they are missing.

This would have avoided #1092.
@bors
Copy link
Contributor

bors commented May 17, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: brson
Pushing 3605b7e to master...

@bors bors merged commit ab40997 into rust-lang:master May 17, 2017
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

4 participants