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

rustdoc: add option to abort the process on markdown differences #46883

Merged
merged 2 commits into from
Dec 29, 2017

Conversation

QuietMisdreavus
Copy link
Member

In the efforts of keeping the std docs free of markdown warnings, this PR adds a stopgap measure to make sure the CI fails if it detects a markdown difference. It does this by adding a new unstable flag to rustdoc, --deny-render-differences, which bootstrap then passes to rustdoc when documenting std and friends.

The implementation is... probably not the cleanest option. It currently adds an extra branch after it prints the markdown warnings, which just prints a final line and calls ::std::process::abort(1). I did it like this because if it just panics regularly, it looks like an ICE, an even though html::render::run returns a Result, that Result is also just expected immediately, generating the same problem. This way bypasses the panic handler at the top of the thread and looks like a proper failure. Since i don't have a real error Handler there, this is the best i can do without pulling in a real error system for rustdoc.

This PR is blocked on #46853, which will fix the rendering differences that were present on master when i started this branch.

@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

(rust_highfive has picked a reviewer for you, use r? to override)

@GuillaumeGomez
Copy link
Member

r=me once CI passed.

@bors
Copy link
Contributor

bors commented Dec 20, 2017

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

@kennytm kennytm added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Dec 20, 2017
@kennytm
Copy link
Member

kennytm commented Dec 21, 2017

#46853 has been merged.

@QuietMisdreavus
Copy link
Member Author

Just rebased atop it. Let's see what travis says.

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 21, 2017
@@ -256,6 +256,9 @@ pub fn opts() -> Vec<RustcOptGroup> {
unstable("sort-modules-by-appearance", |o| {
o.optflag("", "sort-modules-by-appearance", "sort modules by where they appear in the \
program, rather than alphabetically")
unstable("deny-render-differences", |o| {
Copy link
Member

Choose a reason for hiding this comment

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

Missing }), above this line 😒

[00:43:34] error: incorrect close delimiter: `]`
[00:43:34]    --> /checkout/src/librustdoc/lib.rs:263:5
[00:43:34]     |
[00:43:34] 263 |     ]
[00:43:34]     |     ^
[00:43:34]     |
[00:43:34] note: unclosed delimiter
[00:43:34]    --> /checkout/src/librustdoc/lib.rs:256:52
[00:43:34]     |
[00:43:34] 256 |         unstable("sort-modules-by-appearance", |o| {
[00:43:34]     |                                                    ^
[00:43:34] 
[00:43:34] error: incorrect close delimiter: `]`
[00:43:34]    --> /checkout/src/librustdoc/lib.rs:263:5
[00:43:34]     |
[00:43:34] 263 |     ]
[00:43:34]     |     ^
[00:43:34]     |
[00:43:34] note: unclosed delimiter
[00:43:34]    --> /checkout/src/librustdoc/lib.rs:256:17
[00:43:34]     |
[00:43:34] 256 |         unstable("sort-modules-by-appearance", |o| {
[00:43:34]     |                 ^
[00:43:34] 
[00:43:35] error: expected one of `.`, `;`, `?`, `}`, or an operator, found `unstable`
[00:43:35]    --> /checkout/src/librustdoc/lib.rs:259:9
[00:43:35]     |
[00:43:35] 258 |                                                          program, rather than alphabetically")
[00:43:35]     |                                                                                               - expected one of `.`, `;`, `?`, `}`, or an operator here
[00:43:35] 259 |         unstable("deny-render-differences", |o| {
[00:43:35]     |         ^^^^^^^^ unexpected token
[00:43:35] 
[00:43:35] error: aborting due to 3 previous errors
[00:43:35] 
[00:43:35] error: Could not compile `rustdoc`.

@QuietMisdreavus
Copy link
Member Author

Whoops, fixed that now. Travis is green.

@QuietMisdreavus
Copy link
Member Author

@bors r=GuillaumeGomez

@bors
Copy link
Contributor

bors commented Dec 29, 2017

📌 Commit dfbb946 has been approved by GuillaumeGomez

@bors
Copy link
Contributor

bors commented Dec 29, 2017

⌛ Testing commit dfbb946 with merge 966fdf1...

bors added a commit that referenced this pull request Dec 29, 2017
rustdoc: add option to abort the process on markdown differences

In the efforts of keeping the std docs free of markdown warnings, this PR adds a stopgap measure to make sure the CI fails if it detects a markdown difference. It does this by adding a new unstable flag to rustdoc, `--deny-render-differences`, which bootstrap then passes to rustdoc when documenting std and friends.

The implementation is... probably not the cleanest option. It currently adds an extra branch after it prints the markdown warnings, which just prints a final line and calls `::std::process::abort(1)`. I did it like this because if it just panics regularly, it looks like an ICE, an even though `html::render::run` returns a Result, that Result is also just `expect`ed immediately, generating the same problem. This way bypasses the panic handler at the top of the thread and looks like a proper failure. Since i don't have a real error Handler there, this is the best i can do without pulling in a real error system for rustdoc.

This PR is blocked on #46853, which will fix the rendering differences that were present on master when i started this branch.
@bors
Copy link
Contributor

bors commented Dec 29, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: GuillaumeGomez
Pushing 966fdf1 to master...

@bors bors merged commit dfbb946 into rust-lang:master Dec 29, 2017
@MaloJaffre MaloJaffre mentioned this pull request Dec 29, 2017
10 tasks
@QuietMisdreavus QuietMisdreavus deleted the faildown branch February 26, 2018 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants