-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Scrape code examples from examples/ directory for Rustdoc #9525
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
Changes from all commits
8acf0e8
3f9a4f2
a0122df
63c4346
49a3a54
4ac6835
b14a778
711539f
6772991
5ed35a4
d19cfd2
48056e5
ff13eb5
0c8e1f8
0b2e293
b9b39a6
dbcabc7
0792cde
82d937e
70f3821
d29ac15
8331d7d
223adac
19c8f05
4705566
17c6df7
8b06a0f
e52a9d9
b948fc8
e4a65b9
0deeea8
1120957
0a2382b
33718c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -165,7 +165,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)? | ||
|
@@ -647,6 +647,42 @@ 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 output_dir = cx.files().deps_dir(unit); | ||
Ok(output_dir.join(format!("{}.examples", unit.buildkey()))) | ||
}; | ||
|
||
if unit.mode.is_doc_scrape() { | ||
debug_assert!(cx.bcx.scrape_units.contains(unit)); | ||
|
||
rustdoc.arg("-Zunstable-options"); | ||
|
||
rustdoc | ||
.arg("--scrape-examples-output-path") | ||
.arg(scrape_output_path(unit)?); | ||
|
||
// Only scrape example for items from crates in the workspace, to reduce generated file size | ||
for pkg in cx.bcx.ws.members() { | ||
rustdoc | ||
.arg("--scrape-examples-target-crate") | ||
.arg(pkg.name()); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this interact if you do something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the other way around. If you have a file fn main() {
foo::f();
bar::f();
baz::f();
Vec::new();
} Then the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.ws.is_member(&unit.pkg) { | ||
// We only pass scraped examples to packages in the workspace | ||
// 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)?; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,6 +76,9 @@ pub struct CompileOptions { | |
/// Whether the `--document-private-items` flags was specified and should | ||
/// be forwarded to `rustdoc`. | ||
pub rustdoc_document_private_items: bool, | ||
/// Whether the `--scrape-examples` flag was specified and build flags for | ||
/// examples should be forwarded to `rustdoc`. | ||
pub rustdoc_scrape_examples: Option<CompileFilter>, | ||
/// Whether the build process should check the minimum Rust version | ||
/// defined in the cargo metadata for a crate. | ||
pub honor_rust_version: bool, | ||
|
@@ -94,12 +97,13 @@ impl<'a> CompileOptions { | |
target_rustc_args: None, | ||
local_rustdoc_args: None, | ||
rustdoc_document_private_items: false, | ||
rustdoc_scrape_examples: None, | ||
honor_rust_version: true, | ||
}) | ||
} | ||
} | ||
|
||
#[derive(Clone, PartialEq, Eq, Debug)] | ||
#[derive(PartialEq, Eq, Debug)] | ||
pub enum Packages { | ||
Default, | ||
All, | ||
|
@@ -334,6 +338,7 @@ pub fn create_bcx<'a, 'cfg>( | |
ref target_rustc_args, | ||
ref local_rustdoc_args, | ||
rustdoc_document_private_items, | ||
ref rustdoc_scrape_examples, | ||
honor_rust_version, | ||
} = *options; | ||
let config = ws.config(); | ||
|
@@ -351,7 +356,7 @@ pub fn create_bcx<'a, 'cfg>( | |
)?; | ||
} | ||
} | ||
CompileMode::Doc { .. } | CompileMode::Doctest => { | ||
CompileMode::Doc { .. } | CompileMode::Doctest | CompileMode::Docscrape => { | ||
if std::env::var("RUSTDOC_FLAGS").is_ok() { | ||
config.shell().warn( | ||
"Cargo does not read `RUSTDOC_FLAGS` environment variable. Did you mean `RUSTDOCFLAGS`?" | ||
|
@@ -363,8 +368,16 @@ pub fn create_bcx<'a, 'cfg>( | |
|
||
let target_data = RustcTargetData::new(ws, &build_config.requested_kinds)?; | ||
|
||
let specs = spec.to_package_id_specs(ws)?; | ||
let has_dev_units = if filter.need_dev_deps(build_config.mode) { | ||
let all_packages = &Packages::All; | ||
let need_reverse_dependencies = rustdoc_scrape_examples.is_some(); | ||
let full_specs = if need_reverse_dependencies { | ||
all_packages | ||
} else { | ||
spec | ||
}; | ||
|
||
let resolve_specs = full_specs.to_package_id_specs(ws)?; | ||
let has_dev_units = if filter.need_dev_deps(build_config.mode) || need_reverse_dependencies { | ||
HasDevUnits::Yes | ||
} else { | ||
HasDevUnits::No | ||
|
@@ -374,7 +387,7 @@ pub fn create_bcx<'a, 'cfg>( | |
&target_data, | ||
&build_config.requested_kinds, | ||
cli_features, | ||
&specs, | ||
&resolve_specs, | ||
has_dev_units, | ||
crate::core::resolver::features::ForceAllTargets::No, | ||
)?; | ||
|
@@ -408,6 +421,11 @@ pub fn create_bcx<'a, 'cfg>( | |
// Find the packages in the resolver that the user wants to build (those | ||
// passed in with `-p` or the defaults from the workspace), and convert | ||
// Vec<PackageIdSpec> to a Vec<PackageId>. | ||
let specs = if need_reverse_dependencies { | ||
spec.to_package_id_specs(ws)? | ||
} else { | ||
resolve_specs.clone() | ||
}; | ||
let to_build_ids = resolve.specs_to_ids(&specs)?; | ||
// Now get the `Package` for each `PackageId`. This may trigger a download | ||
// if the user specified `-p` for a dependency that is not downloaded. | ||
|
@@ -487,6 +505,30 @@ pub fn create_bcx<'a, 'cfg>( | |
interner, | ||
)?; | ||
|
||
let mut scrape_units = match rustdoc_scrape_examples { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this is one place where the final implementation might diverge significantly in a way that's better to implement nowadays. Whether or not this is necessary to do at this time I think should probably get some input from other folks. Right now with the CLI argument I think this is the proper place to do this (albeit wonky to call [lib]
doc-scrape-examples = true # the default is true, so this isn't necessary
[[example]]
name = "scratch"
doc-scrape-examples = false # in case you think this is sloppy code not to be referenced In a world like that with manifest-based configuration I don't think that this is the appropriate location to manage this. With manifest-based configuration I think you'd process and make new units as part of The end result I think roughly boils down to the same thing in that you can configure what's scraped and additionally you the representation in Cargo is "it's all in the unit graph". Not having an explicit list of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh and in terms of a "how do we roll this out" story I would imagine that you would have to specify There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok this sounds like a good plan. I'll add a link to this comment in the source. |
||
Some(scrape_filter) => { | ||
let to_build_ids = resolve.specs_to_ids(&resolve_specs)?; | ||
let to_builds = pkg_set.get_many(to_build_ids)?; | ||
let mode = CompileMode::Docscrape; | ||
|
||
generate_targets( | ||
ws, | ||
&to_builds, | ||
scrape_filter, | ||
&build_config.requested_kinds, | ||
explicit_host_kind, | ||
mode, | ||
&resolve, | ||
&workspace_resolve, | ||
&resolved_features, | ||
&pkg_set, | ||
&profiles, | ||
interner, | ||
)? | ||
} | ||
None => Vec::new(), | ||
}; | ||
|
||
let std_roots = if let Some(crates) = &config.cli_unstable().build_std { | ||
// Only build libtest if it looks like it is needed. | ||
let mut crates = crates.clone(); | ||
|
@@ -521,6 +563,7 @@ pub fn create_bcx<'a, 'cfg>( | |
&resolved_features, | ||
std_resolve_features.as_ref(), | ||
&units, | ||
&scrape_units, | ||
&std_roots, | ||
build_config.mode, | ||
&target_data, | ||
|
@@ -542,10 +585,17 @@ pub fn create_bcx<'a, 'cfg>( | |
// Rebuild the unit graph, replacing the explicit host targets with | ||
// CompileKind::Host, merging any dependencies shared with build | ||
// dependencies. | ||
let new_graph = rebuild_unit_graph_shared(interner, unit_graph, &units, explicit_host_kind); | ||
let new_graph = rebuild_unit_graph_shared( | ||
interner, | ||
unit_graph, | ||
&units, | ||
&scrape_units, | ||
explicit_host_kind, | ||
); | ||
// This would be nicer with destructuring assignment. | ||
units = new_graph.0; | ||
unit_graph = new_graph.1; | ||
scrape_units = new_graph.1; | ||
unit_graph = new_graph.2; | ||
} | ||
|
||
let mut extra_compiler_args = HashMap::new(); | ||
|
@@ -560,6 +610,7 @@ pub fn create_bcx<'a, 'cfg>( | |
} | ||
extra_compiler_args.insert(units[0].clone(), args); | ||
} | ||
|
||
for unit in &units { | ||
if unit.mode.is_doc() || unit.mode.is_doc_test() { | ||
let mut extra_args = local_rustdoc_args.clone(); | ||
|
@@ -621,6 +672,7 @@ pub fn create_bcx<'a, 'cfg>( | |
target_data, | ||
units, | ||
unit_graph, | ||
scrape_units, | ||
)?; | ||
|
||
Ok(bcx) | ||
|
@@ -742,17 +794,18 @@ impl CompileFilter { | |
match mode { | ||
CompileMode::Test | CompileMode::Doctest | CompileMode::Bench => true, | ||
CompileMode::Check { test: true } => true, | ||
CompileMode::Build | CompileMode::Doc { .. } | CompileMode::Check { test: false } => { | ||
match *self { | ||
CompileFilter::Default { .. } => false, | ||
CompileFilter::Only { | ||
ref examples, | ||
ref tests, | ||
ref benches, | ||
.. | ||
} => examples.is_specific() || tests.is_specific() || benches.is_specific(), | ||
} | ||
} | ||
CompileMode::Build | ||
| CompileMode::Doc { .. } | ||
| CompileMode::Docscrape | ||
| CompileMode::Check { test: false } => match *self { | ||
CompileFilter::Default { .. } => false, | ||
CompileFilter::Only { | ||
ref examples, | ||
ref tests, | ||
ref benches, | ||
.. | ||
} => examples.is_specific() || tests.is_specific() || benches.is_specific(), | ||
}, | ||
CompileMode::RunCustomBuild => panic!("Invalid mode"), | ||
} | ||
} | ||
|
@@ -1342,7 +1395,9 @@ fn filter_default_targets(targets: &[Target], mode: CompileMode) -> Vec<&Target> | |
}) | ||
.collect() | ||
} | ||
CompileMode::Doctest | CompileMode::RunCustomBuild => panic!("Invalid mode {:?}", mode), | ||
CompileMode::Doctest | CompileMode::Docscrape | CompileMode::RunCustomBuild => { | ||
panic!("Invalid mode {:?}", mode) | ||
} | ||
} | ||
} | ||
|
||
|
@@ -1454,8 +1509,9 @@ fn rebuild_unit_graph_shared( | |
interner: &UnitInterner, | ||
unit_graph: UnitGraph, | ||
roots: &[Unit], | ||
scrape_units: &[Unit], | ||
to_host: CompileKind, | ||
) -> (Vec<Unit>, UnitGraph) { | ||
) -> (Vec<Unit>, Vec<Unit>, UnitGraph) { | ||
let mut result = UnitGraph::new(); | ||
// Map of the old unit to the new unit, used to avoid recursing into units | ||
// that have already been computed to improve performance. | ||
|
@@ -1466,7 +1522,11 @@ fn rebuild_unit_graph_shared( | |
traverse_and_share(interner, &mut memo, &mut result, &unit_graph, root, to_host) | ||
}) | ||
.collect(); | ||
(new_roots, result) | ||
let new_scrape_units = scrape_units | ||
.iter() | ||
.map(|unit| memo.get(unit).unwrap().clone()) | ||
.collect(); | ||
(new_roots, new_scrape_units, result) | ||
} | ||
|
||
/// Recursive function for rebuilding the graph. | ||
|
Uh oh!
There was an error while loading. Please reload this page.