Skip to content

Replace deprecated tempdir with tempfile #1903

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 28, 2019
Merged

Replace deprecated tempdir with tempfile #1903

merged 1 commit into from
Jul 28, 2019

Conversation

@tesuji
Copy link
Contributor

@tesuji tesuji commented Jun 20, 2019

No description provided.

@kinnison
Copy link
Contributor

kinnison commented Jun 21, 2019

Fundamentally I'm good with this, though there are test failures, so please resolve those before I review deeply.

@tesuji
Copy link
Contributor Author

tesuji commented Jun 21, 2019 •
edited
Loading 

failures:
---- remove_toolchain_then_add_again stdout ----
thread 'remove_toolchain_then_add_again' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 1142, kind: Other, message: "An attempt was made to create more links on a file than the file system supports." }', src\libcore\result.rs:997:5
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

I'm now convinced that something on our side has messed up.

@tesuji tesuji force-pushed the tempfile branch 4 times, most recently from 01bd8ef to 95eb0c4 Compare July 1, 2019 09:04
@bors
Copy link
Contributor

bors commented Jul 2, 2019

☔ The latest upstream changes (presumably #1923) made this pull request unmergeable. Please resolve the merge conflicts.

@kinnison
Copy link
Contributor

kinnison commented Jul 28, 2019

Hey @lzutao Could you rebase this against current master and see how it looks?

kinnison
kinnison approved these changes Jul 28, 2019
View reviewed changes
Copy link
Contributor

@kinnison kinnison left a comment

Choose a reason for hiding this comment

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

I think this is okay. I'd like a second opinion before merge.

rbtcollins
rbtcollins approved these changes Jul 28, 2019
View reviewed changes
Copy link
Contributor

@rbtcollins rbtcollins left a comment

Choose a reason for hiding this comment

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

I don't like the new API so much, I have to say.

@rbtcollins rbtcollins merged commit c104e3b into rust-lang:master Jul 28, 2019
@tesuji tesuji deleted the tempfile branch July 29, 2019 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@rbtcollins rbtcollins rbtcollins approved these changes

@kinnison kinnison kinnison approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tesuji @kinnison @bors @rbtcollins