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

Scrape code examples from examples/ directory for Rustdoc #9525

Merged
merged 34 commits into from
Oct 28, 2021
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
8acf0e8
Initial support for scrape-examples
willcrichton May 11, 2021
3f9a4f2
Add repository URL
willcrichton May 11, 2021
a0122df
Share all examples across each unit in worksapce
willcrichton May 12, 2021
63c4346
Factor scraping and rendering into separate calls to rustdoc
willcrichton Jun 1, 2021
49a3a54
Add workspace root
willcrichton Jun 2, 2021
4ac6835
Fix for issue #9198
willcrichton Jun 3, 2021
b14a778
Change cargo strategy to go through rustdoc instead of doctest
willcrichton Jun 3, 2021
711539f
Add unstable notice for --scrape-examples
willcrichton Jun 3, 2021
6772991
Remove unnecessary collect
willcrichton Jun 3, 2021
5ed35a4
Fix test
willcrichton Jun 15, 2021
d19cfd2
Fix breakage
willcrichton Aug 25, 2021
48056e5
Allow scraping examples from library
willcrichton Aug 26, 2021
ff13eb5
Add comments
willcrichton Aug 27, 2021
0c8e1f8
Fix documentation
willcrichton Aug 27, 2021
0b2e293
Add tracking issue and unstable documentation
willcrichton Sep 14, 2021
b9b39a6
Pass metadata flag to rustdoc, ensure that Doc and Check units have s…
willcrichton Sep 21, 2021
dbcabc7
Change metadata strategy to extract hash from nested compilation
willcrichton Sep 21, 2021
0792cde
Revert change to lift doc units
willcrichton Sep 21, 2021
82d937e
Remove references to json
willcrichton Sep 21, 2021
70f3821
Change scraping strategy to embed in existing unit graph, add Compile…
willcrichton Oct 12, 2021
d29ac15
Remove unused code
willcrichton Oct 12, 2021
8331d7d
Remove more unused code
willcrichton Oct 12, 2021
223adac
Formatting
willcrichton Oct 12, 2021
19c8f05
Fix issues compiling build scripts
willcrichton Oct 13, 2021
4705566
Add documentation and a test
willcrichton Oct 13, 2021
17c6df7
Remove references to "lib" argument
willcrichton Oct 13, 2021
8b06a0f
Update rustdoc tests with -Cmetadata flag
willcrichton Oct 13, 2021
e52a9d9
Add comments and todos
willcrichton Oct 14, 2021
b948fc8
Add test / documentation for scrape-examples cycle-avoidance, add map…
willcrichton Oct 14, 2021
e4a65b9
Fix several bugs when checking wasmtime repo:
willcrichton Oct 27, 2021
0deeea8
Remove unnecessary clones, document out_dir
willcrichton Oct 27, 2021
1120957
Change scraping to apply to all workspace packages instead of just
willcrichton Oct 28, 2021
0a2382b
Formatting
willcrichton Oct 28, 2021
33718c7
Fix repeated warning with two calls to to_package_id_specs
willcrichton Oct 28, 2021
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
37 changes: 36 additions & 1 deletion src/bin/cargo/commands/doc.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::command_prelude::*;

use cargo::ops::{self, DocOptions};
use anyhow::anyhow;
use cargo::ops::{self, CompileFilter, DocOptions, FilterRule, LibRule};

