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

Prevent panic when sysroot cannot be computed #61459

Merged
merged 1 commit into from Jun 30, 2019

Conversation

Projects
None yet
7 participants
@GuillaumeGomez
Copy link
Member

commented Jun 2, 2019

Show resolved Hide resolved src/librustdoc/test.rs Outdated
Show resolved Hide resolved src/librustdoc/test.rs Outdated

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:fix-rustdoc-sysroot-panic branch from 4ca49f4 to 670557a Jun 2, 2019

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:fix-rustdoc-sysroot-panic branch from 670557a to a6b9406 Jun 9, 2019

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

commented Jun 9, 2019

Updated!

Show resolved Hide resolved src/librustdoc/test.rs Outdated

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:fix-rustdoc-sysroot-panic branch from a6b9406 to ab277a4 Jun 10, 2019

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2019

Updated.

@QuietMisdreavus
Copy link
Member

left a comment

LGTM! Looks like passing None all the way down to rustc lets it use the proper calculations on OpenBSD.

I did some archaeology to see the history of the sysroot parameter. It looks like the current_exe logic was taken out of core.rs way back in #19161, but that PR didn't touch test.rs, and the logic was carried through during the various IO refactorings over time. This brings the doctest functionality in-line with the rest of rustdoc, and the rest of rustc.

We don't have OpenBSD builders, do we? I'm wondering if there's a way we can test for this. 😕

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2019

@rust-lang/infra Can we check on OpenBSD one way or another?

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

commented Jun 29, 2019

Well, meanwhile, this PR is ready to go.

@bors: r=ollie27,bjorn3,QuietMisdreavus

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

commented Jun 29, 2019

Maybe it'll work this time?

@bors: r=ollie27,bjorn3,QuietMisdreavus

@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2019

📌 Commit ab277a4 has been approved by ollie27,bjorn3,QuietMisdreavus

@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2019

⌛️ Testing commit ab277a4 with merge fd7f48b...

bors added a commit that referenced this pull request Jun 30, 2019

Auto merge of #61459 - GuillaumeGomez:fix-rustdoc-sysroot-panic, r=ol…
…lie27,bjorn3,QuietMisdreavus

Prevent panic when sysroot cannot be computed

Fixes #61377.

cc @rotty @rust-lang/rustdoc

r? @Manishearth
@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2019

☀️ Test successful - checks-azure, checks-travis, status-appveyor
Approved by: ollie27,bjorn3,QuietMisdreavus
Pushing fd7f48b to master...

@bors bors added the merged-by-bors label Jun 30, 2019

@bors bors merged commit ab277a4 into rust-lang:master Jun 30, 2019

3 checks passed

Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
pr Build #20190629.47 succeeded
Details
@bjorn3

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2019

@GuillaumeGomez Why did you include me in the r= list? Every other time I commented on a PR, I didnt get included.

@GuillaumeGomez GuillaumeGomez deleted the GuillaumeGomez:fix-rustdoc-sysroot-panic branch Jun 30, 2019

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2019

Well, you reviewed it so it seemed ok for me to include you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.