Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion src/cargo/core/compiler/unit_dependencies.rs
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently cargo doc generates docs for all transitive dependencies, which creates a lot of noise.

This allows library authors to control their documentation surface by
marking which dependencies are part of their public API.

The reason is that indirect private deps have no impact on the reader of the docs since they cannot be used and this can dramatically speed up documentation builds, especially with packages like windows-sys in a dependency tree

Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ struct State<'a, 'gctx> {
/// dependency from a to b was added purely because it was a dev-dependency.
/// This is used during `connect_run_custom_build_deps`.
dev_dependency_edges: HashSet<(Unit, Unit)>,

/// Package IDs of the root units (i.e. the packages the user asked to
/// compile). Used when deciding whether to document a dependency: roots
/// always get all their direct deps documented, while non-roots only
/// get public deps documented when `-Zpublic-dependency` is active.
root_pkg_ids: HashSet<PackageId>,
}

/// A boolean-like to indicate if a `Unit` is an artifact or not.
Expand Down Expand Up @@ -126,6 +132,7 @@ pub fn build_unit_dependencies<'a, 'gctx>(
interner,
scrape_units,
dev_dependency_edges: HashSet::new(),
root_pkg_ids: roots.iter().map(|u| u.pkg.package_id()).collect(),
};

let std_unit_deps = calc_deps_of_std(&mut state, std_roots)?;
Expand Down Expand Up @@ -637,6 +644,19 @@ fn compute_deps_doc(
// built. If we're documenting *all* libraries, then we also depend on
// the documentation of the library being built.
let mut ret = Vec::new();

let public_deps_enabled = state.gctx.cli_unstable().public_dependency
|| unit
.pkg
.manifest()
.unstable_features()
.is_enabled(Feature::public_dependency());

// Whether this package is a root of the compilation (i.e. selected by
// the user). Roots always have all their direct deps documented,
// regardless of public/private status.
let is_root = state.root_pkg_ids.contains(&unit.pkg.package_id());

for (id, deps) in state.deps(unit, unit_for) {
let Some(dep_lib) = calc_artifact_deps(unit, unit_for, id, &deps, state, &mut ret)? else {
continue;
Expand All @@ -657,7 +677,17 @@ fn compute_deps_doc(
IS_NO_ARTIFACT_DEP,
)?;
ret.push(lib_unit_dep);
if dep_lib.documented() && state.intent.wants_deps_docs() {

// Decide whether to document this dependency. When
// public-dependency is enabled, only document direct deps of root
// packages and public deps (recursively).
let should_doc_dep = if is_root || !public_deps_enabled {
true
} else {
state.resolve().is_public_dep(unit.pkg.package_id(), id)
};

if dep_lib.documented() && state.intent.wants_deps_docs() && should_doc_dep {
// Document this lib as well.
let doc_unit_dep = new_unit_dep(
state,
Expand Down
327 changes: 327 additions & 0 deletions tests/testsuite/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4029,3 +4029,330 @@ fn mergeable_info_dep_collision() {
// ...and the fingerprint content are different (path to dep.json different)
assert_ne!(first_fingerprint, second_fingerprint);
}

#[cargo_test(nightly, reason = "public-dependency feature is unstable")]
fn doc_with_public_dependency_transitive() {
// selected is the user-chosen package
// foo-dep is a direct dep of selected
// public-bar-dep is a public dep of foo-dep
// public-baz-dep is a public dep of public-bar-dep
// All four should be documented since the whole chain is public.

Package::new("public-baz-dep", "0.0.1")
.file("src/lib.rs", "pub fn public_baz_dep() {}")
.publish();

Package::new("public-bar-dep", "0.0.1")
.cargo_feature("public-dependency")
.add_dep(
cargo_test_support::registry::Dependency::new("public-baz-dep", "0.0.1").public(true),
)
.file("src/lib.rs", "pub fn public_bar_dep() {}")
.publish();

Package::new("foo-dep", "0.0.1")
.cargo_feature("public-dependency")
.add_dep(
cargo_test_support::registry::Dependency::new("public-bar-dep", "0.0.1").public(true),
)
.file("src/lib.rs", "pub fn foo_dep() {}")
.publish();

let p = project()
.file(
"Cargo.toml",
r#"
cargo-features = ["public-dependency"]

[package]
name = "selected"
version = "0.0.1"
edition = "2021"

[dependencies]
foo-dep = "0.0.1"
"#,
)
.file("src/lib.rs", "pub fn selected() {}")
.build();

p.cargo("doc -Zpublic-dependency")
.masquerade_as_nightly_cargo(&["public-dependency"])
.with_stderr_data(
str![[r#"
[UPDATING] `dummy-registry` index
[LOCKING] 3 packages to latest compatible versions
[DOWNLOADING] crates ...
[DOWNLOADED] public-baz-dep v0.0.1 (registry `dummy-registry`)
[DOWNLOADED] public-bar-dep v0.0.1 (registry `dummy-registry`)
[DOWNLOADED] foo-dep v0.0.1 (registry `dummy-registry`)
[DOCUMENTING] public-baz-dep v0.0.1
[CHECKING] public-baz-dep v0.0.1
[DOCUMENTING] public-bar-dep v0.0.1
[CHECKING] public-bar-dep v0.0.1
[DOCUMENTING] foo-dep v0.0.1
[CHECKING] foo-dep v0.0.1
[DOCUMENTING] selected v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[GENERATED] [ROOT]/foo/target/doc/selected/index.html

"#]]
.unordered(),
)
.run();

// All four are documented: the whole chain is public.
assert!(p.root().join("target/doc/selected/index.html").is_file());
assert!(p.root().join("target/doc/foo_dep/index.html").is_file());
assert!(
p.root()
.join("target/doc/public_bar_dep/index.html")
.is_file()
);
assert!(
p.root()
.join("target/doc/public_baz_dep/index.html")
.is_file()
);
}

#[cargo_test(nightly, reason = "public-dependency feature is unstable")]
fn doc_direct_deps_always_documented() {
// Direct dependencies should always be documented regardless of public flag
// foo -> bar (public=true), baz (public=false)
// Both bar and baz should be documented since they are direct deps

Package::new("bar", "0.0.1")
.file("src/lib.rs", "pub fn bar() {}")
.publish();

Package::new("baz", "0.0.1")
.file("src/lib.rs", "pub fn baz() {}")
.publish();

let p = project()
.file(
"Cargo.toml",
r#"
cargo-features = ["public-dependency"]

[package]
name = "foo"
version = "0.0.1"
edition = "2021"

[dependencies]
bar = { version = "0.0.1", public = true }
baz = { version = "0.0.1", public = false }
"#,
)
.file("src/lib.rs", "pub fn foo() {}")
.build();

p.cargo("doc -Zpublic-dependency")
.masquerade_as_nightly_cargo(&["public-dependency"])
.with_stderr_data(
str![[r#"
[UPDATING] `dummy-registry` index
[LOCKING] 2 packages to latest compatible versions
[DOWNLOADING] crates ...
[DOWNLOADED] bar v0.0.1 (registry `dummy-registry`)
[DOWNLOADED] baz v0.0.1 (registry `dummy-registry`)
[DOCUMENTING] bar v0.0.1
[CHECKING] bar v0.0.1
[DOCUMENTING] baz v0.0.1
[CHECKING] baz v0.0.1
[DOCUMENTING] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[GENERATED] [ROOT]/foo/target/doc/foo/index.html

"#]]
.unordered(),
)
.run();

// Both direct deps should be documented
assert!(p.root().join("target/doc/foo/index.html").is_file());
assert!(p.root().join("target/doc/bar/index.html").is_file());
assert!(p.root().join("target/doc/baz/index.html").is_file());
}

#[cargo_test(nightly, reason = "public-dependency feature is unstable")]
fn doc_with_transitive_private_dependency() {
// foo -> bar (direct dep) -> baz (private dep of bar, transitive to foo)
// baz should NOT be documented because it is a private transitive dep.

Package::new("baz", "0.0.1")
.file("src/lib.rs", "pub fn baz() {}")
.publish();

Package::new("bar", "0.0.1")
.cargo_feature("public-dependency")
.add_dep(cargo_test_support::registry::Dependency::new("baz", "0.0.1").public(false))
.file("src/lib.rs", "pub fn bar() {}")
.publish();

let p = project()
.file(
"Cargo.toml",
r#"
cargo-features = ["public-dependency"]

[package]
name = "foo"
version = "0.0.1"
edition = "2021"

[dependencies]
bar = "0.0.1"
"#,
)
.file("src/lib.rs", "pub fn foo() {}")
.build();

p.cargo("doc -Zpublic-dependency")
.masquerade_as_nightly_cargo(&["public-dependency"])
.with_stderr_data(
str![[r#"
[UPDATING] `dummy-registry` index
[LOCKING] 2 packages to latest compatible versions
[DOWNLOADING] crates ...
[DOWNLOADED] baz v0.0.1 (registry `dummy-registry`)
[DOWNLOADED] bar v0.0.1 (registry `dummy-registry`)
[CHECKING] baz v0.0.1
[DOCUMENTING] bar v0.0.1
[CHECKING] bar v0.0.1
[DOCUMENTING] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[GENERATED] [ROOT]/foo/target/doc/foo/index.html

"#]]
.unordered(),
)
.run();

assert!(p.root().join("target/doc/foo/index.html").is_file());
assert!(p.root().join("target/doc/bar/index.html").is_file());
assert!(!p.root().join("target/doc/baz/index.html").is_file());
}

#[cargo_test(nightly, reason = "public-dependency feature is unstable")]
fn doc_mixed_public_private_deps() {
// foo -> pub_dep (public), priv_dep (private), priv_dep_with_dep (unannotated)
// priv_dep_with_dep -> transitive

Package::new("pub_dep", "0.0.1")
.file("src/lib.rs", "pub fn pub_dep() {}")
.publish();

Package::new("priv_dep", "0.0.1")
.file("src/lib.rs", "pub fn priv_dep() {}")
.publish();

Package::new("transitive", "0.0.1")
.file("src/lib.rs", "pub fn transitive() {}")
.publish();

Package::new("priv_dep_with_dep", "0.0.1")
.dep("transitive", "0.0.1")
.file("src/lib.rs", "pub fn priv_dep_with_dep() {}")
.publish();

let p = project()
.file(
"Cargo.toml",
r#"
cargo-features = ["public-dependency"]

[package]
name = "foo"
version = "0.0.1"
edition = "2021"

[dependencies]
pub_dep = { version = "0.0.1", public = true }
priv_dep = { version = "0.0.1", public = false }
priv_dep_with_dep = "0.0.1"
"#,
)
.file("src/lib.rs", "pub fn foo() {}")
.build();

p.cargo("doc -Zpublic-dependency")
.masquerade_as_nightly_cargo(&["public-dependency"])
.run();

assert!(p.root().join("target/doc/foo/index.html").is_file());
assert!(p.root().join("target/doc/pub_dep/index.html").is_file());
assert!(p.root().join("target/doc/priv_dep/index.html").is_file());
assert!(
p.root()
.join("target/doc/priv_dep_with_dep/index.html")
.is_file()
);
assert!(!p.root().join("target/doc/transitive/index.html").is_file());
}

#[cargo_test(nightly, reason = "public-dependency feature is unstable")]
fn doc_workspace_member_private_dep() {
// selected and skipped are both workspace members.
// selected has a private dep on skipped.
// skipped has a dep on transitive (a registry crate).
//
// Running `cargo doc -p selected`, selected is the root so all its
// direct deps (skipped) are documented. But skipped is not a root,
// so the public-dependency filter applies: transitive is not marked
// public by skipped, so it should not be documented.

Package::new("transitive", "0.0.1")
.file("src/lib.rs", "pub fn transitive() {}")
.publish();
Comment on lines +4305 to +4307
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move transitive into the workspace? There are enough confusion when dealing with direct vs transitive dependencies, it would be good to have everything in the workspace to make the intent clear.


let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = ["selected", "skipped"]
"#,
)
.file(
"selected/Cargo.toml",
r#"
cargo-features = ["public-dependency"]

[package]
name = "selected"
version = "0.0.1"
edition = "2021"

[dependencies]
skipped = { path = "../skipped", public = false }
"#,
)
.file("selected/src/lib.rs", "pub fn selected() {}")
.file(
"skipped/Cargo.toml",
r#"
[package]
name = "skipped"
version = "0.0.1"
edition = "2021"

[dependencies]
transitive = "0.0.1"
"#,
)
.file("skipped/src/lib.rs", "pub fn skipped() {}")
.build();

p.cargo("doc -p selected -Zpublic-dependency")
.masquerade_as_nightly_cargo(&["public-dependency"])
.run();

assert!(p.root().join("target/doc/selected/index.html").is_file());
assert!(p.root().join("target/doc/skipped/index.html").is_file());
// transitive is not documented: skipped is not a root, so the
// public-dependency filter kicks in and transitive is not public
assert!(!p.root().join("target/doc/transitive/index.html").is_file());
}