Add `cargo rustdoc` for passing arbitrary flags to rustdoc #1977

Manishearth · 2015-09-09T08:50:39Z

Quite useful for doing cargo rustdoc -- --no-defaults to get a doc package with private items.

This is similar to cargo rustc, except that the flags passed to cargo rustdoc are applied to both the main package and dependencies.

A friend of mine was looking for this feature, couldn't find a way to do it without directly calling rustdoc. This would probably be useful for using the full configurability of rustdoc in cargo-applications.

r? @alexcrichton

rust-highfive · 2015-09-09T08:50:51Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

steveklabnik · 2015-09-09T17:38:20Z

conceputally, 👍 from me.

alexcrichton · 2015-09-11T18:08:36Z

src/bin/rustdoc.rs

+    -q, --quiet              No output printed to stdout
+    --color WHEN             Coloring: auto, always, never
+
+By default the documentation for the local package and all dependencies is


I think this may want to customize this text instead of just copy/pasting from cargo help doc, for example this should probably start out by saying that it is intended to configure the invocation of rustdoc on one crate and not mention much about dependencies up here at the top.

alexcrichton · 2015-09-11T18:14:34Z

cc @brson, @wycats, a new Cargo subcommand, but it fits well within the pattern of cargo rustc so I'm fine landing without an RFC.

brson · 2015-09-11T18:15:45Z

sgtm

Manishearth · 2015-09-12T06:08:46Z

So, I actually want the subcommand to pass the rustdoc flags to all dependencies. Rustdoc output would be weird if only one crate was compiled with a set of flags.

For cargo rustc it makes sense to limit it to the final crate, since it's mostly for tweaking linker args and whatnot. But for cargo rustdoc we want to finally get a uniform chunk of documentation, which means the flags should be passed down.

I thought this PR did do that, turns out it doesn't, I'll see if I can make it work so that the args are passed to upstream deps too.

(This is why I didn't add a check for multiple targets -- this should behave exactly like cargo doc, just that every rustdoc invocation gets the extra flags)

alexcrichton · 2015-09-13T17:51:50Z

I would personally not want to land this with subtly different behavior than cargo rustc when both commands seem quite similar. Even with rustdoc I can imagine there are use cases of only passing flags to the top-level crate rather than all dependencies, although I'd certainly believe that it's more rare.

Configuration of rustdoc for all crates in the dependency graph is certainly desired, but I think that wants a more principled solution than just an extra command which passes all the flags everywhere. E.g. this is a recipe for all projects having their documentation building step be something like cargo rustdoc -- <ten obscure flags>, where instead the options could be stored in the manifest (for example) and the idiom would still be just cargo doc.

Manishearth · 2015-09-14T02:24:32Z

Alright, fixed so that it mirrorrs rustc

E.g. this is a recipe for all projects having their documentation building step be something like cargo rustdoc -- , where instead the options could be stored in the manifest (for example) and the idiom would still be just cargo doc.

So there are multiple use cases here. One is for the default build, and one is for specialized builds. For example, cargo build might be the default, but I may want to occasionally be able to run cargo rustc -- -C codegen_units=10 or something during development. Or cargo rustc -- -Z extra-plugins=plugin -L /path/to/plugin -D foo. These aren't part of the build step, these are tweaks I'm running during development. Editing a file seems ... unwieldy, since these aren't part of the regular build.

I'd prefer if we had both a cargo-config based solution, and a terminal-based solution. The simplest one seems to be allowing cargo rustc and cargo rustdoc to have a --flag-all flag, which passes the provided flag to all deps (instead of the top one), and overrides any rustc-flags/rustdoc-flags in the cargo config.

alexcrichton · 2015-09-14T16:16:26Z

Yes I agree that passing custom flags shouldn't always involve editing files, but I was mostly just saying that this should follow cargo rustc, discussion for this kind of feature probably shouldn't happen on this PR and it would likely require an RFC.

alexcrichton · 2015-09-14T16:18:40Z

src/bin/bench.rs

@@ -83,6 +83,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
                                            &options.flag_test,
                                            &options.flag_example,
                                            &options.flag_bench),
+            extra_rustdoc_args: vec![],


Cargo tends to favor Vec::new() over vec![] currently

Manishearth · 2015-09-14T22:59:40Z

(addressed)

alexcrichton · 2015-09-16T14:48:30Z

src/bin/rustdoc.rs

+<opts>... will all be passed to the final rustdoc invocation, not any of the
+dependencies. Note that rustdoc will still unconditionally receive
+arguments such as -L, --extern, and --crate-type, and the specified <opts>...
+will simply be added to the compiler invocation.


s/compiler/rustdoc/

bors · 2015-09-22T02:00:44Z

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

Manishearth · 2015-09-26T17:03:12Z

Updated.

It occurred to me that there's no reason for us to not allow multiple targets for cargo rustdoc, so I removed that restriction entirely.

alexcrichton · 2015-09-28T17:06:35Z

The restriction for only one target actually mirrors cargo rustc to ensure that the custom flags are targeted at only one target instead of accidentally at many. I believe it's possible to have many targets in one package with e.g. many binary targets. Could the restriction be re-added?

Manishearth · 2015-09-28T18:21:01Z

But why would someone care about flags applying "accidentally" to many? These are docs, not code. It makes some sense to want to avoid applying the wrong extra link args, but the rustdoc flags are pretty benign.

