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.

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

cargo doc shouldn't specify --document-private-items twice for binaries #8373

Closed
davepacheco opened this issue Jun 17, 2020 · 2 comments
Closed

cargo doc shouldn't specify --document-private-items twice for binaries #8373

davepacheco opened this issue Jun 17, 2020 · 2 comments
Labels
C-bug Category: bug Command-doc

Comments

@davepacheco
Copy link
Contributor

Problem

PR #7593 changed Cargo to document private items for binary targets by default. This is great!

In my case, I've got a crate with two binaries and a library. The library is really only to support the binaries and the test suite. There's no need for public documentation. Instead, I want to configure the repo so that cargo doc documents the private items in the library. I created a .cargo/config that specifies build.rustdocflags = "--document-private-items". If I run cargo doc --lib, this does the right thing (generates library documentation that includes private items). But if I just run cargo doc, this fails because it tries to generate docs for the binaries, hits the behavior added in #7593, and passes --document-private-items twice to rustdoc.

Here's an example with a demo repo that I created to test this:

$ cargo doc -v
    Checking cargo-doc-rustdocflags v0.1.0 (/Users/dap/oxide/cargo-rustdoc-flags)
 Documenting cargo-doc-rustdocflags v0.1.0 (/Users/dap/oxide/cargo-rustdoc-flags)
     Running `rustdoc --edition=2018 --crate-type lib --crate-name cargo_doc_rustdocflags src/lib.rs -o /Users/dap/oxide/cargo-rustdoc-flags/target/doc --error-format=json --json=diagnostic-rendered-ansi -L dependency=/Users/dap/oxide/cargo-rustdoc-flags/target/debug/deps --document-private-items`
     Running `rustc --crate-name cargo_doc_rustdocflags --edition=2018 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type lib --emit=dep-info,metadata -C debuginfo=2 -C metadata=fd5eda5c10a1d5e5 -C extra-filename=-fd5eda5c10a1d5e5 --out-dir /Users/dap/oxide/cargo-rustdoc-flags/target/debug/deps -C incremental=/Users/dap/oxide/cargo-rustdoc-flags/target/debug/incremental -L dependency=/Users/dap/oxide/cargo-rustdoc-flags/target/debug/deps`
     Running `rustdoc --edition=2018 --crate-type bin --crate-name hello src/bin/hello/main.rs -o /Users/dap/oxide/cargo-rustdoc-flags/target/doc --error-format=json --json=diagnostic-rendered-ansi --document-private-items -L dependency=/Users/dap/oxide/cargo-rustdoc-flags/target/debug/deps --extern cargo_doc_rustdocflags=/Users/dap/oxide/cargo-rustdoc-flags/target/debug/deps/libcargo_doc_rustdocflags-fd5eda5c10a1d5e5.rmeta --document-private-items`
error: Option 'document-private-items' given more than once

error: Could not document `cargo-doc-rustdocflags`.

Caused by:
  process didn't exit successfully: `rustdoc --edition=2018 --crate-type bin --crate-name hello src/bin/hello/main.rs -o /Users/dap/oxide/cargo-rustdoc-flags/target/doc --error-format=json --json=diagnostic-rendered-ansi --document-private-items -L dependency=/Users/dap/oxide/cargo-rustdoc-flags/target/debug/deps --extern cargo_doc_rustdocflags=/Users/dap/oxide/cargo-rustdoc-flags/target/debug/deps/libcargo_doc_rustdocflags-fd5eda5c10a1d5e5.rmeta --document-private-items` (exit code: 1)
warning: build failed, waiting for other jobs to finish...
error: build failed

Steps

  1. Clone this repo: https://github.com/davepacheco/bug-cargo-doc-rustdocflags
  2. Run cargo doc. Instead of generating docs, it fails.

I expect you'd also run into this without a .cargo/config if you specified RUSTDOCFLAGS=--document-private-items in the environment.

Possible Solution(s): It would be nice if the auto-document-private-items behavior for binary targets checked whether --document-private-items is already part of the rustdoc flags before adding it.

You can work around this problem by disabling docs for the binaries, using a block like this in Cargo.toml:

