Refactor rustdoc #52257

steveklabnik · 2018-07-11T14:13:15Z

This is based on #52194 and so shouldn't be merged until it gets merged.

Now that plugin functionality has been removed, let's kill PluginManager. Additionally, rustdoc now follows the standard cargo layout instead of dumping everything at the top level.

r? @rust-lang/rustdoc

QuietMisdreavus

(extremely over-dramatic voice) Expunge all traces of its foul legacy, make sure there is no memory left to honor!

...but first, a couple comments.

QuietMisdreavus · 2018-07-11T14:40:44Z

src/librustdoc/src/lib.rs

                                             .position(|&(p, ..)| {
                                                 p == *pass
                                             }) {
-                Some(i) => passes::PASSES[i].1,
+                                                 */


We can probably scrap this zombie code, yeah?

Ugh, yes, I meant to and forgot. :)

QuietMisdreavus · 2018-07-11T14:46:34Z

src/librustdoc/Cargo.toml

-[lib]
-name = "rustdoc"
-path = "lib.rs"
-


Why move all the files into an inner src directory? Does anything else in-tree do this?

Because it's the default.

Does anything else in-tree do this?

I'm not sure, but I'm of the opinion it should ;)

If it weren't for the error index generator using bits of librustdoc, we may be able to get away with moving all this code into src/tools/rustdoc, where this pattern would definitely fit in. As-is, nothing else directly in src/ is structured like this. I'm personally indifferent to the change, but it's going to break (and be broken by) every rustdoc PR. (And several people seem interested in cleaning up rustdoc right now...)

Heck, we could possibly get away with it anyway, since a Cargo project can have both a lib and a bin. We'd just need to point bootstrap and the error index generator (and anything else that may point at librustdoc directly) at the new folder.

I think it's better to keep it. At least some tests rely on it (like error index rendering iirc).

If we're moving all the source files we should at least rename librustdoc to just rustdoc.

QuietMisdreavus · 2018-07-11T14:53:05Z

Also, the removal of PluginResult clashes with #52211, though moving everything into an inner src directory clashes with literally every open rustdoc PR, so pick your poison.

GuillaumeGomez · 2018-07-11T15:51:37Z

src/librustdoc/src/lib.rs

+            eprintln!("WARNING: --plugins no longer functions; see CVE-2018-1000622");
+        }
+
+        if !plugin_path.is_none() {


Since this is printing the sale as the previous if condition, could you merge the both please?

They don't; the name of the command is in the warning.

Good point, my bad!

GuillaumeGomez · 2018-07-11T15:51:51Z

src/librustdoc/src/lib.rs

+            eprintln!("WARNING: --plugin-path no longer functions; see CVE-2018-1000622");
+        }
+
+        info!("Executing passes");


Why this add?

It wasn't added; it was in the old code. It used to say passes/plugins

Oh sorry! Damn diff...

GuillaumeGomez · 2018-07-11T15:52:07Z

src/librustdoc/src/lib.rs

                None => {
                    error!("unknown pass {}, skipping", *pass);
+


Why adding this line?

Oh, this is just how I'd write it. I find the former cramped.

QuietMisdreavus · 2018-07-19T20:55:22Z

@steveklabnik With #52194 merged, can you rebase this PR? I'd like to get this in so i can try to rework the passes system.

steveklabnik · 2018-07-23T13:20:23Z

Rebased and removed the zombie code :)

QuietMisdreavus

Thanks for cleaning this up! I'm mostly okay with this now, but i want @GuillaumeGomez and/or @ollie27 to sign off on this too, especially the code layout change.

QuietMisdreavus · 2018-07-23T14:07:22Z

src/librustdoc/Cargo.toml

-[lib]
-name = "rustdoc"
-path = "lib.rs"
-


If it weren't for the error index generator using bits of librustdoc, we may be able to get away with moving all this code into src/tools/rustdoc, where this pattern would definitely fit in. As-is, nothing else directly in src/ is structured like this. I'm personally indifferent to the change, but it's going to break (and be broken by) every rustdoc PR. (And several people seem interested in cleaning up rustdoc right now...)

Heck, we could possibly get away with it anyway, since a Cargo project can have both a lib and a bin. We'd just need to point bootstrap and the error index generator (and anything else that may point at librustdoc directly) at the new folder.

rust-highfive · 2018-07-23T14:34:12Z

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[01:12:41] travis_fold:end:stage0-rustdoc-themes

[01:12:41] travis_time:end:stage0-rustdoc-themes:start=1532356443941579459,finish=1532356445052331239,duration=1110751780

[01:12:41] thread 'main' panicked at 'read_dir failed: Os { code: 2, kind: NotFound, message: "No such file or directory" }', libcore/result.rs:945:5
[01:12:41] 
[01:12:41] 
[01:12:41] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/rustdoc-themes" "/checkout/obj/build/bootstrap/debug/rustdoc" "/checkout/src/librustdoc/html/static/themes"
[01:12:41] expected success, got: exit code: 101
[01:12:41] expected success, got: exit code: 101
[01:12:41] 
[01:12:41] 
[01:12:41] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:12:41] Build completed unsuccessfully in 0:31:52
[01:12:42] make: *** [check] Error 1
[01:12:42] Makefile:58: recipe for target 'check' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0012b193
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

steveklabnik · 2018-07-23T14:58:37Z

especially the code layout change.

