Skip to content

Prevent panic when sysroot cannot be computed #61459

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
Jun 30, 2019
Merged

Prevent panic when sysroot cannot be computed #61459

merged 1 commit into from
Jun 30, 2019

Conversation

GuillaumeGomez
Copy link
Member

Fixes #61377.

cc @rotty @rust-lang/rustdoc

r? @Manishearth

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 2, 2019
ollie27
ollie27 suggested changes Jun 2, 2019
View reviewed changes
@GuillaumeGomez GuillaumeGomez force-pushed the fix-rustdoc-sysroot-panic branch from 4ca49f4 to 670557a Compare June 2, 2019 14:45
@GuillaumeGomez GuillaumeGomez force-pushed the fix-rustdoc-sysroot-panic branch from 670557a to a6b9406 Compare June 9, 2019 15:51
@GuillaumeGomez
Copy link
Member Author

Updated!

bjorn3
bjorn3 reviewed Jun 10, 2019
View reviewed changes
@GuillaumeGomez GuillaumeGomez force-pushed the fix-rustdoc-sysroot-panic branch from a6b9406 to ab277a4 Compare June 10, 2019 09:04
@GuillaumeGomez
Copy link
Member Author

Updated.

QuietMisdreavus
QuietMisdreavus approved these changes Jun 14, 2019
View reviewed changes
Copy link
Member

@QuietMisdreavus QuietMisdreavus left a comment

Choose a reason for hiding this comment

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

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
Copy link
Member Author

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

@GuillaumeGomez
Copy link
Member Author

Well, meanwhile, this PR is ready to go.

@bors: r=ollie27,bjorn3,QuietMisdreavus

@GuillaumeGomez
Copy link
Member Author

Maybe it'll work this time?

@bors: r=ollie27,bjorn3,QuietMisdreavus

@bors
Copy link
Collaborator

bors commented Jun 29, 2019

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

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 29, 2019
@bors
Copy link
Collaborator

bors commented Jun 30, 2019

⌛ Testing commit ab277a4 with merge fd7f48b...

bors added a commit that referenced this pull request Jun 30, 2019
@bors
Auto merge of #61459 - GuillaumeGomez:fix-rustdoc-sysroot-panic, r=ol…
fd7f48b 
…lie27,bjorn3,QuietMisdreavus

Prevent panic when sysroot cannot be computed

Fixes #61377.

cc @rotty @rust-lang/rustdoc

r? @Manishearth
@bors
Copy link
Collaborator

bors 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 This PR was explicitly merged by bors. label Jun 30, 2019
@bors bors merged commit ab277a4 into rust-lang:master Jun 30, 2019
@bjorn3
Copy link
Member

bjorn3 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 fix-rustdoc-sysroot-panic branch June 30, 2019 15:10
@GuillaumeGomez
Copy link
Member Author

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
Reviewers

@bjorn3 bjorn3 bjorn3 left review comments

@ollie27 ollie27 ollie27 requested changes

@QuietMisdreavus QuietMisdreavus QuietMisdreavus approved these changes

Assignees

@Manishearth Manishearth

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rustdoc should not assume current_exe() is infallible
7 participants
@GuillaumeGomez @bors @bjorn3 @QuietMisdreavus @ollie27 @Manishearth @rust-highfive