pub fn cli() -> App {
subcommand("doc")
Expand All @@ -19,6 +20,13 @@ pub fn cli() -> App {
)
.arg(opt("no-deps", "Don't build documentation for dependencies"))
.arg(opt("document-private-items", "Document private items"))
.arg(
opt(
"scrape-examples",
"Scrape examples to include as function documentation",
)
.value_name("FLAGS"),
)
.arg_jobs()
.arg_targets_lib_bin_example(
"Document only this package's library",
Expand Down Expand Up @@ -48,6 +56,33 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
args.compile_options(config, mode, Some(&ws), ProfileChecking::Custom)?;
compile_opts.rustdoc_document_private_items = args.is_present("document-private-items");

// TODO(wcrichto): move scrape example configuration into Cargo.toml before stabilization
// See: https://github.com/rust-lang/cargo/pull/9525#discussion_r728470927
compile_opts.rustdoc_scrape_examples = match args.value_of("scrape-examples") {
Some(s) => Some(match s {
"all" => CompileFilter::new_all_targets(),
"examples" => CompileFilter::new(
LibRule::False,
FilterRule::none(),
FilterRule::none(),
FilterRule::All,
FilterRule::none(),
),
_ => {
return Err(CliError::from(anyhow!(
r#"--scrape-examples must take "all" or "examples" as an argument"#
)));
}
}),
None => None,
};

if compile_opts.rustdoc_scrape_examples.is_some() {
config
.cli_unstable()
.fail_if_stable_opt("--scrape-examples", 9910)?;
}

let doc_opts = DocOptions {
open_result: args.is_present("open"),
compile_opts,
Expand Down
8 changes: 8 additions & 0 deletions src/cargo/core/compiler/build_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ pub enum CompileMode {
Doc { deps: bool },
/// A target that will be tested with `rustdoc`.
Doctest,
/// An example or library that will be scraped for function calls by `rustdoc`.
Docscrape,
willcrichton marked this conversation as resolved.
Show resolved Hide resolved
/// A marker for Units that represent the execution of a `build.rs` script.
RunCustomBuild,
}
Expand All @@ -166,6 +168,7 @@ impl ser::Serialize for CompileMode {
Bench => "bench".serialize(s),
Doc { .. } => "doc".serialize(s),
Doctest => "doctest".serialize(s),
Docscrape => "docscrape".serialize(s),
RunCustomBuild => "run-custom-build".serialize(s),
}
}
Expand All @@ -187,6 +190,11 @@ impl CompileMode {
self == CompileMode::Doctest
}

/// Returns `true` if this is scraping examples for documentation.
pub fn is_doc_scrape(self) -> bool {
self == CompileMode::Docscrape
}

/// Returns `true` if this is any type of test (test, benchmark, doc test, or
/// check test).
pub fn is_any_test(self) -> bool {
Expand Down
5 changes: 5 additions & 0 deletions src/cargo/core/compiler/build_context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ pub struct BuildContext<'a, 'cfg> {
/// The dependency graph of units to compile.
pub unit_graph: UnitGraph,

/// Reverse-dependencies of documented units, used by the rustdoc --scrape-examples flag.
pub scrape_units: Vec<Unit>,

/// The list of all kinds that are involved in this build
pub all_kinds: HashSet<CompileKind>,
}
Expand All @@ -61,6 +64,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
target_data: RustcTargetData<'cfg>,
roots: Vec<Unit>,
unit_graph: UnitGraph,
scrape_units: Vec<Unit>,
) -> CargoResult<BuildContext<'a, 'cfg>> {
let all_kinds = unit_graph
.keys()
Expand All @@ -79,6 +83,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
target_data,
roots,
unit_graph,
scrape_units,
all_kinds,
})
}
Expand Down
5 changes: 4 additions & 1 deletion src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,10 @@ impl TargetInfo {
}
}
CompileMode::Check { .. } => Ok((vec![FileType::new_rmeta()], Vec::new())),
CompileMode::Doc { .. } | CompileMode::Doctest | CompileMode::RunCustomBuild => {
CompileMode::Doc { .. }
| CompileMode::Doctest
| CompileMode::Docscrape
| CompileMode::RunCustomBuild => {
panic!("asked for rustc output for non-rustc mode")
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/cargo/core/compiler/context/compilation_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,11 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
// but Cargo does not know about that.
vec![]
}
CompileMode::Docscrape => {
// Docscrape only generates temporary *.examples files to pass to rustdoc
// so they're not important to track
vec![]
}
CompileMode::Test
| CompileMode::Build
| CompileMode::Bench
Expand Down
42 changes: 42 additions & 0 deletions src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ pub struct Context<'a, 'cfg> {
/// compilation is happening (only object, only bitcode, both, etc), and is
/// precalculated early on.
pub lto: HashMap<Unit, Lto>,

/// Map of Doc/Docscrape units to metadata for their -Cmetadata flag.
/// See Context::find_metadata_units for more details.
pub metadata_for_doc_units: HashMap<Unit, Metadata>,
}

impl<'a, 'cfg> Context<'a, 'cfg> {
Expand Down Expand Up @@ -121,6 +125,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
rustc_clients: HashMap::new(),
pipelining,
lto: HashMap::new(),
metadata_for_doc_units: HashMap::new(),
})
}

Expand All @@ -135,6 +140,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
self.prepare()?;
custom_build::build_map(&mut self)?;
self.check_collisions()?;
self.compute_metadata_for_doc_units();

// We need to make sure that if there were any previous docs
// already compiled, they were compiled with the same Rustc version that we're currently
Expand Down Expand Up @@ -614,4 +620,40 @@ impl<'a, 'cfg> Context<'a, 'cfg> {

Ok(client)
}