yeah, I mean I guess you'd want to do the whole compiler at once, so if yinz hate that change, I'm happy to remove it. It's always felt off to me, but I can also understand consistency within the compiler itself

QuietMisdreavus · 2018-07-24T20:02:48Z

I asked @GuillaumeGomez on discord about the way this moves the source files, and he disagreed with it.

We talked more about it, and i think it would be better to instead move the files into src/tools/rustdoc/src instead, alongside the stub main.rs that's already there. However, it would be better to do this in a separate PR. I really want to have this pass refactor landed so i can tweak them some more. >_>

steveklabnik · 2018-07-24T20:45:49Z

Okay, dropped the file moves.

QuietMisdreavus · 2018-07-24T21:28:09Z

Thanks so much! I'm calling this good to go!

@bors r+

bors · 2018-07-24T21:28:10Z

📌 Commit 0bf2a06 has been approved by QuietMisdreavus

QuietMisdreavus · 2018-07-24T21:29:38Z

@bors r-

Oops, i guess i should wait on travis first.

QuietMisdreavus · 2018-07-24T22:15:41Z

@bors r+

bors · 2018-07-24T22:15:42Z

📌 Commit 0bf2a06 has been approved by QuietMisdreavus

bors · 2018-07-24T22:25:49Z

⌛ Testing commit 0bf2a06 with merge 46804ef...

Refactor rustdoc This is based on #52194 and so shouldn't be merged until it gets merged. Now that plugin functionality has been removed, let's kill PluginManager. Additionally, rustdoc now follows the standard cargo layout instead of dumping everything at the top level. r? @rust-lang/rustdoc

bors · 2018-07-25T00:31:53Z

☀️ Test successful - status-appveyor, status-travis
Approved by: QuietMisdreavus
Pushing 46804ef to master...

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 11, 2018

QuietMisdreavus reviewed Jul 11, 2018

View reviewed changes

GuillaumeGomez reviewed Jul 11, 2018

View reviewed changes

bjorn3 mentioned this pull request Jul 13, 2018

Misc rustdoc changes #52211

Merged

pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 23, 2018

steveklabnik force-pushed the refactor-rustdoc branch from d8a4f42 to efaac8c Compare July 23, 2018 13:19

QuietMisdreavus reviewed Jul 23, 2018

View reviewed changes

QuietMisdreavus mentioned this pull request Jul 23, 2018

Cleanup some rustdoc code #52628

Merged

steveklabnik force-pushed the refactor-rustdoc branch from efaac8c to ac35901 Compare July 24, 2018 20:44

steveklabnik added 2 commits July 24, 2018 16:44

remove pluginmanager

86d2ba4

remove zombie code

0bf2a06

steveklabnik force-pushed the refactor-rustdoc branch from ac35901 to 0bf2a06 Compare July 24, 2018 20:45

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 24, 2018

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 24, 2018

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 24, 2018

bors merged commit 0bf2a06 into rust-lang:master Jul 25, 2018

pnkfelix mentioned this pull request Oct 9, 2018

error: internal compiler error: Accessing (*_310) with the kind Write(Move) shouldn't be possible #54597

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor rustdoc #52257

Refactor rustdoc #52257

steveklabnik commented Jul 11, 2018 •

edited

Loading

QuietMisdreavus left a comment

QuietMisdreavus Jul 11, 2018

steveklabnik Jul 11, 2018

QuietMisdreavus Jul 11, 2018

steveklabnik Jul 11, 2018

QuietMisdreavus Jul 23, 2018

GuillaumeGomez Jul 23, 2018

ollie27 Jul 23, 2018

QuietMisdreavus commented Jul 11, 2018

GuillaumeGomez Jul 11, 2018

steveklabnik Jul 11, 2018

GuillaumeGomez Jul 11, 2018

GuillaumeGomez Jul 11, 2018

steveklabnik Jul 11, 2018

GuillaumeGomez Jul 11, 2018

GuillaumeGomez Jul 11, 2018

steveklabnik Jul 11, 2018

QuietMisdreavus commented Jul 19, 2018

steveklabnik commented Jul 23, 2018

QuietMisdreavus left a comment

QuietMisdreavus Jul 23, 2018

rust-highfive commented Jul 23, 2018

steveklabnik commented Jul 23, 2018

QuietMisdreavus commented Jul 24, 2018

steveklabnik commented Jul 24, 2018

QuietMisdreavus commented Jul 24, 2018

bors commented Jul 24, 2018

QuietMisdreavus commented Jul 24, 2018

QuietMisdreavus commented Jul 24, 2018

bors commented Jul 24, 2018

bors commented Jul 24, 2018

bors commented Jul 25, 2018

Refactor rustdoc #52257

Refactor rustdoc #52257

Conversation

steveklabnik commented Jul 11, 2018 • edited Loading

QuietMisdreavus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuietMisdreavus commented Jul 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuietMisdreavus commented Jul 19, 2018

steveklabnik commented Jul 23, 2018

QuietMisdreavus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rust-highfive commented Jul 23, 2018

steveklabnik commented Jul 23, 2018

QuietMisdreavus commented Jul 24, 2018

steveklabnik commented Jul 24, 2018

QuietMisdreavus commented Jul 24, 2018

bors commented Jul 24, 2018

QuietMisdreavus commented Jul 24, 2018

QuietMisdreavus commented Jul 24, 2018

bors commented Jul 24, 2018

bors commented Jul 24, 2018

bors commented Jul 25, 2018

steveklabnik commented Jul 11, 2018 •

edited

Loading