Besides, it's not possible to filter targets for docs in the current framework. (https://github.com/rust-lang/cargo/blob/master/src/bin/rustc.rs#L82, https://github.com/rust-lang/cargo/blob/master/src/bin/doc.rs#L69). Could be fixed though.

alexcrichton · 2015-09-28T20:56:43Z

This is inconsistent with cargo rustc, which this is intending to mirror, so it needs to reject the situation where multiple targets may apply. Although it may be less harmful in the documentation than compile case, it's still important that these commands maintain consistency.

For filtering, that's why I was saying that the --bin and --lib flags will need to be added to cargo rustdoc.

bors · 2015-09-30T23:46:00Z

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

jonas-schievink · 2015-10-31T12:53:54Z

@Manishearth What's the status of this? I'd love to use this for internal documentation.

Manishearth · 2015-10-31T12:54:46Z

Bit busy right now. I'm not really sure how to add the other flags, and sort of disagree that we need to.

alexcrichton · 2015-11-02T19:53:48Z

@Manishearth would you like me to take this over? To add the new flags you'd want to change the compile filter being used and construct it in a manner similar to the other subcommands.

Manishearth · 2015-11-02T20:21:04Z

Sure, take it over. I don't have a lot of continuous time to work on things, and this is at the back of my queue :)

(Yeah, I know the overall steps, haven't had the time to look into the specifics)

This rebases rust-lang#1977 onto master and also tweaks the behavior to match `cargo rustc` when there are multiple targets in play.

alexcrichton · 2015-11-09T22:15:40Z

Rebased in #2129

rust-highfive assigned alexcrichton Sep 9, 2015

alexcrichton reviewed Sep 11, 2015
View reviewed changes

alexcrichton reviewed Sep 14, 2015
View reviewed changes

alexcrichton reviewed Sep 16, 2015
View reviewed changes

Manishearth added 6 commits September 26, 2015 22:31

Add cargo rustdoc for passing arbitrary flags to rustdoc

2f1bddb

Add test for cargo rustdoc

4d169b0

Address review comments

ec4bd70

More tests

7a7b0bd

Address review comments

54411ea

allow multiple targets

457e7bd

Manishearth force-pushed the rustdoc_args branch from 16a8052 to 457e7bd Compare September 26, 2015 17:02

alexcrichton added a commit to alexcrichton/cargo that referenced this pull request Nov 9, 2015

Rebase cargo rustdoc and tweak behavior

54127ce

This rebases rust-lang#1977 onto master and also tweaks the behavior to match `cargo rustc` when there are multiple targets in play.

alexcrichton closed this Nov 9, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `cargo rustdoc` for passing arbitrary flags to rustdoc #1977

Add `cargo rustdoc` for passing arbitrary flags to rustdoc #1977

Manishearth commented Sep 9, 2015

rust-highfive commented Sep 9, 2015

steveklabnik commented Sep 9, 2015

alexcrichton Sep 11, 2015

alexcrichton commented Sep 11, 2015

brson commented Sep 11, 2015

Manishearth commented Sep 12, 2015

alexcrichton commented Sep 13, 2015

Manishearth commented Sep 14, 2015

alexcrichton commented Sep 14, 2015

alexcrichton Sep 14, 2015

Manishearth commented Sep 14, 2015

alexcrichton Sep 16, 2015

bors commented Sep 22, 2015

Manishearth commented Sep 26, 2015

alexcrichton commented Sep 28, 2015

Manishearth commented Sep 28, 2015

alexcrichton commented Sep 28, 2015

bors commented Sep 30, 2015

jonas-schievink commented Oct 31, 2015

Manishearth commented Oct 31, 2015

alexcrichton commented Nov 2, 2015

Manishearth commented Nov 2, 2015

alexcrichton commented Nov 9, 2015

Add cargo rustdoc for passing arbitrary flags to rustdoc #1977

Add cargo rustdoc for passing arbitrary flags to rustdoc #1977

Conversation

Manishearth commented Sep 9, 2015

rust-highfive commented Sep 9, 2015

steveklabnik commented Sep 9, 2015

alexcrichton Sep 11, 2015

Choose a reason for hiding this comment

alexcrichton commented Sep 11, 2015

brson commented Sep 11, 2015

Manishearth commented Sep 12, 2015

alexcrichton commented Sep 13, 2015

Manishearth commented Sep 14, 2015

alexcrichton commented Sep 14, 2015

alexcrichton Sep 14, 2015

Choose a reason for hiding this comment

Manishearth commented Sep 14, 2015

alexcrichton Sep 16, 2015

Choose a reason for hiding this comment

bors commented Sep 22, 2015

Manishearth commented Sep 26, 2015

alexcrichton commented Sep 28, 2015

Manishearth commented Sep 28, 2015

alexcrichton commented Sep 28, 2015

bors commented Sep 30, 2015

jonas-schievink commented Oct 31, 2015

Manishearth commented Oct 31, 2015

alexcrichton commented Nov 2, 2015

Manishearth commented Nov 2, 2015

alexcrichton commented Nov 9, 2015

Add `cargo rustdoc` for passing arbitrary flags to rustdoc #1977

Add `cargo rustdoc` for passing arbitrary flags to rustdoc #1977