/// Finds metadata for Doc/Docscrape units.
///
/// rustdoc needs a -Cmetadata flag in order to recognize StableCrateIds that refer to
/// items in the crate being documented. The -Cmetadata flag used by reverse-dependencies
/// will be the metadata of the Cargo unit that generated the current library's rmeta file,
/// which should be a Check unit.
///
/// If the current crate has reverse-dependencies, such a Check unit should exist, and so
/// we use that crate's metadata. If not, we use the crate's Doc unit so at least examples
/// scraped from the current crate can be used when documenting the current crate.
pub fn compute_metadata_for_doc_units(&mut self) {
for unit in self.bcx.unit_graph.keys() {
if !unit.mode.is_doc() && !unit.mode.is_doc_scrape() {
continue;
}

let matching_units = self
.bcx
.unit_graph
.keys()
.filter(|other| {
unit.pkg == other.pkg
&& unit.target == other.target
&& !other.mode.is_doc_scrape()
})
.collect::<Vec<_>>();
let metadata_unit = matching_units
.iter()
.find(|other| other.mode.is_check())
.or_else(|| matching_units.iter().find(|other| other.mode.is_doc()))
.unwrap_or(&unit);
self.metadata_for_doc_units
.insert(unit.clone(), self.files().metadata(metadata_unit));
}
}
}
42 changes: 41 additions & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ mod unit;
pub mod unit_dependencies;
pub mod unit_graph;