[[bin]]
name = # your binary's name
doc = false

Notes

Output of cargo version:

$ cargo version
cargo 1.42.0 (86334295e 2020-01-31)

I see the same on cargo 1.45.0-nightly (9fcb8c1d2 2020-05-25). I'm on OS X 10.15.4.
@davepacheco davepacheco added the C-bug Category: bug label Jun 17, 2020
@davepacheco davepacheco mentioned this issue Jun 17, 2020
Merged
5 tasks
This was referenced Jun 26, 2020
Closed
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jul 11, 2021
@Dylan-DPC
Rollup merge of rust-lang#73936 - zachlute:rustdoc-optflagmulti, r=jy…
c65a6d7 
…n514

Rustdoc: Change all 'optflag' arguments to 'optflagmulti'

Because specifying these flags multiple times will never be discernibly different in functionality from specifying them a single time, there is no reason to fail and report an error to the user.

This might be a slightly controversial change. it's tough to say, but it's hard to imagine a case where somebody was depending on this behavior, and doing this seem actively better for the user.

This originally came up in discussion of a fix for  [Cargo rust-lang#8373](rust-lang/cargo#8373), in [Cargo PR rust-lang#8422](rust-lang/cargo#8422).

The issue is that Cargo will automatically add things like `--document-private-items` to binaries, because it's the only thing that makes sense there. Then some poor user comes along and adds `--document-private-items` to their `rustdoc` flags for the project and suddenly they're getting errors for specifying a flag twice and need to track down which targets to actually add it to without getting duplicates for reasons they won't understand without deep understanding of Cargo behavior.

We're apparently hesitant to inspect `rustdoc` flags provided by the user directly in Cargo, because they're supposed to be opaque, so looking to see if it's already provided before adding it is evidently a non-starter. In trying to resolve that, one suggestion I came up with was to just change `rustdoc` to support passing the flag multiple times, because the user's intent should be clear and it's not *really* an error, so maybe this is a case of 'be permissive in what you accept'.

This PR is an attempt to do that in a straightforward manner for purposes of discussion.
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jul 11, 2021
@JohnTitor
Rollup merge of rust-lang#73936 - zachlute:rustdoc-optflagmulti, r=jy…
25dda36 
…n514

Rustdoc: Change all 'optflag' arguments to 'optflagmulti'

Because specifying these flags multiple times will never be discernibly different in functionality from specifying them a single time, there is no reason to fail and report an error to the user.

This might be a slightly controversial change. it's tough to say, but it's hard to imagine a case where somebody was depending on this behavior, and doing this seem actively better for the user.

This originally came up in discussion of a fix for  [Cargo rust-lang#8373](rust-lang/cargo#8373), in [Cargo PR rust-lang#8422](rust-lang/cargo#8422).

The issue is that Cargo will automatically add things like `--document-private-items` to binaries, because it's the only thing that makes sense there. Then some poor user comes along and adds `--document-private-items` to their `rustdoc` flags for the project and suddenly they're getting errors for specifying a flag twice and need to track down which targets to actually add it to without getting duplicates for reasons they won't understand without deep understanding of Cargo behavior.

We're apparently hesitant to inspect `rustdoc` flags provided by the user directly in Cargo, because they're supposed to be opaque, so looking to see if it's already provided before adding it is evidently a non-starter. In trying to resolve that, one suggestion I came up with was to just change `rustdoc` to support passing the flag multiple times, because the user's intent should be clear and it's not *really* an error, so maybe this is a case of 'be permissive in what you accept'.

This PR is an attempt to do that in a straightforward manner for purposes of discussion.
@zachlute
Copy link
Contributor

Rustdoc PR #73936 was merged today, which means that specifying --document-private-items (or any other flag) multiple times will no longer be an error and will simply silently act like you only specified it once.

I believe this can be closed now.

@ehuss
Copy link
Contributor

ehuss commented Jul 12, 2021

Yup, thanks!

@ehuss ehuss closed this as completed Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: bug Command-doc
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Stop adding the --document-private-items flag if it's already being set zachlute/cargo
3 participants
@ehuss @zachlute @davepacheco