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

Document build dependencies and dev dependencies #8562

Closed
wants to merge 1 commit into from
Closed

Document build dependencies and dev dependencies #8562

wants to merge 1 commit into from

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Jul 29, 2020

Add a new --kinds option to cargo doc, which allows to select what
kinds of dependencies should be documented: build dependencies, dev
dependencies, or normal dependencies. Multiple kinds of dependencies
can be specified, e.g., cargo doc --kinds=dev,normal.

When --kinds options is used, the default behaviour of documenting
normal dependencies is suppressed unless asked for explicitly with
--kinds=normal. The --kinds option also conflicts with --no-deps.

Follow up on proposal from #7077. Resolves #3475.

Open questions:

  1. Should --kinds document normal dependencies by default, obviating the need
    for normal kind of dependencies? Current answer is no.
  2. Should --kinds interact with --no-deps flag? Current answer is no, but
    presence or absence of --no-deps could control if dependencies of selected
    dependencies are also documented.

@rust-highfive
Copy link

r? @ehuss

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 29, 2020
src/bin/cargo/commands/doc.rs Outdated Show resolved Hide resolved
@ehuss
Copy link
Contributor

ehuss commented Aug 4, 2020

Hi! Thanks for the PR!

1. Should --deps document normal dependencies by default,

I think the default of "no" here makes sense to me.

2. Should --deps interact with --no-deps flag?

Hm, I would be inclined to just print an error if both flags are specified. They are somewhat contradictory. It's probably not too important either way.

A few questions and observations:

  • It's an interesting choice to never do transitive dependencies when using --deps. Do you think there might be a use case where someone would want transitive + normal + dev + build? (FWIW, there's a bunch of notes about transitive deps in Tracking issue for -Zrustdoc-map #8296 with links to other issues.)
  • What do you think about adding an "all" option? I think the previous PR had something like that, and it seems like it would be a useful shortcut. My only concern is that it could be confusing regarding the "transitive" question above.

@tmiasko
Copy link
Contributor Author

tmiasko commented Aug 9, 2020

Do you think there might be a use case where someone would want transitive + normal + dev + build?

When I was referring to the transitive dependencies I was mostly thinking about
normal transitive dependencies of those selected. Normal dependencies might be
part of public API of the crate being documented, so I think there is
relatively clear use-case for those.

I like idea of all-deps, no-deps, direct-deps from #8296. If it were to become
a thing at some point, I think that it should work together with feature here.

I added all kind, and changed the flag name to --kinds. I think this would
be less confusing with respect to the question of transitive dependencies. We
could be even more explicit with --dep-kinds?

I also noticed that this sometimes introduces an output filename collision for
documented crates, but didn't have time to investigate yet.

EDIT: The current implementation modifies the compile mode to distinguish
between direct / indirect dependencies. As a result the same unit might occur
more than once with slightly different mode, which in turn introduces
conflicts. Not sure how to address this yet. Any suggestions?

@ehuss
Copy link
Contributor

ehuss commented Sep 2, 2020

I kinda like the original flag name of --deps. It has a good symmetry with --no-deps, and I think is very clear in what it does. Would you consider maybe switching it back?

As for the question of how to indicate whether or not you want transitive deps, I don't have any specific ideas. I'd be fine for deferring that decision to later. It could be something like --deps=normal,transitive or something like that. I'm not sure there's a strong use case for that. I'm also thinking that in the eventual future if public-private dependencies is stabilized, then cargo could always just do "reachable public" dependencies, and never bother with unreachable private ones.

the same unit might occur more than once with slightly different mode

Do you have a specific example where this happens? Just keep in mind that this is already a problem, so I'm not sure if you're seeing something new. #6313 has some links for rustdoc issues (like any time there is more than one version).

@ehuss ehuss added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 9, 2020
Add a new `--deps` option to cargo doc, which allows to select what
kinds of dependencies should be documented: build dependencies, dev
dependencies, or normal dependencies. Multiple kinds of dependencies
can be specified, e.g., `cargo doc --deps=dev,normal`.

When `--deps` options is used, the default behaviour of documenting
normal dependencies is suppressed unless asked for explicitly with
`--deps=normal`. The `--deps` option also conflicts with `--no-deps`.
@tmiasko
Copy link
Contributor Author

tmiasko commented Sep 22, 2020

After looking at this a bit more, it seems to me it will require completely different approach and more substantial changes. Thanks for reviewing this so far. I will close this for now.

@tmiasko tmiasko closed this Sep 22, 2020
@tmiasko tmiasko deleted the doc-deps-kinds branch September 22, 2020 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optionally document dev-dependencies
3 participants