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

Add unstable documentation dependencies under [doc-dependencies] #10435

Closed
wants to merge 7 commits into from

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Feb 28, 2022

This pull-request introduce the concept of documentation dependencies similar to development dependencies except that documentation dependencies are only activated for doc and doc-test.

How should we test this PR?

Cargo.toml:

cargo-features = ["doc-dependencies"]

[package]
name = "foo"
version = "0.1.0"

[doc-dependencies]
sysinfo = "0.23" 

src/lib.rs

#[cfg(doc)]
pub use sysinfo::System;

cargo doc you should see the System type.

How should we review this PR?

Commit by commit.

The most important change is in src/cargo/core/compiler/unit_dependencies.rs:

                         // If this dependency is **not** a transitive dependency, then it
-                        // only applies to test/example targets.
-                        if !dep.is_transitive()
-                            && !unit.target.is_test()
-                            && !unit.target.is_example()
-                            && !unit.mode.is_doc_scrape()
-                            && !unit.mode.is_any_test()
-                        {
-                            return false;
+                        // only applies to test/example or doc/doctest targets.
+                        match dep.kind() {
+                            DepKind::Development => {
+                                if !unit.target.is_test()
+                                    && !unit.target.is_example()
+                                    && !unit.mode.is_doc_scrape()
+                                    && !unit.mode.is_any_test()
+                                {
+                                    return false;
+                                }
+                            }
+                            DepKind::Documentation => {
+                                if !unit.mode.is_doc() && !unit.mode.is_doc_test() {
+                                    return false;
+                                }
+                            }
+                            DepKind::Normal | DepKind::Build => {}
+                        }

Additional information

  • Decouple -Z avoid-dev-deps from interacting with doc deps
  • Add unstable flag for the documentation interaction in cargo tree
  • Resolve the cycle dependency stack overflow

Fixes #8905
cc @GuillaumeGomez

@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 Feb 28, 2022
@GuillaumeGomez
Copy link
Member

This is awesome, thanks a lot!

@joshtriplett
Copy link
Member

joshtriplett commented Mar 1, 2022

We talked about this at length in today's @rust-lang/cargo meeting.

Currently, Rust has three kinds of dependencies: dependencies, dev-dependencies, and build-dependencies. Adding a fourth kind of dependency would be a substantial increase in complexity.

The original issue, #8905 , started out talking about using feature flags for this, and in our discussions today we felt like that approach would be much simpler and make more use of existing Cargo facilities.

Crates today already use features for this, and then add docs.rs metadata telling docs.rs to use those features. And features can already control dependencies or features of dependencies.

We'd be open to the idea of adding a mechanism to automatically enable a specific feature for cargo doc by default.

Also, separately, this is something we'd consider to be a large feature, for which we'd like to see an RFC design rather than going directly to a PR. Doing design iteration in a PR is much higher bandwidth for us than reviewing an RFC.

I'm going to close this PR in favor of #8905 , and propose that we discuss the feature design further there and subsequently in an RFC.

@GuillaumeGomez
Copy link
Member

The biggest issue is that it using features makes cfg(doc) and cfg(doctest) pretty much useless and force users to handle it through features. It also means that you can't "control" how users build the documentation (which is a huge issue) and you can't specify a feature only for doc builds.

Just to be sure: if an RFC about this is made, would you be interested into it?

@joshtriplett
Copy link
Member

@GuillaumeGomez Sorry, I missed including a detail from the meeting: We'd be open to the idea of adding a mechanism to automatically enable a specific feature for cargo doc by default. I've updated the comment above accordingly.

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.

There is currently no way to specify features automatically when using cargo doc
5 participants