use std::collections::HashSet;
use std::env;
use std::ffi::{OsStr, OsString};
use std::fs::{self, File};
Expand Down Expand Up @@ -165,7 +166,7 @@ fn compile<'cfg>(
let force = exec.force_rebuild(unit) || force_rebuild;
let mut job = fingerprint::prepare_target(cx, unit, force)?;
job.before(if job.freshness() == Freshness::Dirty {
let work = if unit.mode.is_doc() {
let work = if unit.mode.is_doc() || unit.mode.is_doc_scrape() {
rustdoc(cx, unit)?
} else {
rustc(cx, unit, exec)?
Expand Down Expand Up @@ -647,6 +648,45 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
rustdoc.args(args);
}

let metadata = cx.metadata_for_doc_units[&unit];
rustdoc.arg("-C").arg(format!("metadata={}", metadata));
alexcrichton marked this conversation as resolved.
Show resolved Hide resolved

let scrape_output_path = |unit: &Unit| -> CargoResult<PathBuf> {
let layout = cx.files().layout(unit.kind);
let output_dir = layout.prepare_tmp()?;
Ok(output_dir.join(format!("{}.examples", unit.buildkey())))
};

if unit.mode.is_doc_scrape() {
rustdoc.arg("-Zunstable-options");

rustdoc
.arg("--scrape-examples-output-path")
.arg(scrape_output_path(unit)?);

// Limit the scraped examples to just crates in the root set
let root_packages = cx
.bcx
.roots
.iter()
.map(|root| root.pkg.name())
.collect::<HashSet<_>>();
for pkg in root_packages {
rustdoc.arg("--scrape-examples-target-crate").arg(pkg);
}
Copy link
Member

Choose a reason for hiding this comment

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

How does this interact if you do something like cargo doc -p foo -p bar where the "root set" is larger than one? Presumably the documenation for foo wouldn't want to get --scrape-examples-target-crate bar, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the other way around. If you have a file examples/ex.rs like

fn main() {
  foo::f();
  bar::f();
  baz::f();
  Vec::new();
}

Then the --scrape-examples-target-crate indicates which calls to save as examples. So if you do -p foo -p bar then only foo::f() and bar::f() will be saved, and not baz::f() or Vec::new().

Copy link
Member

Choose a reason for hiding this comment

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

What if foo and bar are unrelated crates though? In that what if they're both leaves of the dependency tree (or distinct roots depending on your tree perspective)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what your concern is. This flag is being passed to reverse-dependencies being scraped, not to packages being documented. So this isn't saying "documentation for crate foo should include examples for crate bar". It's saying "examples for foo and bar should be scraped from example, and those examples will be available while documenting foo and bar."

Copy link
Member

Choose a reason for hiding this comment

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

I'm still slightly concerned about this but I don't know enough about these new rustdoc flags to really bottom our my concern so "if it works it works"

} else if cx.bcx.scrape_units.len() > 0 && cx.bcx.roots.contains(unit) {
willcrichton marked this conversation as resolved.
Show resolved Hide resolved
// We only pass scraped examples to packages in the workspace (bcx.roots)
// since examples are only coming from reverse-dependencies of workspace packages

rustdoc.arg("-Zunstable-options");

for scrape_unit in &cx.bcx.scrape_units {
rustdoc
.arg("--with-examples")
.arg(scrape_output_path(scrape_unit)?);
}
}

build_deps_args(&mut rustdoc, cx, unit)?;
rustdoc::add_root_urls(cx, unit, &mut rustdoc)?;

Expand Down
1 change: 1 addition & 0 deletions src/cargo/core/compiler/timings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ impl<'cfg> Timings<'cfg> {
CompileMode::Bench => target.push_str(" (bench)"),
CompileMode::Doc { .. } => target.push_str(" (doc)"),
CompileMode::Doctest => target.push_str(" (doc test)"),
CompileMode::Docscrape => target.push_str(" (doc scrape)"),
CompileMode::RunCustomBuild => target.push_str(" (run)"),
}
let unit_time = UnitTime {
Expand Down
29 changes: 28 additions & 1 deletion src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ struct State<'a, 'cfg> {
target_data: &'a RustcTargetData<'cfg>,
profiles: &'a Profiles,
interner: &'a UnitInterner,
scrape_units: &'a [Unit],

/// A set of edges in `unit_dependencies` where (a, b) means that the
/// dependency from a to b was added purely because it was a dev-dependency.
Expand All @@ -61,6 +62,7 @@ pub fn build_unit_dependencies<'a, 'cfg>(
features: &'a ResolvedFeatures,
std_resolve: Option<&'a (Resolve, ResolvedFeatures)>,
roots: &[Unit],
scrape_units: &[Unit],
std_roots: &HashMap<CompileKind, Vec<Unit>>,
global_mode: CompileMode,
target_data: &'a RustcTargetData<'cfg>,
Expand Down Expand Up @@ -91,6 +93,7 @@ pub fn build_unit_dependencies<'a, 'cfg>(
target_data,
profiles,
interner,
scrape_units,
dev_dependency_edges: HashSet::new(),
};

Expand Down Expand Up @@ -467,6 +470,22 @@ fn compute_deps_doc(
if unit.target.is_bin() || unit.target.is_example() {
ret.extend(maybe_lib(unit, state, unit_for)?);
}

// Add all units being scraped for examples as a dependency of Doc units.
for scrape_unit in state.scrape_units.iter() {
let unit_for = UnitFor::new_normal();
deps_of(scrape_unit, state, unit_for)?;
alexcrichton marked this conversation as resolved.
Show resolved Hide resolved
ret.push(new_unit_dep(
state,
scrape_unit,
&scrape_unit.pkg,
&scrape_unit.target,
unit_for,
scrape_unit.kind,
scrape_unit.mode,
)?);
}

Ok(ret)
}

Expand Down Expand Up @@ -558,7 +577,7 @@ fn dep_build_script(
/// Choose the correct mode for dependencies.
fn check_or_build_mode(mode: CompileMode, target: &Target) -> CompileMode {
match mode {
CompileMode::Check { .. } | CompileMode::Doc { .. } => {
CompileMode::Check { .. } | CompileMode::Doc { .. } | CompileMode::Docscrape => {
if target.for_host() {
// Plugin and proc macro targets should be compiled like
// normal.
Expand Down Expand Up @@ -695,6 +714,14 @@ fn connect_run_custom_build_deps(state: &mut State<'_, '_>) {
&& other.unit.target.is_linkable()
&& other.unit.pkg.manifest().links().is_some()
})
// Avoid cycles when using the doc --scrape-examples feature:
// Say a workspace has crates A and B where A has a build-dependency on B.
// The Doc units for A and B will have a dependency on the Docscrape for both A and B.
// So this would add a dependency from B-build to A-build, causing a cycle:
// B (build) -> A (build) -> B(build)
// See the test scrape_examples_avoid_build_script_cycle for a concrete example.
// To avoid this cycle, we filter out the B -> A (docscrape) dependency.
.filter(|(_parent, other)| !other.unit.mode.is_doc_scrape())
// Skip dependencies induced via dev-dependencies since
// connections between `links` and build scripts only happens
// via normal dependencies. Otherwise since dev-dependencies can
Expand Down
4 changes: 3 additions & 1 deletion src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,9 @@ impl Profiles {
(InternedString::new("dev"), None)
}
}
CompileMode::Doc { .. } => (InternedString::new("doc"), None),
CompileMode::Doc { .. } | CompileMode::Docscrape => {
(InternedString::new("doc"), None)
}
}
} else {
(self.requested_profile, None)
Expand Down
Loading