From 8acf0e823afddeb4901b8ecdc66efff595d3789a Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 11 May 2021 07:32:10 -0700 Subject: [PATCH 01/34] Initial support for scrape-examples --- src/bin/cargo/commands/doc.rs | 5 +++ src/cargo/core/compiler/compilation.rs | 37 +++++++++++++++++++++++ src/cargo/ops/cargo_compile.rs | 42 +++++++++++++++++++++++++- src/cargo/ops/cargo_package.rs | 1 + src/cargo/ops/cargo_test.rs | 35 ++------------------- src/cargo/util/command_prelude.rs | 1 + 6 files changed, 87 insertions(+), 34 deletions(-) diff --git a/src/bin/cargo/commands/doc.rs b/src/bin/cargo/commands/doc.rs index 875cfcfcc46..ffcfa804de1 100644 --- a/src/bin/cargo/commands/doc.rs +++ b/src/bin/cargo/commands/doc.rs @@ -19,6 +19,10 @@ 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 from examples/ directory to include as function documentation", + )) .arg_jobs() .arg_targets_lib_bin_example( "Document only this package's library", @@ -47,6 +51,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { let mut compile_opts = args.compile_options(config, mode, Some(&ws), ProfileChecking::Custom)?; compile_opts.rustdoc_document_private_items = args.is_present("document-private-items"); + compile_opts.rustdoc_scrape_examples = args.is_present("scrape-examples"); let doc_opts = DocOptions { open_result: args.is_present("open"), diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 3b21e4f43fd..fc3a3c58d73 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -101,6 +101,43 @@ pub struct Compilation<'cfg> { target_runners: HashMap)>>, } +impl Doctest { + pub fn rustdoc_process(&self, compilation: &Compilation<'_>) -> CargoResult { + let Doctest { + args, + unstable_opts, + unit, + linker: _, + script_meta, + } = self; + + let mut p = compilation.rustdoc_process(unit, *script_meta)?; + p.arg("--crate-name").arg(&unit.target.crate_name()); + p.arg("--test"); + + for &rust_dep in &[ + &compilation.deps_output[&unit.kind], + &compilation.deps_output[&CompileKind::Host], + ] { + let mut arg = OsString::from("dependency="); + arg.push(rust_dep); + p.arg("-L").arg(arg); + } + + for native_dep in compilation.native_dirs.iter() { + p.arg("-L").arg(native_dep); + } + + p.args(args); + + if *unstable_opts { + p.arg("-Zunstable-options"); + } + + Ok(p) + } +} + impl<'cfg> Compilation<'cfg> { pub fn new<'a>(bcx: &BuildContext<'a, 'cfg>) -> CargoResult> { let mut rustc = bcx.rustc().process(); diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index d37f304644c..31297aa7fef 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -24,6 +24,7 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use std::hash::{Hash, Hasher}; +use std::iter; use std::sync::Arc; use crate::core::compiler::unit_dependencies::build_unit_dependencies; @@ -76,6 +77,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: bool, /// 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 +98,13 @@ impl<'a> CompileOptions { target_rustc_args: None, local_rustdoc_args: None, rustdoc_document_private_items: false, + rustdoc_scrape_examples: false, honor_rust_version: true, }) } } -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(PartialEq, Eq, Debug)] pub enum Packages { Default, All, @@ -334,6 +339,7 @@ pub fn create_bcx<'a, 'cfg>( ref target_rustc_args, ref local_rustdoc_args, rustdoc_document_private_items, + rustdoc_scrape_examples, honor_rust_version, } = *options; let config = ws.config(); @@ -580,6 +586,40 @@ pub fn create_bcx<'a, 'cfg>( .extend(args); } } + + if rustdoc_scrape_examples { + let mut example_compile_opts = CompileOptions::new(ws.config(), CompileMode::Doctest)?; + example_compile_opts.cli_features = options.cli_features.clone(); + example_compile_opts.build_config.mode = CompileMode::Doctest; + example_compile_opts.spec = + Packages::Packages(vec![unit.pkg.manifest().name().as_str().to_owned()]); + example_compile_opts.filter = CompileFilter::Only { + all_targets: false, + lib: LibRule::False, + bins: FilterRule::none(), + examples: FilterRule::All, + benches: FilterRule::none(), + tests: FilterRule::none(), + }; + let example_compilation = ops::compile(ws, &example_compile_opts)?; + + let args = extra_compiler_args.entry(unit.clone()).or_default(); + for doc_test in example_compilation.to_doc_test.iter() { + let ex_path = doc_test.unit.target.src_path().path().unwrap(); + let ex_path_rel = ex_path.strip_prefix(doc_test.unit.pkg.root())?; + let p = doc_test.rustdoc_process(&example_compilation)?; + let arg = iter::once(format!("{}", ex_path_rel.display())) + .chain( + p.get_args() + .iter() + .map(|s| s.clone().into_string().unwrap()), + ) + .collect::>() + .join(" "); + args.extend_from_slice(&["--scrape-examples".to_owned(), arg]); + } + args.push("-Zunstable-options".to_owned()); + } } if honor_rust_version { diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 48477ea25e0..0ce23f6e080 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -765,6 +765,7 @@ fn run_verify( target_rustc_args: rustc_args, local_rustdoc_args: None, rustdoc_document_private_items: false, + rustdoc_scrape_examples: false, honor_rust_version: true, }, &exec, diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index f07f67fb53b..21563e8d6bf 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -164,13 +164,7 @@ fn run_doc_tests( let doctest_in_workspace = config.cli_unstable().doctest_in_workspace; for doctest_info in &compilation.to_doc_test { - let Doctest { - args, - unstable_opts, - unit, - linker, - script_meta, - } = doctest_info; + let Doctest { unit, linker, .. } = doctest_info; if !doctest_xcompile { match unit.kind { @@ -185,9 +179,7 @@ fn run_doc_tests( } config.shell().status("Doc-tests", unit.target.name())?; - let mut p = compilation.rustdoc_process(unit, *script_meta)?; - p.arg("--crate-name").arg(&unit.target.crate_name()); - p.arg("--test"); + let mut p = doctest_info.rustdoc_process(compilation)?; if doctest_in_workspace { add_path_args(ws, unit, &mut p); @@ -219,33 +211,10 @@ fn run_doc_tests( } } - for &rust_dep in &[ - &compilation.deps_output[&unit.kind], - &compilation.deps_output[&CompileKind::Host], - ] { - let mut arg = OsString::from("dependency="); - arg.push(rust_dep); - p.arg("-L").arg(arg); - } - - for native_dep in compilation.native_dirs.iter() { - p.arg("-L").arg(native_dep); - } - for arg in test_args { p.arg("--test-args").arg(arg); } - if config.shell().verbosity() == Verbosity::Quiet { - p.arg("--test-args").arg("--quiet"); - } - - p.args(args); - - if *unstable_opts { - p.arg("-Zunstable-options"); - } - config .shell() .verbose(|shell| shell.status("Running", p.to_string()))?; diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index d0cf17824b1..7e8857d2223 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -544,6 +544,7 @@ pub trait ArgMatchesExt { target_rustc_args: None, local_rustdoc_args: None, rustdoc_document_private_items: false, + rustdoc_scrape_examples: false, honor_rust_version: !self._is_present("ignore-rust-version"), }; From 3f9a4f2b3247b089fedfc6946c462f7a7742bca2 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 11 May 2021 08:55:44 -0700 Subject: [PATCH 02/34] Add repository URL --- src/cargo/ops/cargo_compile.rs | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 31297aa7fef..39b67137224 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -26,6 +26,7 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use std::hash::{Hash, Hasher}; use std::iter; use std::sync::Arc; +use url::Url; use crate::core::compiler::unit_dependencies::build_unit_dependencies; use crate::core::compiler::unit_graph::{self, UnitDep, UnitGraph}; @@ -588,6 +589,32 @@ pub fn create_bcx<'a, 'cfg>( } if rustdoc_scrape_examples { + let args = extra_compiler_args.entry(unit.clone()).or_default(); + + let get_repository = || -> Option { + let repository = unit.pkg.manifest().metadata().repository.as_ref()?; + let repo_url = Url::parse(repository).ok()?; + let domain = repo_url.domain()?; + + match domain { + "github.com" => { + // TODO(wcrichto): is there a canonical way to get the current commit? + // Would prefer that over linking to master + let gitref = "master"; + Some(format!("{}/tree/{}", String::from(repo_url), gitref)) + } + _ => None, + } + }; + + if let Some(repository) = get_repository() { + args.push("--repository-url".to_owned()); + args.push(repository); + } + + // While --scrape-examples and --repository-url are unstable + args.push("-Zunstable-options".to_owned()); + let mut example_compile_opts = CompileOptions::new(ws.config(), CompileMode::Doctest)?; example_compile_opts.cli_features = options.cli_features.clone(); example_compile_opts.build_config.mode = CompileMode::Doctest; @@ -603,7 +630,6 @@ pub fn create_bcx<'a, 'cfg>( }; let example_compilation = ops::compile(ws, &example_compile_opts)?; - let args = extra_compiler_args.entry(unit.clone()).or_default(); for doc_test in example_compilation.to_doc_test.iter() { let ex_path = doc_test.unit.target.src_path().path().unwrap(); let ex_path_rel = ex_path.strip_prefix(doc_test.unit.pkg.root())?; @@ -618,7 +644,6 @@ pub fn create_bcx<'a, 'cfg>( .join(" "); args.extend_from_slice(&["--scrape-examples".to_owned(), arg]); } - args.push("-Zunstable-options".to_owned()); } } From a0122df7e91a5963545d49e6d271b62d7280fd60 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 12 May 2021 15:00:56 -0700 Subject: [PATCH 03/34] Share all examples across each unit in worksapce --- src/cargo/ops/cargo_compile.rs | 108 ++++++++++++++++++--------------- 1 file changed, 59 insertions(+), 49 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 39b67137224..db2946795ff 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -567,6 +567,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(); @@ -587,62 +588,71 @@ pub fn create_bcx<'a, 'cfg>( .extend(args); } } + } - if rustdoc_scrape_examples { - let args = extra_compiler_args.entry(unit.clone()).or_default(); + if rustdoc_scrape_examples && units.len() > 0 { + let mut args = Vec::new(); - let get_repository = || -> Option { - let repository = unit.pkg.manifest().metadata().repository.as_ref()?; - let repo_url = Url::parse(repository).ok()?; - let domain = repo_url.domain()?; + let get_repository = || -> Option { + let repository = units[0].pkg.manifest().metadata().repository.as_ref()?; + let repo_url = Url::parse(repository).ok()?; + let domain = repo_url.domain()?; - match domain { - "github.com" => { - // TODO(wcrichto): is there a canonical way to get the current commit? - // Would prefer that over linking to master - let gitref = "master"; - Some(format!("{}/tree/{}", String::from(repo_url), gitref)) - } - _ => None, + match domain { + "github.com" => { + // TODO(wcrichto): is there a canonical way to get the current commit? + // Would prefer that over linking to master + let gitref = "master"; + Some(format!("{}/tree/{}", String::from(repo_url), gitref)) } - }; - - if let Some(repository) = get_repository() { - args.push("--repository-url".to_owned()); - args.push(repository); + _ => None, } + }; - // While --scrape-examples and --repository-url are unstable - args.push("-Zunstable-options".to_owned()); + if let Some(repository) = get_repository() { + args.push("--repository-url".to_owned()); + args.push(repository); + } - let mut example_compile_opts = CompileOptions::new(ws.config(), CompileMode::Doctest)?; - example_compile_opts.cli_features = options.cli_features.clone(); - example_compile_opts.build_config.mode = CompileMode::Doctest; - example_compile_opts.spec = - Packages::Packages(vec![unit.pkg.manifest().name().as_str().to_owned()]); - example_compile_opts.filter = CompileFilter::Only { - all_targets: false, - lib: LibRule::False, - bins: FilterRule::none(), - examples: FilterRule::All, - benches: FilterRule::none(), - tests: FilterRule::none(), - }; - let example_compilation = ops::compile(ws, &example_compile_opts)?; - - for doc_test in example_compilation.to_doc_test.iter() { - let ex_path = doc_test.unit.target.src_path().path().unwrap(); - let ex_path_rel = ex_path.strip_prefix(doc_test.unit.pkg.root())?; - let p = doc_test.rustdoc_process(&example_compilation)?; - let arg = iter::once(format!("{}", ex_path_rel.display())) - .chain( - p.get_args() - .iter() - .map(|s| s.clone().into_string().unwrap()), - ) - .collect::>() - .join(" "); - args.extend_from_slice(&["--scrape-examples".to_owned(), arg]); + // While --scrape-examples and --repository-url are unstable + args.push("-Zunstable-options".to_owned()); + + let mut example_compile_opts = CompileOptions::new(ws.config(), CompileMode::Doctest)?; + example_compile_opts.cli_features = options.cli_features.clone(); + example_compile_opts.build_config.mode = CompileMode::Doctest; + example_compile_opts.spec = Packages::All; + example_compile_opts.filter = CompileFilter::Only { + all_targets: false, + lib: LibRule::False, + bins: FilterRule::none(), + examples: FilterRule::All, + benches: FilterRule::none(), + tests: FilterRule::none(), + }; + let example_compilation = ops::compile(ws, &example_compile_opts)?; + + for doc_test in example_compilation.to_doc_test.iter() { + let ex_path = doc_test.unit.target.src_path().path().unwrap(); + let ex_path_rel = ex_path.strip_prefix(ws.root())?; + let p = doc_test.rustdoc_process(&example_compilation)?; + let arg = iter::once(format!("{}", ex_path_rel.display())) + .chain( + p.get_args() + .iter() + .map(|s| s.clone().into_string().unwrap()), + ) + .collect::>() + .join(" "); + args.extend_from_slice(&["--scrape-examples".to_owned(), arg]); + println!("ex: {}", ex_path_rel.display()); + } + + for unit in unit_graph.keys() { + if unit.mode.is_doc() && ws.is_member(&unit.pkg) { + extra_compiler_args + .entry(unit.clone()) + .or_default() + .extend(args.clone()); } } } From 63c4346a8f3e00f10db606e9beaa4a89539964ef Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 1 Jun 2021 14:02:45 -0700 Subject: [PATCH 04/34] Factor scraping and rendering into separate calls to rustdoc --- src/cargo/ops/cargo_compile.rs | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index db2946795ff..8382149c633 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -24,8 +24,8 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use std::hash::{Hash, Hasher}; -use std::iter; use std::sync::Arc; +use tempfile::Builder as TempFileBuilder; use url::Url; use crate::core::compiler::unit_dependencies::build_unit_dependencies; @@ -631,22 +631,24 @@ pub fn create_bcx<'a, 'cfg>( }; let example_compilation = ops::compile(ws, &example_compile_opts)?; - for doc_test in example_compilation.to_doc_test.iter() { - let ex_path = doc_test.unit.target.src_path().path().unwrap(); - let ex_path_rel = ex_path.strip_prefix(ws.root())?; - let p = doc_test.rustdoc_process(&example_compilation)?; - let arg = iter::once(format!("{}", ex_path_rel.display())) - .chain( - p.get_args() - .iter() - .map(|s| s.clone().into_string().unwrap()), - ) - .collect::>() - .join(" "); - args.extend_from_slice(&["--scrape-examples".to_owned(), arg]); - println!("ex: {}", ex_path_rel.display()); + let td = TempFileBuilder::new().prefix("cargo-doc").tempdir()?; + for (i, doc_test) in example_compilation.to_doc_test.iter().enumerate() { + let mut p = doc_test.rustdoc_process(&example_compilation)?; + p.arg(doc_test.unit.target.src_path().path().unwrap()); + + let path = td.path().join(format!("{}.json", i)); + p.arg("--scrape-examples").arg(&path); + p.arg("-Z").arg("unstable-options"); + config + .shell() + .verbose(|shell| shell.status("Running", p.to_string()))?; + p.exec()?; + args.extend_from_slice(&["--with-examples".to_string(), format!("{}", path.display())]); } + // FIXME(wcrichto): how to delete the tempdir after use outside of this function? + td.into_path(); + for unit in unit_graph.keys() { if unit.mode.is_doc() && ws.is_member(&unit.pkg) { extra_compiler_args From 49a3a54dfaf8f3748a89e1a7ad3ef21eba3d6da2 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 1 Jun 2021 19:10:28 -0700 Subject: [PATCH 05/34] Add workspace root --- src/cargo/core/compiler/context/mod.rs | 28 ++++++++++++-------------- src/cargo/ops/cargo_compile.rs | 11 ++++++++-- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 3d8967ee9ed..153c65f7888 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -206,21 +206,19 @@ impl<'a, 'cfg> Context<'a, 'cfg> { // If the unit has a build script, add `OUT_DIR` to the // environment variables. - if unit.target.is_lib() { - for dep in &self.bcx.unit_graph[unit] { - if dep.unit.mode.is_run_custom_build() { - let out_dir = self - .files() - .build_script_out_dir(&dep.unit) - .display() - .to_string(); - let script_meta = self.get_run_build_script_metadata(&dep.unit); - self.compilation - .extra_env - .entry(script_meta) - .or_insert_with(Vec::new) - .push(("OUT_DIR".to_string(), out_dir)); - } + for dep in &self.bcx.unit_graph[unit] { + if dep.unit.mode.is_run_custom_build() { + let out_dir = self + .files() + .build_script_out_dir(&dep.unit) + .display() + .to_string(); + let script_meta = self.get_run_build_script_metadata(&dep.unit); + self.compilation + .extra_env + .entry(script_meta) + .or_insert_with(Vec::new) + .push(("OUT_DIR".to_string(), out_dir)); } } diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 8382149c633..e9c8df193e3 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -614,9 +614,10 @@ pub fn create_bcx<'a, 'cfg>( args.push(repository); } - // While --scrape-examples and --repository-url are unstable + // Needed while --scrape-examples and --repository-url are unstable args.push("-Zunstable-options".to_owned()); + // Get examples via the `--examples` filter let mut example_compile_opts = CompileOptions::new(ws.config(), CompileMode::Doctest)?; example_compile_opts.cli_features = options.cli_features.clone(); example_compile_opts.build_config.mode = CompileMode::Doctest; @@ -631,13 +632,19 @@ pub fn create_bcx<'a, 'cfg>( }; let example_compilation = ops::compile(ws, &example_compile_opts)?; + // FIXME(wcrichto): ideally the call locations would be cached in the target/ directory + // so they don't have to be recomputed let td = TempFileBuilder::new().prefix("cargo-doc").tempdir()?; + for (i, doc_test) in example_compilation.to_doc_test.iter().enumerate() { let mut p = doc_test.rustdoc_process(&example_compilation)?; - p.arg(doc_test.unit.target.src_path().path().unwrap()); + let src = doc_test.unit.target.src_path().path().unwrap(); + p.arg(src); let path = td.path().join(format!("{}.json", i)); p.arg("--scrape-examples").arg(&path); + p.arg("--workspace-root") + .arg(&ws.root().display().to_string()); p.arg("-Z").arg("unstable-options"); config .shell() From 4ac68352f083007abfbcb9278953a3d2c740d945 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 2 Jun 2021 18:43:02 -0700 Subject: [PATCH 06/34] Fix for issue #9198 --- src/cargo/core/compiler/unit_dependencies.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index 91cfd92660b..b09bf48653a 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -419,7 +419,10 @@ fn compute_deps_doc( state: &mut State<'_, '_>, unit_for: UnitFor, ) -> CargoResult> { - let deps = state.deps(unit, unit_for, &|dep| dep.kind() == DepKind::Normal); + let deps = state.deps(unit, unit_for, &|dep| match dep.kind() { + DepKind::Normal | DepKind::Development => true, + DepKind::Build => false, + }); // To document a library, we depend on dependencies actually being // built. If we're documenting *all* libraries, then we also depend on From b14a778e6a4aeaf6cdc3e70c2a8a5812d3715da4 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 2 Jun 2021 18:43:27 -0700 Subject: [PATCH 07/34] Change cargo strategy to go through rustdoc instead of doctest --- src/cargo/core/compiler/build_context/mod.rs | 7 +- src/cargo/ops/cargo_compile.rs | 102 ++++++++----------- 2 files changed, 49 insertions(+), 60 deletions(-) diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index f1f31336743..05fe616b9fb 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -8,6 +8,7 @@ use crate::util::errors::CargoResult; use crate::util::interning::InternedString; use crate::util::Rustc; use std::collections::{HashMap, HashSet}; +use std::ffi::OsString; use std::path::PathBuf; mod target_info; @@ -31,7 +32,7 @@ pub struct BuildContext<'a, 'cfg> { pub build_config: &'a BuildConfig, /// Extra compiler args for either `rustc` or `rustdoc`. - pub extra_compiler_args: HashMap>, + pub extra_compiler_args: HashMap>, /// Package downloader. /// @@ -57,7 +58,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { packages: PackageSet<'cfg>, build_config: &'a BuildConfig, profiles: Profiles, - extra_compiler_args: HashMap>, + extra_compiler_args: HashMap>, target_data: RustcTargetData<'cfg>, roots: Vec, unit_graph: UnitGraph, @@ -119,7 +120,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { &self.target_data.info(unit.kind).rustdocflags } - pub fn extra_args_for(&self, unit: &Unit) -> Option<&Vec> { + pub fn extra_args_for(&self, unit: &Unit) -> Option<&Vec> { self.extra_compiler_args.get(unit) } } diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index e9c8df193e3..081cb05c501 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -23,13 +23,13 @@ //! repeats until the queue is empty. use std::collections::{BTreeSet, HashMap, HashSet}; +use std::ffi::OsString; use std::hash::{Hash, Hasher}; use std::sync::Arc; -use tempfile::Builder as TempFileBuilder; -use url::Url; use crate::core::compiler::unit_dependencies::build_unit_dependencies; use crate::core::compiler::unit_graph::{self, UnitDep, UnitGraph}; +use crate::core::compiler::Layout; use crate::core::compiler::{standard_lib, TargetInfo}; use crate::core::compiler::{BuildConfig, BuildContext, Compilation, Context}; use crate::core::compiler::{CompileKind, CompileMode, CompileTarget, RustcTargetData, Unit}; @@ -565,7 +565,10 @@ pub fn create_bcx<'a, 'cfg>( extra_args_name ); } - extra_compiler_args.insert(units[0].clone(), args); + extra_compiler_args.insert( + units[0].clone(), + args.into_iter().map(OsString::from).collect::>(), + ); } for unit in &units { @@ -585,42 +588,16 @@ pub fn create_bcx<'a, 'cfg>( extra_compiler_args .entry(unit.clone()) .or_default() - .extend(args); + .extend(args.into_iter().map(OsString::from).collect::>()); } } } - if rustdoc_scrape_examples && units.len() > 0 { - let mut args = Vec::new(); - - let get_repository = || -> Option { - let repository = units[0].pkg.manifest().metadata().repository.as_ref()?; - let repo_url = Url::parse(repository).ok()?; - let domain = repo_url.domain()?; - - match domain { - "github.com" => { - // TODO(wcrichto): is there a canonical way to get the current commit? - // Would prefer that over linking to master - let gitref = "master"; - Some(format!("{}/tree/{}", String::from(repo_url), gitref)) - } - _ => None, - } - }; - - if let Some(repository) = get_repository() { - args.push("--repository-url".to_owned()); - args.push(repository); - } - - // Needed while --scrape-examples and --repository-url are unstable - args.push("-Zunstable-options".to_owned()); - - // Get examples via the `--examples` filter - let mut example_compile_opts = CompileOptions::new(ws.config(), CompileMode::Doctest)?; + if rustdoc_scrape_examples { + let compile_mode = CompileMode::Doc { deps: false }; + let mut example_compile_opts = CompileOptions::new(ws.config(), compile_mode)?; example_compile_opts.cli_features = options.cli_features.clone(); - example_compile_opts.build_config.mode = CompileMode::Doctest; + example_compile_opts.build_config.mode = compile_mode; example_compile_opts.spec = Packages::All; example_compile_opts.filter = CompileFilter::Only { all_targets: false, @@ -630,31 +607,42 @@ pub fn create_bcx<'a, 'cfg>( benches: FilterRule::none(), tests: FilterRule::none(), }; - let example_compilation = ops::compile(ws, &example_compile_opts)?; - - // FIXME(wcrichto): ideally the call locations would be cached in the target/ directory - // so they don't have to be recomputed - let td = TempFileBuilder::new().prefix("cargo-doc").tempdir()?; - - for (i, doc_test) in example_compilation.to_doc_test.iter().enumerate() { - let mut p = doc_test.rustdoc_process(&example_compilation)?; - let src = doc_test.unit.target.src_path().path().unwrap(); - p.arg(src); - - let path = td.path().join(format!("{}.json", i)); - p.arg("--scrape-examples").arg(&path); - p.arg("--workspace-root") - .arg(&ws.root().display().to_string()); - p.arg("-Z").arg("unstable-options"); - config - .shell() - .verbose(|shell| shell.status("Running", p.to_string()))?; - p.exec()?; - args.extend_from_slice(&["--with-examples".to_string(), format!("{}", path.display())]); + example_compile_opts.rustdoc_scrape_examples = false; + + let exec: Arc = Arc::new(DefaultExecutor); + let interner = UnitInterner::new(); + let mut bcx = create_bcx(ws, &example_compile_opts, &interner)?; + let dest = bcx.profiles.get_dir_name(); + let paths = { + // FIXME(wcrichto): is there a better place to store these files? + let layout = Layout::new(ws, None, &dest)?; + let output_dir = layout.prepare_tmp()?; + bcx.roots + .iter() + .map(|unit| output_dir.join(format!("{}-calls.json", unit.buildkey()))) + .collect::>() + }; + + for (path, unit) in paths.iter().zip(bcx.roots.iter()) { + bcx.extra_compiler_args + .entry(unit.clone()) + .or_insert_with(Vec::new) + .extend_from_slice(&[ + "-Zunstable-options".into(), + "--scrape-examples".into(), + path.clone().into_os_string(), + ]); } - // FIXME(wcrichto): how to delete the tempdir after use outside of this function? - td.into_path(); + let cx = Context::new(&bcx)?; + cx.compile(&exec)?; + + let args = paths + .into_iter() + .map(|path| vec!["--with-examples".into(), path.into_os_string()].into_iter()) + .flatten() + .chain(vec!["-Zunstable-options".into()].into_iter()) + .collect::>(); for unit in unit_graph.keys() { if unit.mode.is_doc() && ws.is_member(&unit.pkg) { From 711539fee33de3006f0faf10cd83d45c136a99f6 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 2 Jun 2021 18:50:19 -0700 Subject: [PATCH 08/34] Add unstable notice for --scrape-examples --- src/bin/cargo/commands/doc.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/bin/cargo/commands/doc.rs b/src/bin/cargo/commands/doc.rs index ffcfa804de1..4364edc9cdb 100644 --- a/src/bin/cargo/commands/doc.rs +++ b/src/bin/cargo/commands/doc.rs @@ -53,6 +53,13 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { compile_opts.rustdoc_document_private_items = args.is_present("document-private-items"); compile_opts.rustdoc_scrape_examples = args.is_present("scrape-examples"); + if compile_opts.rustdoc_scrape_examples { + // FIXME(wcrichto): add a tracking issue once this gets merged and add issue number below + config + .cli_unstable() + .fail_if_stable_opt("--scrape-examples", 0)?; + } + let doc_opts = DocOptions { open_result: args.is_present("open"), compile_opts, From 67729913952af5fc027141a6a84d29b1269e1a81 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 2 Jun 2021 19:11:20 -0700 Subject: [PATCH 09/34] Remove unnecessary collect --- src/cargo/ops/cargo_compile.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 081cb05c501..55cabab8307 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -588,7 +588,7 @@ pub fn create_bcx<'a, 'cfg>( extra_compiler_args .entry(unit.clone()) .or_default() - .extend(args.into_iter().map(OsString::from).collect::>()); + .extend(args.into_iter().map(OsString::from)); } } } From 5ed35a4be70dcf156190d8b31bab0e5e3062b4c7 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 15 Jun 2021 15:20:44 -0700 Subject: [PATCH 10/34] Fix test --- tests/testsuite/rename_deps.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testsuite/rename_deps.rs b/tests/testsuite/rename_deps.rs index cc0b7173623..dd182619181 100644 --- a/tests/testsuite/rename_deps.rs +++ b/tests/testsuite/rename_deps.rs @@ -266,11 +266,11 @@ fn can_run_doc_tests() { .with_stderr_contains( "\ [DOCTEST] foo -[RUNNING] `rustdoc [..]--test [..]src/lib.rs \ +[RUNNING] `rustdoc [..]--test \ [..] \ --extern bar=[CWD]/target/debug/deps/libbar-[..].rlib \ --extern baz=[CWD]/target/debug/deps/libbar-[..].rlib \ - [..]` + [..]src/lib.rs` ", ) .run(); From d19cfd2c72cfce633bea4845b53a092cf4cc6b39 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 25 Aug 2021 16:40:45 -0700 Subject: [PATCH 11/34] Fix breakage --- src/cargo/ops/cargo_test.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index 21563e8d6bf..492fb3903d5 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -215,6 +215,10 @@ fn run_doc_tests( p.arg("--test-args").arg(arg); } + if config.shell().verbosity() == Verbosity::Quiet { + p.arg("--test-args").arg("--quiet"); + } + config .shell() .verbose(|shell| shell.status("Running", p.to_string()))?; From 48056e5a9b1516e8b5eb71a6cbb2f7cbc86f231c Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 25 Aug 2021 20:16:30 -0700 Subject: [PATCH 12/34] Allow scraping examples from library --- src/bin/cargo/commands/doc.rs | 44 ++++++++++++++++++++++++++----- src/cargo/ops/cargo_compile.rs | 25 +++++++----------- src/cargo/ops/cargo_package.rs | 2 +- src/cargo/util/command_prelude.rs | 2 +- 4 files changed, 48 insertions(+), 25 deletions(-) diff --git a/src/bin/cargo/commands/doc.rs b/src/bin/cargo/commands/doc.rs index 4364edc9cdb..b67287c5392 100644 --- a/src/bin/cargo/commands/doc.rs +++ b/src/bin/cargo/commands/doc.rs @@ -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") @@ -19,10 +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 from examples/ directory to include as function documentation", - )) + .arg( + opt( + "scrape-examples", + "Scrape examples from examples/ directory to include as function documentation", + ) + .value_name("FLAGS"), + ) .arg_jobs() .arg_targets_lib_bin_example( "Document only this package's library", @@ -51,9 +55,35 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { let mut compile_opts = args.compile_options(config, mode, Some(&ws), ProfileChecking::Custom)?; compile_opts.rustdoc_document_private_items = args.is_present("document-private-items"); - compile_opts.rustdoc_scrape_examples = args.is_present("scrape-examples"); + compile_opts.rustdoc_scrape_examples = match args.value_of("scrape-examples") { + Some(s) => { + let (examples, lib) = match s { + "all" => (true, true), + "examples" => (true, false), + "lib" => (false, true), + _ => { + return Err(CliError::from(anyhow!( + r#"--scrape-examples must take "all", "examples", or "lib" as an argument"# + ))); + } + }; + Some(CompileFilter::Only { + all_targets: false, + lib: if lib { LibRule::True } else { LibRule::False }, + bins: FilterRule::none(), + examples: if examples { + FilterRule::All + } else { + FilterRule::none() + }, + benches: FilterRule::none(), + tests: FilterRule::none(), + }) + } + None => None, + }; - if compile_opts.rustdoc_scrape_examples { + if compile_opts.rustdoc_scrape_examples.is_some() { // FIXME(wcrichto): add a tracking issue once this gets merged and add issue number below config .cli_unstable() diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 55cabab8307..ac76b666e6b 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -80,7 +80,7 @@ pub struct CompileOptions { 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: bool, + pub rustdoc_scrape_examples: Option, /// Whether the build process should check the minimum Rust version /// defined in the cargo metadata for a crate. pub honor_rust_version: bool, @@ -99,7 +99,7 @@ impl<'a> CompileOptions { target_rustc_args: None, local_rustdoc_args: None, rustdoc_document_private_items: false, - rustdoc_scrape_examples: false, + rustdoc_scrape_examples: None, honor_rust_version: true, }) } @@ -231,7 +231,7 @@ impl Packages { } } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, Clone)] pub enum LibRule { /// Include the library, fail if not present True, @@ -241,13 +241,13 @@ pub enum LibRule { False, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum FilterRule { All, Just(Vec), } -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum CompileFilter { Default { /// Flag whether targets can be safely skipped when required-features are not satisfied. @@ -340,7 +340,7 @@ pub fn create_bcx<'a, 'cfg>( ref target_rustc_args, ref local_rustdoc_args, rustdoc_document_private_items, - rustdoc_scrape_examples, + ref rustdoc_scrape_examples, honor_rust_version, } = *options; let config = ws.config(); @@ -593,21 +593,14 @@ pub fn create_bcx<'a, 'cfg>( } } - if rustdoc_scrape_examples { + if let Some(filter) = rustdoc_scrape_examples { let compile_mode = CompileMode::Doc { deps: false }; let mut example_compile_opts = CompileOptions::new(ws.config(), compile_mode)?; example_compile_opts.cli_features = options.cli_features.clone(); example_compile_opts.build_config.mode = compile_mode; example_compile_opts.spec = Packages::All; - example_compile_opts.filter = CompileFilter::Only { - all_targets: false, - lib: LibRule::False, - bins: FilterRule::none(), - examples: FilterRule::All, - benches: FilterRule::none(), - tests: FilterRule::none(), - }; - example_compile_opts.rustdoc_scrape_examples = false; + example_compile_opts.filter = filter.clone(); + example_compile_opts.rustdoc_scrape_examples = None; let exec: Arc = Arc::new(DefaultExecutor); let interner = UnitInterner::new(); diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 0ce23f6e080..fe932c21fd7 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -765,7 +765,7 @@ fn run_verify( target_rustc_args: rustc_args, local_rustdoc_args: None, rustdoc_document_private_items: false, - rustdoc_scrape_examples: false, + rustdoc_scrape_examples: None, honor_rust_version: true, }, &exec, diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 7e8857d2223..56e743d935e 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -544,7 +544,7 @@ pub trait ArgMatchesExt { target_rustc_args: None, local_rustdoc_args: None, rustdoc_document_private_items: false, - rustdoc_scrape_examples: false, + rustdoc_scrape_examples: None, honor_rust_version: !self._is_present("ignore-rust-version"), }; From ff13eb5ad101241084bdcc60cf6ef21b2d84f82e Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 26 Aug 2021 19:42:22 -0700 Subject: [PATCH 13/34] Add comments --- src/cargo/ops/cargo_compile.rs | 76 ++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 32 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index ac76b666e6b..ab6fae0a6b3 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -594,42 +594,54 @@ pub fn create_bcx<'a, 'cfg>( } if let Some(filter) = rustdoc_scrape_examples { - let compile_mode = CompileMode::Doc { deps: false }; - let mut example_compile_opts = CompileOptions::new(ws.config(), compile_mode)?; - example_compile_opts.cli_features = options.cli_features.clone(); - example_compile_opts.build_config.mode = compile_mode; - example_compile_opts.spec = Packages::All; - example_compile_opts.filter = filter.clone(); - example_compile_opts.rustdoc_scrape_examples = None; - - let exec: Arc = Arc::new(DefaultExecutor); - let interner = UnitInterner::new(); - let mut bcx = create_bcx(ws, &example_compile_opts, &interner)?; - let dest = bcx.profiles.get_dir_name(); + // Run cargo rustdoc --scrape-examples to generate calls.jsons for each file in `filter` let paths = { - // FIXME(wcrichto): is there a better place to store these files? - let layout = Layout::new(ws, None, &dest)?; - let output_dir = layout.prepare_tmp()?; - bcx.roots - .iter() - .map(|unit| output_dir.join(format!("{}-calls.json", unit.buildkey()))) - .collect::>() - }; + // Run in doc mode with the given `filter` + let compile_mode = CompileMode::Doc { deps: false }; + let mut example_compile_opts = CompileOptions::new(ws.config(), compile_mode)?; + example_compile_opts.cli_features = options.cli_features.clone(); + example_compile_opts.build_config.mode = compile_mode; + example_compile_opts.spec = Packages::All; + example_compile_opts.filter = filter.clone(); + example_compile_opts.rustdoc_scrape_examples = None; + + // Setup recursive Cargo context + let exec: Arc = Arc::new(DefaultExecutor); + let interner = UnitInterner::new(); + let mut bcx = create_bcx(ws, &example_compile_opts, &interner)?; + + // Make an output path for calls.json for each build unit + let paths = { + // FIXME(wcrichto): is there a better place to store these files? + let dest = bcx.profiles.get_dir_name(); + let layout = Layout::new(ws, None, &dest)?; + let output_dir = layout.prepare_tmp()?; + bcx.roots + .iter() + .map(|unit| output_dir.join(format!("{}-calls.json", unit.buildkey()))) + .collect::>() + }; - for (path, unit) in paths.iter().zip(bcx.roots.iter()) { - bcx.extra_compiler_args - .entry(unit.clone()) - .or_insert_with(Vec::new) - .extend_from_slice(&[ - "-Zunstable-options".into(), - "--scrape-examples".into(), - path.clone().into_os_string(), - ]); - } + // Add --scrape-examples to each build unit's rustdoc args + for (path, unit) in paths.iter().zip(bcx.roots.iter()) { + bcx.extra_compiler_args + .entry(unit.clone()) + .or_insert_with(Vec::new) + .extend_from_slice(&[ + "-Zunstable-options".into(), + "--scrape-examples".into(), + path.clone().into_os_string(), + ]); + } - let cx = Context::new(&bcx)?; - cx.compile(&exec)?; + // Invoke recursive Cargo + let cx = Context::new(&bcx)?; + cx.compile(&exec)?; + + paths + }; + // Add "--with-examples *-calls.json" to the current rustdoc invocation let args = paths .into_iter() .map(|path| vec!["--with-examples".into(), path.into_os_string()].into_iter()) From 0c8e1f826181bc6313a263558e5df0a29224c754 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 26 Aug 2021 20:41:43 -0700 Subject: [PATCH 14/34] Fix documentation --- src/bin/cargo/commands/doc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/cargo/commands/doc.rs b/src/bin/cargo/commands/doc.rs index b67287c5392..cc23eb5bfb4 100644 --- a/src/bin/cargo/commands/doc.rs +++ b/src/bin/cargo/commands/doc.rs @@ -23,7 +23,7 @@ pub fn cli() -> App { .arg( opt( "scrape-examples", - "Scrape examples from examples/ directory to include as function documentation", + "Scrape examples to include as function documentation", ) .value_name("FLAGS"), ) From 0b2e2937aeb026f83cb8df155258fc1081671bd0 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 14 Sep 2021 10:17:24 -0700 Subject: [PATCH 15/34] Add tracking issue and unstable documentation --- src/bin/cargo/commands/doc.rs | 3 +-- src/doc/src/reference/unstable.md | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/bin/cargo/commands/doc.rs b/src/bin/cargo/commands/doc.rs index cc23eb5bfb4..31a82a8176b 100644 --- a/src/bin/cargo/commands/doc.rs +++ b/src/bin/cargo/commands/doc.rs @@ -84,10 +84,9 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { }; if compile_opts.rustdoc_scrape_examples.is_some() { - // FIXME(wcrichto): add a tracking issue once this gets merged and add issue number below config .cli_unstable() - .fail_if_stable_opt("--scrape-examples", 0)?; + .fail_if_stable_opt("--scrape-examples", 9910)?; } let doc_opts = DocOptions { diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index ae550d7e5c2..69887d10fe6 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -1207,7 +1207,7 @@ for the appropriate target and influenced by any other RUSTFLAGS. * Tracking Issue: [#9778](https://github.com/rust-lang/cargo/issues/9778) * PR: [#9627](https://github.com/rust-lang/cargo/pull/9627) -The `different-binary-name` feature allows setting the filename of the binary without having to obey the +The `different-binary-name` feature allows setting the filename of the binary without having to obey the restrictions placed on crate names. For example, the crate name must use only `alphanumeric` characters or `-` or `_`, and cannot be empty. @@ -1378,7 +1378,23 @@ The 2021 edition has been stabilized in the 1.56 release. See the [`edition` field](manifest.md#the-edition-field) for more information on setting the edition. See [`cargo fix --edition`](../commands/cargo-fix.md) and [The Edition Guide](../../edition-guide/index.html) for more information on migrating existing projects. + ### Custom named profiles Custom named profiles have been stabilized in the 1.57 release. See the [profiles chapter](profiles.md#custom-profiles) for more information. + + +### scrape-examples + +* RFC: [#3123](https://github.com/rust-lang/rfcs/pull/3123) +* Tracking Issue: [#9910](https://github.com/rust-lang/cargo/issues/9910) + +The `--scrape-examples` argument to the `doc` command tells Rustdoc to search +crates in the current workspace for calls to functions. Those call-sites are then +included as documentation. The flag can take an argument of `all`, `lib`, or `examples` +which configures which crate in the workspace to analyze for examples. For instance: + +``` +cargo doc --scrape-examples examples -Z unstable-options +``` From b9b39a6ed3dcd976f180f9111bc78bc9a0fbb156 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 21 Sep 2021 11:40:01 -0700 Subject: [PATCH 16/34] Pass metadata flag to rustdoc, ensure that Doc and Check units have same metadata hash --- .../compiler/context/compilation_files.rs | 12 ++++- src/cargo/core/compiler/mod.rs | 4 ++ src/cargo/core/compiler/unit_dependencies.rs | 15 ++++-- src/cargo/ops/cargo_compile.rs | 50 ++++++++++++++++--- 4 files changed, 69 insertions(+), 12 deletions(-) diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index 2cbc5f23861..d02d9ef2341 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -540,7 +540,17 @@ fn compute_metadata( // `panic=abort` and `panic=unwind` artifacts, additionally with various // settings like debuginfo and whatnot. unit.profile.hash(&mut hasher); - unit.mode.hash(&mut hasher); + + // For RFC #3123, we need the metadata of a Check and Doc unit of the same crate + // to be the same. So we add a special case that ensures Doc and Check are hashed + // to the same value. + match unit.mode { + CompileMode::Doc { .. } | CompileMode::Check { .. } => { + (CompileMode::Check { test: false }).hash(&mut hasher) + } + mode => mode.hash(&mut hasher), + }; + cx.lto[unit].hash(&mut hasher); // Artifacts compiled for the host should have a different metadata diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index d1bd08e8204..12105f368d3 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -640,6 +640,10 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); } + // This is needed so StableCrateId can be computed correctly. + let meta = cx.files().metadata(unit); + rustdoc.arg("-C").arg(&format!("metadata={}", meta)); + add_error_format_and_color(cx, &mut rustdoc, false); add_allow_features(cx, &mut rustdoc); diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index b09bf48653a..c4475f92b1e 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -419,10 +419,17 @@ fn compute_deps_doc( state: &mut State<'_, '_>, unit_for: UnitFor, ) -> CargoResult> { - let deps = state.deps(unit, unit_for, &|dep| match dep.kind() { - DepKind::Normal | DepKind::Development => true, - DepKind::Build => false, - }); + // FIXME(wcrichto): target.is_lib() is probably not the correct way to check + // if the unit needs dev-dependencies + let deps = state.deps( + unit, + unit_for, + &|dep| match (unit.target.is_lib(), dep.kind()) { + (_, DepKind::Normal) => true, + (false, DepKind::Development) => true, + _ => false, + }, + ); // To document a library, we depend on dependencies actually being // built. If we're documenting *all* libraries, then we also depend on diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index ab6fae0a6b3..0371b006b47 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -540,6 +540,7 @@ pub fn create_bcx<'a, 'cfg>( if build_config.mode == (CompileMode::Doc { deps: true }) { remove_duplicate_doc(build_config, &units, &mut unit_graph); } + lift_doc_units_to_root(&mut units, &mut unit_graph); if build_config .requested_kinds @@ -624,14 +625,28 @@ pub fn create_bcx<'a, 'cfg>( // Add --scrape-examples to each build unit's rustdoc args for (path, unit) in paths.iter().zip(bcx.roots.iter()) { - bcx.extra_compiler_args + let args = bcx + .extra_compiler_args .entry(unit.clone()) - .or_insert_with(Vec::new) - .extend_from_slice(&[ - "-Zunstable-options".into(), - "--scrape-examples".into(), - path.clone().into_os_string(), - ]); + .or_insert_with(Vec::new); + + args.extend_from_slice(&[ + "-Zunstable-options".into(), + "--scrape-examples-output-path".into(), + path.clone().into_os_string(), + ]); + + let crate_names = units + .iter() + .map(|unit| { + vec![ + "--scrape-examples-target-crate".into(), + OsString::from(unit.pkg.name().as_str()), + ] + .into_iter() + }) + .flatten(); + args.extend(crate_names); } // Invoke recursive Cargo @@ -1680,6 +1695,27 @@ fn opt_patterns_and_names( Ok((opt_patterns, opt_names)) } +/// Removes all CompileMode::Doc units from the unit graph and lifts them to the root set. +/// +/// This is sound because rustdoc has no actual dependency on the generated files from one +/// invocation to the next. +/// +/// This is necessary because for RFC #3123, we need Doc and Check units of the same crate +/// to have the same metadata hash. This pass ensures that they have the same dependency set. +/// Also it exposes more parallelism during document generation! +fn lift_doc_units_to_root(root_units: &mut Vec, unit_graph: &mut UnitGraph) { + for deps in unit_graph.values_mut() { + deps.retain(|dep| { + if dep.unit.mode.is_doc() { + root_units.push(dep.unit.clone()); + false + } else { + true + } + }); + } +} + /// Removes duplicate CompileMode::Doc units that would cause problems with /// filename collisions. /// From dbcabc76d3b149dd6bfc478296026326a66944e1 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 21 Sep 2021 15:45:18 -0700 Subject: [PATCH 17/34] Change metadata strategy to extract hash from nested compilation --- .../compiler/context/compilation_files.rs | 12 +------ src/cargo/core/compiler/mod.rs | 6 +--- src/cargo/ops/cargo_compile.rs | 33 +++++++++++++++++++ 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index d02d9ef2341..2cbc5f23861 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -540,17 +540,7 @@ fn compute_metadata( // `panic=abort` and `panic=unwind` artifacts, additionally with various // settings like debuginfo and whatnot. unit.profile.hash(&mut hasher); - - // For RFC #3123, we need the metadata of a Check and Doc unit of the same crate - // to be the same. So we add a special case that ensures Doc and Check are hashed - // to the same value. - match unit.mode { - CompileMode::Doc { .. } | CompileMode::Check { .. } => { - (CompileMode::Check { test: false }).hash(&mut hasher) - } - mode => mode.hash(&mut hasher), - }; - + unit.mode.hash(&mut hasher); cx.lto[unit].hash(&mut hasher); // Artifacts compiled for the host should have a different metadata diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 12105f368d3..9140cc5b346 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -12,7 +12,7 @@ mod job; mod job_queue; mod layout; mod links; -mod lto; +pub mod lto; mod output_depinfo; pub mod rustdoc; pub mod standard_lib; @@ -640,10 +640,6 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); } - // This is needed so StableCrateId can be computed correctly. - let meta = cx.files().metadata(unit); - rustdoc.arg("-C").arg(&format!("metadata={}", meta)); - add_error_format_and_color(cx, &mut rustdoc, false); add_allow_features(cx, &mut rustdoc); diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 0371b006b47..4378edf3585 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -25,8 +25,10 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use std::ffi::OsString; use std::hash::{Hash, Hasher}; +use std::iter; use std::sync::Arc; +use crate::core::compiler::lto; use crate::core::compiler::unit_dependencies::build_unit_dependencies; use crate::core::compiler::unit_graph::{self, UnitDep, UnitGraph}; use crate::core::compiler::Layout; @@ -649,6 +651,37 @@ pub fn create_bcx<'a, 'cfg>( args.extend(crate_names); } + // Find the check unit corresponding to each documented crate, then add the -C metadata=... + // flag to the doc unit for that crate + { + let mut cx = Context::new(&bcx)?; + cx.lto = lto::generate(&bcx)?; + cx.prepare_units()?; + + for unit in units.iter() { + let mut root_deps = bcx + .unit_graph + .iter() + .map(|(k, v)| iter::once(k).chain(v.iter().map(|dep| &dep.unit))) + .flatten(); + + let check_unit = root_deps.find(|dep| { + dep.pkg == unit.pkg && dep.target == unit.target && dep.mode.is_check() + }); + + if let Some(check_unit) = check_unit { + let metadata = cx.files().metadata(check_unit); + extra_compiler_args + .entry(unit.clone()) + .or_default() + .extend_from_slice(&[ + "-C".into(), + OsString::from(format!("metadata={}", metadata)), + ]); + } + } + } + // Invoke recursive Cargo let cx = Context::new(&bcx)?; cx.compile(&exec)?; From 0792cde98c85214a49933f605827bb0c4fa2d75b Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 21 Sep 2021 15:50:23 -0700 Subject: [PATCH 18/34] Revert change to lift doc units --- src/cargo/ops/cargo_compile.rs | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 4378edf3585..f4259f8e4e0 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -542,7 +542,6 @@ pub fn create_bcx<'a, 'cfg>( if build_config.mode == (CompileMode::Doc { deps: true }) { remove_duplicate_doc(build_config, &units, &mut unit_graph); } - lift_doc_units_to_root(&mut units, &mut unit_graph); if build_config .requested_kinds @@ -1728,27 +1727,6 @@ fn opt_patterns_and_names( Ok((opt_patterns, opt_names)) } -/// Removes all CompileMode::Doc units from the unit graph and lifts them to the root set. -/// -/// This is sound because rustdoc has no actual dependency on the generated files from one -/// invocation to the next. -/// -/// This is necessary because for RFC #3123, we need Doc and Check units of the same crate -/// to have the same metadata hash. This pass ensures that they have the same dependency set. -/// Also it exposes more parallelism during document generation! -fn lift_doc_units_to_root(root_units: &mut Vec, unit_graph: &mut UnitGraph) { - for deps in unit_graph.values_mut() { - deps.retain(|dep| { - if dep.unit.mode.is_doc() { - root_units.push(dep.unit.clone()); - false - } else { - true - } - }); - } -} - /// Removes duplicate CompileMode::Doc units that would cause problems with /// filename collisions. /// From 82d937e3ec9020ce6a99bc4cf6f6de3534561ba0 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 21 Sep 2021 16:00:10 -0700 Subject: [PATCH 19/34] Remove references to json --- src/cargo/ops/cargo_compile.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index f4259f8e4e0..aa575d81b5f 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -596,7 +596,7 @@ pub fn create_bcx<'a, 'cfg>( } if let Some(filter) = rustdoc_scrape_examples { - // Run cargo rustdoc --scrape-examples to generate calls.jsons for each file in `filter` + // Run cargo rustdoc --scrape-examples to generate calls for each file in `filter` let paths = { // Run in doc mode with the given `filter` let compile_mode = CompileMode::Doc { deps: false }; @@ -612,7 +612,7 @@ pub fn create_bcx<'a, 'cfg>( let interner = UnitInterner::new(); let mut bcx = create_bcx(ws, &example_compile_opts, &interner)?; - // Make an output path for calls.json for each build unit + // Make an output path for calls for each build unit let paths = { // FIXME(wcrichto): is there a better place to store these files? let dest = bcx.profiles.get_dir_name(); @@ -620,7 +620,7 @@ pub fn create_bcx<'a, 'cfg>( let output_dir = layout.prepare_tmp()?; bcx.roots .iter() - .map(|unit| output_dir.join(format!("{}-calls.json", unit.buildkey()))) + .map(|unit| output_dir.join(format!("{}.calls", unit.buildkey()))) .collect::>() }; @@ -688,7 +688,7 @@ pub fn create_bcx<'a, 'cfg>( paths }; - // Add "--with-examples *-calls.json" to the current rustdoc invocation + // Add "--with-examples *.calls" to the current rustdoc invocation let args = paths .into_iter() .map(|path| vec!["--with-examples".into(), path.into_os_string()].into_iter()) From 70f38213da40a787a58bb0a5bbe6c766d5e8acb6 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 12 Oct 2021 13:17:21 -0700 Subject: [PATCH 20/34] Change scraping strategy to embed in existing unit graph, add CompileMode::Docscrape --- src/cargo/core/compiler/build_config.rs | 7 + src/cargo/core/compiler/build_context/mod.rs | 4 + .../compiler/build_context/target_info.rs | 5 +- .../compiler/context/compilation_files.rs | 4 + src/cargo/core/compiler/job_queue.rs | 1 + src/cargo/core/compiler/mod.rs | 44 +++- src/cargo/core/compiler/timings.rs | 1 + src/cargo/core/compiler/unit_dependencies.rs | 17 +- src/cargo/core/profiles.rs | 2 +- src/cargo/ops/cargo_compile.rs | 194 +++++++----------- 10 files changed, 157 insertions(+), 122 deletions(-) diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index bc829ef2ba2..3f3c769b0a2 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -149,6 +149,7 @@ pub enum CompileMode { Doc { deps: bool }, /// A target that will be tested with `rustdoc`. Doctest, + Docscrape, /// A marker for Units that represent the execution of a `build.rs` script. RunCustomBuild, } @@ -166,6 +167,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), } } @@ -187,6 +189,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 { diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index 05fe616b9fb..83cbcc11e3f 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -48,6 +48,8 @@ pub struct BuildContext<'a, 'cfg> { /// The dependency graph of units to compile. pub unit_graph: UnitGraph, + pub scrape_units: Vec, + /// The list of all kinds that are involved in this build pub all_kinds: HashSet, } @@ -62,6 +64,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { target_data: RustcTargetData<'cfg>, roots: Vec, unit_graph: UnitGraph, + scrape_units: Vec, ) -> CargoResult> { let all_kinds = unit_graph .keys() @@ -80,6 +83,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { target_data, roots, unit_graph, + scrape_units, all_kinds, }) } diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 3c032b6e1a5..6d48786573e 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -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") } } diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index 2cbc5f23861..34b29523303 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -417,6 +417,10 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { // but Cargo does not know about that. vec![] } + CompileMode::Docscrape => { + // FIXME(wcrichto): should this include the *.examples files? + vec![] + } CompileMode::Test | CompileMode::Build | CompileMode::Bench diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index c7f008972e0..a6676b7b0ce 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -1001,6 +1001,7 @@ impl<'cfg> DrainState<'cfg> { let target_name = unit.target.name(); match unit.mode { CompileMode::Doc { .. } => format!("{}(doc)", pkg_name), + CompileMode::Docscrape { .. } => format!("{}(docscrape)", pkg_name), CompileMode::RunCustomBuild => format!("{}(build)", pkg_name), CompileMode::Test | CompileMode::Check { test: true } => match unit.target.kind() { TargetKind::Lib(_) => format!("{}(test)", target_name), diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 9140cc5b346..4b6565b8158 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -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,48 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { rustdoc.args(args); } + let scrape_output_path = |unit: &Unit| -> CargoResult { + 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)?); + + for root in &cx.bcx.roots { + rustdoc + .arg("--scrape-examples-target-crate") + .arg(root.pkg.name()); + } + } else if cx.bcx.scrape_units.len() > 0 { + rustdoc.arg("-Zunstable-options"); + + for scrape_unit in &cx.bcx.scrape_units { + rustdoc + .arg("--with-examples") + .arg(scrape_output_path(scrape_unit)?); + } + + let mut all_units = cx + .bcx + .unit_graph + .values() + .map(|deps| deps.iter().map(|dep| &dep.unit)) + .flatten(); + let check_unit = all_units + .find(|other| { + unit.pkg == other.pkg && unit.target == other.target && other.mode.is_check() + }) + .with_context(|| format!("Could not find check unit for {:?}", unit))?; + let metadata = cx.files().metadata(check_unit); + rustdoc.arg("-C").arg(format!("metadata={}", metadata)); + } + build_deps_args(&mut rustdoc, cx, unit)?; rustdoc::add_root_urls(cx, unit, &mut rustdoc)?; diff --git a/src/cargo/core/compiler/timings.rs b/src/cargo/core/compiler/timings.rs index 5f694a9b0d0..33b46ce1671 100644 --- a/src/cargo/core/compiler/timings.rs +++ b/src/cargo/core/compiler/timings.rs @@ -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 { diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index c4475f92b1e..da1353ad31c 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -47,6 +47,7 @@ struct State<'a, 'cfg> { target_data: &'a RustcTargetData<'cfg>, profiles: &'a Profiles, interner: &'a UnitInterner, + scrape_roots: &'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. @@ -61,6 +62,7 @@ pub fn build_unit_dependencies<'a, 'cfg>( features: &'a ResolvedFeatures, std_resolve: Option<&'a (Resolve, ResolvedFeatures)>, roots: &[Unit], + scrape_roots: &[Unit], std_roots: &HashMap>, global_mode: CompileMode, target_data: &'a RustcTargetData<'cfg>, @@ -91,12 +93,14 @@ pub fn build_unit_dependencies<'a, 'cfg>( target_data, profiles, interner, + scrape_roots, dev_dependency_edges: HashSet::new(), }; let std_unit_deps = calc_deps_of_std(&mut state, std_roots)?; deps_of_roots(roots, &mut state)?; + deps_of_roots(scrape_roots, &mut state)?; super::links::validate_links(state.resolve(), &state.unit_dependencies)?; // Hopefully there aren't any links conflicts with the standard library? @@ -477,6 +481,17 @@ fn compute_deps_doc( if unit.target.is_bin() || unit.target.is_example() { ret.extend(maybe_lib(unit, state, unit_for)?); } + + for scrape_unit in state.scrape_roots.iter() { + ret.push(UnitDep { + unit: scrape_unit.clone(), + unit_for: unit_for.with_dependency(scrape_unit, &scrape_unit.target), + extern_crate_name: InternedString::new(""), + public: false, + noprelude: false, + }); + } + Ok(ret) } @@ -568,7 +583,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. diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index a028ca571fc..11e01f26782 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -323,7 +323,7 @@ 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) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index aa575d81b5f..41117d7010c 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -360,7 +360,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`?" @@ -496,6 +496,31 @@ pub fn create_bcx<'a, 'cfg>( interner, )?; + let mut scrape_units = match rustdoc_scrape_examples { + Some(scrape_filter) => { + let specs = Packages::All.to_package_id_specs(ws)?; + let to_build_ids = resolve.specs_to_ids(&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(); @@ -523,6 +548,24 @@ pub fn create_bcx<'a, 'cfg>( Default::default() }; + let fmt_unit = |u: &Unit| format!("{} {:?} / {:?}", u.target.name(), u.target.kind(), u.mode); + let fmt_units = |v: &[Unit]| v.iter().map(|u| fmt_unit(u)).collect::>().join(", "); + let fmt_graph = |g: &UnitGraph| { + g.iter() + .map(|(k, vs)| { + format!( + "{} =>\n{}", + fmt_unit(k), + vs.iter() + .map(|u| format!(" {}", fmt_unit(&u.unit))) + .collect::>() + .join("\n") + ) + }) + .collect::>() + .join("\n") + }; + let mut unit_graph = build_unit_dependencies( ws, &pkg_set, @@ -530,12 +573,16 @@ pub fn create_bcx<'a, 'cfg>( &resolved_features, std_resolve_features.as_ref(), &units, + &scrape_units, &std_roots, build_config.mode, &target_data, &profiles, interner, )?; + println!("SCRAPE UNITS: {}", fmt_units(&scrape_units)); + println!("BEFORE ROOTS: {}", fmt_units(&units)); + println!("BEFORE GRAPH: {}", fmt_graph(&unit_graph)); // TODO: In theory, Cargo should also dedupe the roots, but I'm uncertain // what heuristics to use in that case. @@ -551,11 +598,20 @@ 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; } + println!("AFTER UNITS: {}", fmt_units(&units)); + println!("AFTER GRAPH: {}", fmt_graph(&unit_graph)); let mut extra_compiler_args = HashMap::new(); if let Some(args) = extra_args { @@ -595,117 +651,6 @@ pub fn create_bcx<'a, 'cfg>( } } - if let Some(filter) = rustdoc_scrape_examples { - // Run cargo rustdoc --scrape-examples to generate calls for each file in `filter` - let paths = { - // Run in doc mode with the given `filter` - let compile_mode = CompileMode::Doc { deps: false }; - let mut example_compile_opts = CompileOptions::new(ws.config(), compile_mode)?; - example_compile_opts.cli_features = options.cli_features.clone(); - example_compile_opts.build_config.mode = compile_mode; - example_compile_opts.spec = Packages::All; - example_compile_opts.filter = filter.clone(); - example_compile_opts.rustdoc_scrape_examples = None; - - // Setup recursive Cargo context - let exec: Arc = Arc::new(DefaultExecutor); - let interner = UnitInterner::new(); - let mut bcx = create_bcx(ws, &example_compile_opts, &interner)?; - - // Make an output path for calls for each build unit - let paths = { - // FIXME(wcrichto): is there a better place to store these files? - let dest = bcx.profiles.get_dir_name(); - let layout = Layout::new(ws, None, &dest)?; - let output_dir = layout.prepare_tmp()?; - bcx.roots - .iter() - .map(|unit| output_dir.join(format!("{}.calls", unit.buildkey()))) - .collect::>() - }; - - // Add --scrape-examples to each build unit's rustdoc args - for (path, unit) in paths.iter().zip(bcx.roots.iter()) { - let args = bcx - .extra_compiler_args - .entry(unit.clone()) - .or_insert_with(Vec::new); - - args.extend_from_slice(&[ - "-Zunstable-options".into(), - "--scrape-examples-output-path".into(), - path.clone().into_os_string(), - ]); - - let crate_names = units - .iter() - .map(|unit| { - vec![ - "--scrape-examples-target-crate".into(), - OsString::from(unit.pkg.name().as_str()), - ] - .into_iter() - }) - .flatten(); - args.extend(crate_names); - } - - // Find the check unit corresponding to each documented crate, then add the -C metadata=... - // flag to the doc unit for that crate - { - let mut cx = Context::new(&bcx)?; - cx.lto = lto::generate(&bcx)?; - cx.prepare_units()?; - - for unit in units.iter() { - let mut root_deps = bcx - .unit_graph - .iter() - .map(|(k, v)| iter::once(k).chain(v.iter().map(|dep| &dep.unit))) - .flatten(); - - let check_unit = root_deps.find(|dep| { - dep.pkg == unit.pkg && dep.target == unit.target && dep.mode.is_check() - }); - - if let Some(check_unit) = check_unit { - let metadata = cx.files().metadata(check_unit); - extra_compiler_args - .entry(unit.clone()) - .or_default() - .extend_from_slice(&[ - "-C".into(), - OsString::from(format!("metadata={}", metadata)), - ]); - } - } - } - - // Invoke recursive Cargo - let cx = Context::new(&bcx)?; - cx.compile(&exec)?; - - paths - }; - - // Add "--with-examples *.calls" to the current rustdoc invocation - let args = paths - .into_iter() - .map(|path| vec!["--with-examples".into(), path.into_os_string()].into_iter()) - .flatten() - .chain(vec!["-Zunstable-options".into()].into_iter()) - .collect::>(); - - for unit in unit_graph.keys() { - if unit.mode.is_doc() && ws.is_member(&unit.pkg) { - extra_compiler_args - .entry(unit.clone()) - .or_default() - .extend(args.clone()); - } - } - } - if honor_rust_version { // Remove any pre-release identifiers for easier comparison let current_version = &target_data.rustc.version; @@ -745,6 +690,7 @@ pub fn create_bcx<'a, 'cfg>( target_data, units, unit_graph, + scrape_units, )?; Ok(bcx) @@ -864,7 +810,10 @@ impl CompileFilter { pub fn need_dev_deps(&self, mode: CompileMode) -> bool { match mode { - CompileMode::Test | CompileMode::Doctest | CompileMode::Bench => true, + CompileMode::Test + | CompileMode::Doctest + | CompileMode::Bench + | CompileMode::Docscrape => true, CompileMode::Check { test: true } => true, CompileMode::Build | CompileMode::Doc { .. } | CompileMode::Check { test: false } => { match *self { @@ -1466,7 +1415,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) + } } } @@ -1578,8 +1529,9 @@ fn rebuild_unit_graph_shared( interner: &UnitInterner, unit_graph: UnitGraph, roots: &[Unit], + scrape_units: &[Unit], to_host: CompileKind, -) -> (Vec, UnitGraph) { +) -> (Vec, Vec, 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. @@ -1590,7 +1542,13 @@ 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| { + traverse_and_share(interner, &mut memo, &mut result, &unit_graph, unit, to_host) + }) + .collect(); + (new_roots, new_scrape_units, result) } /// Recursive function for rebuilding the graph. From d29ac156f2db877bbae09c91917d526bfb08cf99 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 12 Oct 2021 13:19:58 -0700 Subject: [PATCH 21/34] Remove unused code --- src/cargo/core/compiler/context/mod.rs | 28 ++++++++++++++------------ src/cargo/core/compiler/mod.rs | 2 +- src/cargo/ops/cargo_compile.rs | 3 --- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 153c65f7888..3d8967ee9ed 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -206,19 +206,21 @@ impl<'a, 'cfg> Context<'a, 'cfg> { // If the unit has a build script, add `OUT_DIR` to the // environment variables. - for dep in &self.bcx.unit_graph[unit] { - if dep.unit.mode.is_run_custom_build() { - let out_dir = self - .files() - .build_script_out_dir(&dep.unit) - .display() - .to_string(); - let script_meta = self.get_run_build_script_metadata(&dep.unit); - self.compilation - .extra_env - .entry(script_meta) - .or_insert_with(Vec::new) - .push(("OUT_DIR".to_string(), out_dir)); + if unit.target.is_lib() { + for dep in &self.bcx.unit_graph[unit] { + if dep.unit.mode.is_run_custom_build() { + let out_dir = self + .files() + .build_script_out_dir(&dep.unit) + .display() + .to_string(); + let script_meta = self.get_run_build_script_metadata(&dep.unit); + self.compilation + .extra_env + .entry(script_meta) + .or_insert_with(Vec::new) + .push(("OUT_DIR".to_string(), out_dir)); + } } } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 4b6565b8158..ea05e07338a 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -12,7 +12,7 @@ mod job; mod job_queue; mod layout; mod links; -pub mod lto; +mod lto; mod output_depinfo; pub mod rustdoc; pub mod standard_lib; diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 41117d7010c..e10b199528a 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -25,13 +25,10 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use std::ffi::OsString; use std::hash::{Hash, Hasher}; -use std::iter; use std::sync::Arc; -use crate::core::compiler::lto; use crate::core::compiler::unit_dependencies::build_unit_dependencies; use crate::core::compiler::unit_graph::{self, UnitDep, UnitGraph}; -use crate::core::compiler::Layout; use crate::core::compiler::{standard_lib, TargetInfo}; use crate::core::compiler::{BuildConfig, BuildContext, Compilation, Context}; use crate::core::compiler::{CompileKind, CompileMode, CompileTarget, RustcTargetData, Unit}; From 8331d7d151b00c4380b40928764bb322d8200b08 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 12 Oct 2021 14:22:17 -0700 Subject: [PATCH 22/34] Remove more unused code --- src/cargo/core/compiler/build_context/mod.rs | 7 ++-- src/cargo/core/compiler/compilation.rs | 37 -------------------- src/cargo/core/compiler/job_queue.rs | 1 - src/cargo/core/compiler/mod.rs | 2 +- src/cargo/ops/cargo_compile.rs | 31 ++-------------- src/cargo/ops/cargo_test.rs | 31 ++++++++++++++-- tests/testsuite/rename_deps.rs | 4 +-- 7 files changed, 37 insertions(+), 76 deletions(-) diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index 83cbcc11e3f..d947f5a2bc3 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -8,7 +8,6 @@ use crate::util::errors::CargoResult; use crate::util::interning::InternedString; use crate::util::Rustc; use std::collections::{HashMap, HashSet}; -use std::ffi::OsString; use std::path::PathBuf; mod target_info; @@ -32,7 +31,7 @@ pub struct BuildContext<'a, 'cfg> { pub build_config: &'a BuildConfig, /// Extra compiler args for either `rustc` or `rustdoc`. - pub extra_compiler_args: HashMap>, + pub extra_compiler_args: HashMap>, /// Package downloader. /// @@ -60,7 +59,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { packages: PackageSet<'cfg>, build_config: &'a BuildConfig, profiles: Profiles, - extra_compiler_args: HashMap>, + extra_compiler_args: HashMap>, target_data: RustcTargetData<'cfg>, roots: Vec, unit_graph: UnitGraph, @@ -124,7 +123,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { &self.target_data.info(unit.kind).rustdocflags } - pub fn extra_args_for(&self, unit: &Unit) -> Option<&Vec> { + pub fn extra_args_for(&self, unit: &Unit) -> Option<&Vec> { self.extra_compiler_args.get(unit) } } diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index fc3a3c58d73..3b21e4f43fd 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -101,43 +101,6 @@ pub struct Compilation<'cfg> { target_runners: HashMap)>>, } -impl Doctest { - pub fn rustdoc_process(&self, compilation: &Compilation<'_>) -> CargoResult { - let Doctest { - args, - unstable_opts, - unit, - linker: _, - script_meta, - } = self; - - let mut p = compilation.rustdoc_process(unit, *script_meta)?; - p.arg("--crate-name").arg(&unit.target.crate_name()); - p.arg("--test"); - - for &rust_dep in &[ - &compilation.deps_output[&unit.kind], - &compilation.deps_output[&CompileKind::Host], - ] { - let mut arg = OsString::from("dependency="); - arg.push(rust_dep); - p.arg("-L").arg(arg); - } - - for native_dep in compilation.native_dirs.iter() { - p.arg("-L").arg(native_dep); - } - - p.args(args); - - if *unstable_opts { - p.arg("-Zunstable-options"); - } - - Ok(p) - } -} - impl<'cfg> Compilation<'cfg> { pub fn new<'a>(bcx: &BuildContext<'a, 'cfg>) -> CargoResult> { let mut rustc = bcx.rustc().process(); diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index a6676b7b0ce..c7f008972e0 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -1001,7 +1001,6 @@ impl<'cfg> DrainState<'cfg> { let target_name = unit.target.name(); match unit.mode { CompileMode::Doc { .. } => format!("{}(doc)", pkg_name), - CompileMode::Docscrape { .. } => format!("{}(docscrape)", pkg_name), CompileMode::RunCustomBuild => format!("{}(build)", pkg_name), CompileMode::Test | CompileMode::Check { test: true } => match unit.target.kind() { TargetKind::Lib(_) => format!("{}(test)", target_name), diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index ea05e07338a..abc2a11f228 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -665,7 +665,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { .arg("--scrape-examples-target-crate") .arg(root.pkg.name()); } - } else if cx.bcx.scrape_units.len() > 0 { + } else if cx.bcx.scrape_units.len() > 0 && cx.bcx.roots.contains(unit) { rustdoc.arg("-Zunstable-options"); for scrape_unit in &cx.bcx.scrape_units { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index e10b199528a..cff3a456d5e 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -23,7 +23,6 @@ //! repeats until the queue is empty. use std::collections::{BTreeSet, HashMap, HashSet}; -use std::ffi::OsString; use std::hash::{Hash, Hasher}; use std::sync::Arc; @@ -545,24 +544,6 @@ pub fn create_bcx<'a, 'cfg>( Default::default() }; - let fmt_unit = |u: &Unit| format!("{} {:?} / {:?}", u.target.name(), u.target.kind(), u.mode); - let fmt_units = |v: &[Unit]| v.iter().map(|u| fmt_unit(u)).collect::>().join(", "); - let fmt_graph = |g: &UnitGraph| { - g.iter() - .map(|(k, vs)| { - format!( - "{} =>\n{}", - fmt_unit(k), - vs.iter() - .map(|u| format!(" {}", fmt_unit(&u.unit))) - .collect::>() - .join("\n") - ) - }) - .collect::>() - .join("\n") - }; - let mut unit_graph = build_unit_dependencies( ws, &pkg_set, @@ -577,9 +558,6 @@ pub fn create_bcx<'a, 'cfg>( &profiles, interner, )?; - println!("SCRAPE UNITS: {}", fmt_units(&scrape_units)); - println!("BEFORE ROOTS: {}", fmt_units(&units)); - println!("BEFORE GRAPH: {}", fmt_graph(&unit_graph)); // TODO: In theory, Cargo should also dedupe the roots, but I'm uncertain // what heuristics to use in that case. @@ -607,8 +585,6 @@ pub fn create_bcx<'a, 'cfg>( scrape_units = new_graph.1; unit_graph = new_graph.2; } - println!("AFTER UNITS: {}", fmt_units(&units)); - println!("AFTER GRAPH: {}", fmt_graph(&unit_graph)); let mut extra_compiler_args = HashMap::new(); if let Some(args) = extra_args { @@ -620,10 +596,7 @@ pub fn create_bcx<'a, 'cfg>( extra_args_name ); } - extra_compiler_args.insert( - units[0].clone(), - args.into_iter().map(OsString::from).collect::>(), - ); + extra_compiler_args.insert(units[0].clone(), args); } for unit in &units { @@ -643,7 +616,7 @@ pub fn create_bcx<'a, 'cfg>( extra_compiler_args .entry(unit.clone()) .or_default() - .extend(args.into_iter().map(OsString::from)); + .extend(args); } } } diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index 492fb3903d5..f07f67fb53b 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -164,7 +164,13 @@ fn run_doc_tests( let doctest_in_workspace = config.cli_unstable().doctest_in_workspace; for doctest_info in &compilation.to_doc_test { - let Doctest { unit, linker, .. } = doctest_info; + let Doctest { + args, + unstable_opts, + unit, + linker, + script_meta, + } = doctest_info; if !doctest_xcompile { match unit.kind { @@ -179,7 +185,9 @@ fn run_doc_tests( } config.shell().status("Doc-tests", unit.target.name())?; - let mut p = doctest_info.rustdoc_process(compilation)?; + let mut p = compilation.rustdoc_process(unit, *script_meta)?; + p.arg("--crate-name").arg(&unit.target.crate_name()); + p.arg("--test"); if doctest_in_workspace { add_path_args(ws, unit, &mut p); @@ -211,6 +219,19 @@ fn run_doc_tests( } } + for &rust_dep in &[ + &compilation.deps_output[&unit.kind], + &compilation.deps_output[&CompileKind::Host], + ] { + let mut arg = OsString::from("dependency="); + arg.push(rust_dep); + p.arg("-L").arg(arg); + } + + for native_dep in compilation.native_dirs.iter() { + p.arg("-L").arg(native_dep); + } + for arg in test_args { p.arg("--test-args").arg(arg); } @@ -219,6 +240,12 @@ fn run_doc_tests( p.arg("--test-args").arg("--quiet"); } + p.args(args); + + if *unstable_opts { + p.arg("-Zunstable-options"); + } + config .shell() .verbose(|shell| shell.status("Running", p.to_string()))?; diff --git a/tests/testsuite/rename_deps.rs b/tests/testsuite/rename_deps.rs index dd182619181..cc0b7173623 100644 --- a/tests/testsuite/rename_deps.rs +++ b/tests/testsuite/rename_deps.rs @@ -266,11 +266,11 @@ fn can_run_doc_tests() { .with_stderr_contains( "\ [DOCTEST] foo -[RUNNING] `rustdoc [..]--test \ +[RUNNING] `rustdoc [..]--test [..]src/lib.rs \ [..] \ --extern bar=[CWD]/target/debug/deps/libbar-[..].rlib \ --extern baz=[CWD]/target/debug/deps/libbar-[..].rlib \ - [..]src/lib.rs` + [..]` ", ) .run(); From 223adaca5525d3556ea40b5d3c05ad7798b3a5b0 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 12 Oct 2021 14:25:36 -0700 Subject: [PATCH 23/34] Formatting --- src/cargo/core/compiler/build_config.rs | 8 ++++++-- src/cargo/core/profiles.rs | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index 3f3c769b0a2..7126080fea6 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -138,7 +138,9 @@ pub enum CompileMode { /// Building a target with `rustc` to emit `rmeta` metadata only. If /// `test` is true, then it is also compiled with `--test` to check it like /// a test. - Check { test: bool }, + Check { + test: bool, + }, /// Used to indicate benchmarks should be built. This is not used in /// `Unit`, because it is essentially the same as `Test` (indicating /// `--test` should be passed to rustc) and by using `Test` instead it @@ -146,7 +148,9 @@ pub enum CompileMode { Bench, /// A target that will be documented with `rustdoc`. /// If `deps` is true, then it will also document all dependencies. - Doc { deps: bool }, + Doc { + deps: bool, + }, /// A target that will be tested with `rustdoc`. Doctest, Docscrape, diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 11e01f26782..fe88e724e28 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -323,7 +323,9 @@ impl Profiles { (InternedString::new("dev"), None) } } - CompileMode::Doc { .. } | CompileMode::Docscrape => (InternedString::new("doc"), None), + CompileMode::Doc { .. } | CompileMode::Docscrape => { + (InternedString::new("doc"), None) + } } } else { (self.requested_profile, None) From 19c8f05fcc26344007428a5f1e10822399dd58c8 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 12 Oct 2021 17:57:20 -0700 Subject: [PATCH 24/34] Fix issues compiling build scripts --- src/bin/cargo/commands/doc.rs | 39 +++++++---------- src/cargo/core/compiler/mod.rs | 45 +++++++++++--------- src/cargo/core/compiler/unit_dependencies.rs | 30 ++++++------- src/cargo/ops/cargo_compile.rs | 4 +- 4 files changed, 54 insertions(+), 64 deletions(-) diff --git a/src/bin/cargo/commands/doc.rs b/src/bin/cargo/commands/doc.rs index 31a82a8176b..5a2ca6a01e9 100644 --- a/src/bin/cargo/commands/doc.rs +++ b/src/bin/cargo/commands/doc.rs @@ -56,30 +56,21 @@ 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"); compile_opts.rustdoc_scrape_examples = match args.value_of("scrape-examples") { - Some(s) => { - let (examples, lib) = match s { - "all" => (true, true), - "examples" => (true, false), - "lib" => (false, true), - _ => { - return Err(CliError::from(anyhow!( - r#"--scrape-examples must take "all", "examples", or "lib" as an argument"# - ))); - } - }; - Some(CompileFilter::Only { - all_targets: false, - lib: if lib { LibRule::True } else { LibRule::False }, - bins: FilterRule::none(), - examples: if examples { - FilterRule::All - } else { - FilterRule::none() - }, - benches: FilterRule::none(), - tests: FilterRule::none(), - }) - } + 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", "examples", or "lib" as an argument"# + ))); + } + }), None => None, }; diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index abc2a11f228..97061dd5c7f 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -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}; @@ -660,33 +661,37 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { .arg("--scrape-examples-output-path") .arg(scrape_output_path(unit)?); - for root in &cx.bcx.roots { - rustdoc - .arg("--scrape-examples-target-crate") - .arg(root.pkg.name()); + let root_packages = cx + .bcx + .roots + .iter() + .map(|root| root.pkg.name()) + .collect::>(); + for pkg in root_packages { + rustdoc.arg("--scrape-examples-target-crate").arg(pkg); } } else if cx.bcx.scrape_units.len() > 0 && cx.bcx.roots.contains(unit) { - rustdoc.arg("-Zunstable-options"); - - for scrape_unit in &cx.bcx.scrape_units { - rustdoc - .arg("--with-examples") - .arg(scrape_output_path(scrape_unit)?); - } - - let mut all_units = cx + let all_units = cx .bcx .unit_graph .values() .map(|deps| deps.iter().map(|dep| &dep.unit)) .flatten(); - let check_unit = all_units - .find(|other| { - unit.pkg == other.pkg && unit.target == other.target && other.mode.is_check() - }) - .with_context(|| format!("Could not find check unit for {:?}", unit))?; - let metadata = cx.files().metadata(check_unit); - rustdoc.arg("-C").arg(format!("metadata={}", metadata)); + let check_unit = all_units.into_iter().find(|other| { + unit.pkg == other.pkg && unit.target == other.target && other.mode.is_check() + }); + if let Some(check_unit) = check_unit { + let metadata = cx.files().metadata(check_unit); + rustdoc.arg("-C").arg(format!("metadata={}", metadata)); + + 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)?; diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index da1353ad31c..c33d2378efa 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -100,7 +100,6 @@ pub fn build_unit_dependencies<'a, 'cfg>( let std_unit_deps = calc_deps_of_std(&mut state, std_roots)?; deps_of_roots(roots, &mut state)?; - deps_of_roots(scrape_roots, &mut state)?; super::links::validate_links(state.resolve(), &state.unit_dependencies)?; // Hopefully there aren't any links conflicts with the standard library? @@ -425,15 +424,7 @@ fn compute_deps_doc( ) -> CargoResult> { // FIXME(wcrichto): target.is_lib() is probably not the correct way to check // if the unit needs dev-dependencies - let deps = state.deps( - unit, - unit_for, - &|dep| match (unit.target.is_lib(), dep.kind()) { - (_, DepKind::Normal) => true, - (false, DepKind::Development) => true, - _ => false, - }, - ); + let deps = state.deps(unit, unit_for, &|dep| dep.kind() == DepKind::Normal); // To document a library, we depend on dependencies actually being // built. If we're documenting *all* libraries, then we also depend on @@ -483,13 +474,17 @@ fn compute_deps_doc( } for scrape_unit in state.scrape_roots.iter() { - ret.push(UnitDep { - unit: scrape_unit.clone(), - unit_for: unit_for.with_dependency(scrape_unit, &scrape_unit.target), - extern_crate_name: InternedString::new(""), - public: false, - noprelude: false, - }); + let unit_for = UnitFor::new_normal(); + deps_of(scrape_unit, state, unit_for)?; + ret.push(new_unit_dep( + state, + scrape_unit, + &scrape_unit.pkg, + &scrape_unit.target, + unit_for, + scrape_unit.kind, + scrape_unit.mode, + )?); } Ok(ret) @@ -720,6 +715,7 @@ fn connect_run_custom_build_deps(state: &mut State<'_, '_>) { && other.unit.target.is_linkable() && other.unit.pkg.manifest().links().is_some() }) + .filter(|(_, 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 diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index cff3a456d5e..732c0c5dd8a 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -1514,9 +1514,7 @@ fn rebuild_unit_graph_shared( .collect(); let new_scrape_units = scrape_units .iter() - .map(|unit| { - traverse_and_share(interner, &mut memo, &mut result, &unit_graph, unit, to_host) - }) + .map(|unit| memo.get(unit).unwrap().clone()) .collect(); (new_roots, new_scrape_units, result) } From 470556603e1bc8b2db8127bf729afa9f51505a3e Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 12 Oct 2021 19:11:07 -0700 Subject: [PATCH 25/34] Add documentation and a test --- src/cargo/core/compiler/build_config.rs | 9 ++-- src/cargo/core/compiler/build_context/mod.rs | 1 + src/cargo/core/compiler/mod.rs | 49 ++++++++++++-------- src/cargo/core/compiler/unit_dependencies.rs | 13 +++--- tests/testsuite/doc.rs | 37 +++++++++++++++ 5 files changed, 78 insertions(+), 31 deletions(-) diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index 7126080fea6..00733f38ab8 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -138,9 +138,7 @@ pub enum CompileMode { /// Building a target with `rustc` to emit `rmeta` metadata only. If /// `test` is true, then it is also compiled with `--test` to check it like /// a test. - Check { - test: bool, - }, + Check { test: bool }, /// Used to indicate benchmarks should be built. This is not used in /// `Unit`, because it is essentially the same as `Test` (indicating /// `--test` should be passed to rustc) and by using `Test` instead it @@ -148,11 +146,10 @@ pub enum CompileMode { Bench, /// A target that will be documented with `rustdoc`. /// If `deps` is true, then it will also document all dependencies. - Doc { - deps: bool, - }, + 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, /// A marker for Units that represent the execution of a `build.rs` script. RunCustomBuild, diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index d947f5a2bc3..624a6e88ae6 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -47,6 +47,7 @@ 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, /// The list of all kinds that are involved in this build diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 97061dd5c7f..2f8d7bf7d64 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -648,6 +648,30 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { rustdoc.args(args); } + // 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. + let matching_units = cx + .bcx + .unit_graph + .keys() + .filter(|other| { + unit.pkg == other.pkg && unit.target == other.target && !other.mode.is_doc_scrape() + }) + .collect::>(); + 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); + let metadata = cx.files().metadata(metadata_unit); + rustdoc.arg("-C").arg(format!("metadata={}", metadata)); + let scrape_output_path = |unit: &Unit| -> CargoResult { let layout = cx.files().layout(unit.kind); let output_dir = layout.prepare_tmp()?; @@ -661,6 +685,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { .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 @@ -671,26 +696,12 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { rustdoc.arg("--scrape-examples-target-crate").arg(pkg); } } else if cx.bcx.scrape_units.len() > 0 && cx.bcx.roots.contains(unit) { - let all_units = cx - .bcx - .unit_graph - .values() - .map(|deps| deps.iter().map(|dep| &dep.unit)) - .flatten(); - let check_unit = all_units.into_iter().find(|other| { - unit.pkg == other.pkg && unit.target == other.target && other.mode.is_check() - }); - if let Some(check_unit) = check_unit { - let metadata = cx.files().metadata(check_unit); - rustdoc.arg("-C").arg(format!("metadata={}", metadata)); - - rustdoc.arg("-Zunstable-options"); + rustdoc.arg("-Zunstable-options"); - for scrape_unit in &cx.bcx.scrape_units { - rustdoc - .arg("--with-examples") - .arg(scrape_output_path(scrape_unit)?); - } + for scrape_unit in &cx.bcx.scrape_units { + rustdoc + .arg("--with-examples") + .arg(scrape_output_path(scrape_unit)?); } } diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index c33d2378efa..ea592e1496e 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -47,7 +47,7 @@ struct State<'a, 'cfg> { target_data: &'a RustcTargetData<'cfg>, profiles: &'a Profiles, interner: &'a UnitInterner, - scrape_roots: &'a [Unit], + 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. @@ -62,7 +62,7 @@ pub fn build_unit_dependencies<'a, 'cfg>( features: &'a ResolvedFeatures, std_resolve: Option<&'a (Resolve, ResolvedFeatures)>, roots: &[Unit], - scrape_roots: &[Unit], + scrape_units: &[Unit], std_roots: &HashMap>, global_mode: CompileMode, target_data: &'a RustcTargetData<'cfg>, @@ -93,7 +93,7 @@ pub fn build_unit_dependencies<'a, 'cfg>( target_data, profiles, interner, - scrape_roots, + scrape_units, dev_dependency_edges: HashSet::new(), }; @@ -422,8 +422,6 @@ fn compute_deps_doc( state: &mut State<'_, '_>, unit_for: UnitFor, ) -> CargoResult> { - // FIXME(wcrichto): target.is_lib() is probably not the correct way to check - // if the unit needs dev-dependencies let deps = state.deps(unit, unit_for, &|dep| dep.kind() == DepKind::Normal); // To document a library, we depend on dependencies actually being @@ -473,7 +471,8 @@ fn compute_deps_doc( ret.extend(maybe_lib(unit, state, unit_for)?); } - for scrape_unit in state.scrape_roots.iter() { + // 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)?; ret.push(new_unit_dep( @@ -715,6 +714,8 @@ 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 --scrape-examples feature + // FIXME(wcrichto): unclear why this exact filter is the fix .filter(|(_, other)| !other.unit.mode.is_doc_scrape()) // Skip dependencies induced via dev-dependencies since // connections between `links` and build scripts only happens diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 05a6c78aa89..b42717ae46c 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -2148,3 +2148,40 @@ fn doc_fingerprint_unusual_behavior() { assert!(build_doc.join("somefile").exists()); assert!(real_doc.join("somefile").exists()); } + +#[cargo_test] +fn scrape_examples() { + if !is_nightly() { + // --scrape-examples is unstable + return; + } + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + "#, + ) + .file("examples/ex.rs", "fn main() { foo::foo(); }") + .file("src/lib.rs", "pub fn foo() {}\npub fn bar() { foo(); }") + .build(); + + p.cargo("doc -Zunstable-options --scrape-examples all") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[..] foo v0.0.1 ([CWD]) +[..] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + + let doc_html = p.read_file("target/doc/foo/fn.foo.html"); + assert!(doc_html.contains("Examples found in repository")); + assert!(doc_html.contains("More examples")); +} From 17c6df70a112e2166b8c079d88d024167098fa41 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 12 Oct 2021 19:24:26 -0700 Subject: [PATCH 26/34] Remove references to "lib" argument --- src/bin/cargo/commands/doc.rs | 2 +- src/doc/src/reference/unstable.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bin/cargo/commands/doc.rs b/src/bin/cargo/commands/doc.rs index 5a2ca6a01e9..c34f95f793f 100644 --- a/src/bin/cargo/commands/doc.rs +++ b/src/bin/cargo/commands/doc.rs @@ -67,7 +67,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { ), _ => { return Err(CliError::from(anyhow!( - r#"--scrape-examples must take "all", "examples", or "lib" as an argument"# + r#"--scrape-examples must take "all" or "examples" as an argument"# ))); } }), diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 69887d10fe6..7fd19e3a1c7 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -1392,9 +1392,9 @@ Custom named profiles have been stabilized in the 1.57 release. See the The `--scrape-examples` argument to the `doc` command tells Rustdoc to search crates in the current workspace for calls to functions. Those call-sites are then -included as documentation. The flag can take an argument of `all`, `lib`, or `examples` +included as documentation. The flag can take an argument of `all` or `examples` which configures which crate in the workspace to analyze for examples. For instance: ``` -cargo doc --scrape-examples examples -Z unstable-options +cargo doc -Z unstable-options --scrape-examples examples ``` From 8b06a0f2f7c920f801ee93f718bfef5b9a80dd24 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 12 Oct 2021 19:33:23 -0700 Subject: [PATCH 27/34] Update rustdoc tests with -Cmetadata flag --- tests/testsuite/rustdoc.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/testsuite/rustdoc.rs b/tests/testsuite/rustdoc.rs index 3d27739ef92..5650f3e0a5b 100644 --- a/tests/testsuite/rustdoc.rs +++ b/tests/testsuite/rustdoc.rs @@ -32,6 +32,7 @@ fn rustdoc_args() { -o [CWD]/target/doc \ [..] \ --cfg=foo \ + -C metadata=[..] \ -L dependency=[CWD]/target/debug/deps [..]` [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", @@ -83,6 +84,7 @@ fn rustdoc_foo_with_bar_dependency() { -o [CWD]/target/doc \ [..] \ --cfg=foo \ + -C metadata=[..] \ -L dependency=[CWD]/target/debug/deps \ --extern [..]` [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] @@ -122,6 +124,7 @@ fn rustdoc_only_bar_dependency() { -o [CWD]/target/doc \ [..] \ --cfg=foo \ + -C metadata=[..] \ -L dependency=[CWD]/target/debug/deps [..]` [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", @@ -144,6 +147,7 @@ fn rustdoc_same_name_documents_lib() { -o [CWD]/target/doc \ [..] \ --cfg=foo \ + -C metadata=[..] \ -L dependency=[CWD]/target/debug/deps [..]` [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", From e52a9d9f698075a265a4b79468a7876c971abe7e Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 13 Oct 2021 17:20:16 -0700 Subject: [PATCH 28/34] Add comments and todos --- src/bin/cargo/commands/doc.rs | 3 ++ .../compiler/context/compilation_files.rs | 3 +- src/cargo/core/compiler/mod.rs | 3 ++ src/cargo/ops/cargo_compile.rs | 28 +++++++++---------- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/bin/cargo/commands/doc.rs b/src/bin/cargo/commands/doc.rs index c34f95f793f..7f6585688ca 100644 --- a/src/bin/cargo/commands/doc.rs +++ b/src/bin/cargo/commands/doc.rs @@ -55,6 +55,9 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { let mut compile_opts = 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(), diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index 34b29523303..51a833030d2 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -418,7 +418,8 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { vec![] } CompileMode::Docscrape => { - // FIXME(wcrichto): should this include the *.examples files? + // Docscrape only generates temporary *.examples files to pass to rustdoc + // so they're not important to track vec![] } CompileMode::Test diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 2f8d7bf7d64..bce4db8627c 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -696,6 +696,9 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { rustdoc.arg("--scrape-examples-target-crate").arg(pkg); } } else if cx.bcx.scrape_units.len() > 0 && cx.bcx.roots.contains(unit) { + // 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 { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 732c0c5dd8a..883034caa0a 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -780,22 +780,20 @@ impl CompileFilter { pub fn need_dev_deps(&self, mode: CompileMode) -> bool { match mode { - CompileMode::Test - | CompileMode::Doctest - | CompileMode::Bench - | CompileMode::Docscrape => true, + 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"), } } From b948fc86c083ac6a20b1426eb729d7c4ff4bea37 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 14 Oct 2021 16:06:26 -0700 Subject: [PATCH 29/34] Add test / documentation for scrape-examples cycle-avoidance, add map for doc unit -> metadata to Context --- src/cargo/core/compiler/context/mod.rs | 42 +++++++++++++++++ src/cargo/core/compiler/mod.rs | 23 +-------- src/cargo/core/compiler/unit_dependencies.rs | 11 +++-- tests/testsuite/doc.rs | 49 +++++++++++++++++++- 4 files changed, 99 insertions(+), 26 deletions(-) diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 3d8967ee9ed..702923e7003 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -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, + + /// 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, } impl<'a, 'cfg> Context<'a, 'cfg> { @@ -121,6 +125,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { rustc_clients: HashMap::new(), pipelining, lto: HashMap::new(), + metadata_for_doc_units: HashMap::new(), }) } @@ -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 @@ -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::>(); + 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)); + } + } } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index bce4db8627c..6dde4a58436 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -648,28 +648,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { rustdoc.args(args); } - // 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. - let matching_units = cx - .bcx - .unit_graph - .keys() - .filter(|other| { - unit.pkg == other.pkg && unit.target == other.target && !other.mode.is_doc_scrape() - }) - .collect::>(); - 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); - let metadata = cx.files().metadata(metadata_unit); + let metadata = cx.metadata_for_doc_units[&unit]; rustdoc.arg("-C").arg(format!("metadata={}", metadata)); let scrape_output_path = |unit: &Unit| -> CargoResult { diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index ea592e1496e..effe4841513 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -714,9 +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 --scrape-examples feature - // FIXME(wcrichto): unclear why this exact filter is the fix - .filter(|(_, other)| !other.unit.mode.is_doc_scrape()) + // 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 diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index b42717ae46c..9bf66eceba1 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -2150,7 +2150,7 @@ fn doc_fingerprint_unusual_behavior() { } #[cargo_test] -fn scrape_examples() { +fn scrape_examples_basic() { if !is_nightly() { // --scrape-examples is unstable return; @@ -2185,3 +2185,50 @@ fn scrape_examples() { assert!(doc_html.contains("Examples found in repository")); assert!(doc_html.contains("More examples")); } + +#[cargo_test] +fn scrape_examples_avoid_build_script_cycle() { + if !is_nightly() { + // --scrape-examples is unstable + return; + } + + let p = project() + // package with build dependency + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + links = "foo" + + [workspace] + members = ["bar"] + + [build-dependencies] + bar = {path = "bar"} + "#, + ) + .file("src/lib.rs", "") + .file("build.rs", "fn main(){}") + // dependency + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + links = "bar" + "#, + ) + .file("bar/src/lib.rs", "") + .file("bar/build.rs", "fn main(){}") + .build(); + + p.cargo("doc --all -Zunstable-options --scrape-examples all") + .masquerade_as_nightly_cargo() + .run(); +} From e4a65b91be8e6db6f09e7e6083182b99f1f8574c Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 27 Oct 2021 11:42:13 -0700 Subject: [PATCH 30/34] Fix several bugs when checking wasmtime repo: * Docscrape unit not having dev-dependencies included * Sources for reverse-dependencies generated to the wrong directory * Incorrect features being selected for Docscrape units * Panics from Docscrape-dependent packages not being available --- src/cargo/core/compiler/compilation.rs | 6 ++++- .../compiler/context/compilation_files.rs | 14 ++++++++---- src/cargo/core/compiler/mod.rs | 5 +++-- src/cargo/core/compiler/unit_dependencies.rs | 4 +++- src/cargo/ops/cargo_compile.rs | 22 +++++++++++++------ tests/testsuite/doc.rs | 3 +++ 6 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 3b21e4f43fd..ac243ac85ea 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -188,7 +188,11 @@ impl<'cfg> Compilation<'cfg> { unit: &Unit, script_meta: Option, ) -> CargoResult { - let rustdoc = ProcessBuilder::new(&*self.config.rustdoc()?); + let mut rustdoc = ProcessBuilder::new(&*self.config.rustdoc()?); + if self.config.extra_verbose() { + rustdoc.display_env_vars(); + } + let cmd = fill_rustc_tool_env(rustdoc, unit); let mut p = self.fill_env(cmd, &unit.pkg, script_meta, unit.kind, true)?; unit.target.edition().cmd_edition_arg(&mut p); diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index 51a833030d2..b5e069ae931 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -191,7 +191,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { /// Returns the directory where the artifacts for the given unit are /// initially created. pub fn out_dir(&self, unit: &Unit) -> PathBuf { - if unit.mode.is_doc() { + if unit.mode.is_doc() || unit.mode.is_doc_scrape() { self.layout(unit.kind).doc().to_path_buf() } else if unit.mode.is_doc_test() { panic!("doc tests do not have an out dir"); @@ -418,9 +418,15 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { vec![] } CompileMode::Docscrape => { - // Docscrape only generates temporary *.examples files to pass to rustdoc - // so they're not important to track - vec![] + let path = self + .deps_dir(unit) + .join(format!("{}.examples", unit.buildkey())); + vec![OutputFile { + path, + hardlink: None, + export_path: None, + flavor: FileFlavor::Normal, + }] } CompileMode::Test | CompileMode::Build diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 6dde4a58436..2a2f3e74e91 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -652,12 +652,13 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { rustdoc.arg("-C").arg(format!("metadata={}", metadata)); let scrape_output_path = |unit: &Unit| -> CargoResult { - let layout = cx.files().layout(unit.kind); - let output_dir = layout.prepare_tmp()?; + 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 diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index effe4841513..db73143fe88 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -256,6 +256,7 @@ fn compute_deps( if !dep.is_transitive() && !unit.target.is_test() && !unit.target.is_example() + && !unit.mode.is_doc_scrape() && !unit.mode.is_any_test() { return false; @@ -473,7 +474,8 @@ fn compute_deps_doc( // 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(); + // This needs to match the FeaturesFor used in cargo_compile::generate_targets. + let unit_for = UnitFor::new_host(scrape_unit.target.proc_macro()); deps_of(scrape_unit, state, unit_for)?; ret.push(new_unit_dep( state, diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 883034caa0a..5810083920c 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -368,18 +368,27 @@ 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) { - HasDevUnits::Yes + let all_packages = &Packages::All; + let full_specs = if rustdoc_scrape_examples.is_some() { + all_packages } else { - HasDevUnits::No + spec }; + + let specs = spec.to_package_id_specs(ws)?; + let resolve_specs = full_specs.to_package_id_specs(ws)?; + let has_dev_units = + if filter.need_dev_deps(build_config.mode) || rustdoc_scrape_examples.is_some() { + HasDevUnits::Yes + } else { + HasDevUnits::No + }; let resolve = ops::resolve_ws_with_opts( ws, &target_data, &build_config.requested_kinds, cli_features, - &specs, + &resolve_specs, has_dev_units, crate::core::resolver::features::ForceAllTargets::No, )?; @@ -494,8 +503,7 @@ pub fn create_bcx<'a, 'cfg>( let mut scrape_units = match rustdoc_scrape_examples { Some(scrape_filter) => { - let specs = Packages::All.to_package_id_specs(ws)?; - let to_build_ids = resolve.specs_to_ids(&specs)?; + let to_build_ids = resolve.specs_to_ids(&resolve_specs)?; let to_builds = pkg_set.get_many(to_build_ids)?; let mode = CompileMode::Docscrape; diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 9bf66eceba1..80f50046475 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -2184,6 +2184,9 @@ fn scrape_examples_basic() { let doc_html = p.read_file("target/doc/foo/fn.foo.html"); assert!(doc_html.contains("Examples found in repository")); assert!(doc_html.contains("More examples")); + + // Ensure that the reverse-dependency has its sources generated + assert!(p.build_dir().join("doc/src/ex/ex.rs.html").exists()); } #[cargo_test] From 0deeea83126136dbb9fba26810a26e6a5442495b Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 27 Oct 2021 11:51:13 -0700 Subject: [PATCH 31/34] Remove unnecessary clones, document out_dir --- src/cargo/core/compiler/context/compilation_files.rs | 2 ++ src/cargo/ops/cargo_compile.rs | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index b5e069ae931..37ab2520223 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -191,6 +191,8 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { /// Returns the directory where the artifacts for the given unit are /// initially created. pub fn out_dir(&self, unit: &Unit) -> PathBuf { + // Docscrape units need to have doc/ set as the out_dir so sources for reverse-dependencies + // will be put into doc/ and not into deps/ where the *.examples files are stored. if unit.mode.is_doc() || unit.mode.is_doc_scrape() { self.layout(unit.kind).doc().to_path_buf() } else if unit.mode.is_doc_test() { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 5810083920c..8594a174b3d 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -229,7 +229,7 @@ impl Packages { } } -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq)] pub enum LibRule { /// Include the library, fail if not present True, @@ -239,13 +239,13 @@ pub enum LibRule { False, } -#[derive(Debug, Clone)] +#[derive(Debug)] pub enum FilterRule { All, Just(Vec), } -#[derive(Debug, Clone)] +#[derive(Debug)] pub enum CompileFilter { Default { /// Flag whether targets can be safely skipped when required-features are not satisfied. From 11209570c960eede4489b9a7eb9e6d7fa26610c7 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 28 Oct 2021 00:35:34 -0700 Subject: [PATCH 32/34] Change scraping to apply to all workspace packages instead of just root units. Only attach Docscrape unit dependencies to workspace Doc units. Add test for scraping examples with complex reverse dependencies. --- src/cargo/core/compiler/compilation.rs | 6 +- src/cargo/core/compiler/mod.rs | 19 +++--- src/cargo/core/compiler/unit_dependencies.rs | 28 +++++---- tests/testsuite/doc.rs | 63 ++++++++++++++++++++ 4 files changed, 86 insertions(+), 30 deletions(-) diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index ac243ac85ea..3b21e4f43fd 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -188,11 +188,7 @@ impl<'cfg> Compilation<'cfg> { unit: &Unit, script_meta: Option, ) -> CargoResult { - let mut rustdoc = ProcessBuilder::new(&*self.config.rustdoc()?); - if self.config.extra_verbose() { - rustdoc.display_env_vars(); - } - + let rustdoc = ProcessBuilder::new(&*self.config.rustdoc()?); let cmd = fill_rustc_tool_env(rustdoc, unit); let mut p = self.fill_env(cmd, &unit.pkg, script_meta, unit.kind, true)?; unit.target.edition().cmd_edition_arg(&mut p); diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 2a2f3e74e91..2f58b60abeb 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -21,7 +21,6 @@ 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}; @@ -665,18 +664,14 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { .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::>(); - for pkg in root_packages { - rustdoc.arg("--scrape-examples-target-crate").arg(pkg); + // 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()); } - } else if cx.bcx.scrape_units.len() > 0 && cx.bcx.roots.contains(unit) { - // We only pass scraped examples to packages in the workspace (bcx.roots) + } 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"); diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index db73143fe88..ae8ca69aa46 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -473,19 +473,21 @@ fn compute_deps_doc( } // Add all units being scraped for examples as a dependency of Doc units. - for scrape_unit in state.scrape_units.iter() { - // This needs to match the FeaturesFor used in cargo_compile::generate_targets. - let unit_for = UnitFor::new_host(scrape_unit.target.proc_macro()); - deps_of(scrape_unit, state, unit_for)?; - ret.push(new_unit_dep( - state, - scrape_unit, - &scrape_unit.pkg, - &scrape_unit.target, - unit_for, - scrape_unit.kind, - scrape_unit.mode, - )?); + if state.ws.is_member(&unit.pkg) { + for scrape_unit in state.scrape_units.iter() { + // This needs to match the FeaturesFor used in cargo_compile::generate_targets. + let unit_for = UnitFor::new_host(scrape_unit.target.proc_macro()); + deps_of(scrape_unit, state, unit_for)?; + ret.push(new_unit_dep( + state, + scrape_unit, + &scrape_unit.pkg, + &scrape_unit.target, + unit_for, + scrape_unit.kind, + scrape_unit.mode, + )?); + } } Ok(ret) diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 80f50046475..9f278e318ad 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -2235,3 +2235,66 @@ fn scrape_examples_avoid_build_script_cycle() { .masquerade_as_nightly_cargo() .run(); } + + +#[cargo_test] +fn scrape_examples_complex_reverse_dependencies() { + if !is_nightly() { + // --scrape-examples is unstable + return; + } + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dev-dependencies] + a = {path = "a", features = ["feature"]} + b = {path = "b"} + + [workspace] + members = ["b"] + "#, + ) + .file("src/lib.rs", "") + .file("examples/ex.rs", "fn main() { a::f(); }") + .file( + "a/Cargo.toml", + r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + + [lib] + proc-macro = true + + [dependencies] + b = {path = "../b"} + + [features] + feature = [] + "#, + ) + .file("a/src/lib.rs", "#[cfg(feature)] pub fn f();") + .file( + "b/Cargo.toml", + r#" + [package] + name = "b" + version = "0.0.1" + authors = [] + "#, + ) + .file("b/src/lib.rs", "") + .build(); + + p.cargo("doc -Zunstable-options --scrape-examples all") + .masquerade_as_nightly_cargo() + .run(); +} From 0a2382b6db9ab40f91a3bd02b159c334811961b2 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 28 Oct 2021 00:38:16 -0700 Subject: [PATCH 33/34] Formatting --- tests/testsuite/doc.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 9f278e318ad..922a100c8ad 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -2236,7 +2236,6 @@ fn scrape_examples_avoid_build_script_cycle() { .run(); } - #[cargo_test] fn scrape_examples_complex_reverse_dependencies() { if !is_nightly() { @@ -2281,7 +2280,7 @@ fn scrape_examples_complex_reverse_dependencies() { feature = [] "#, ) - .file("a/src/lib.rs", "#[cfg(feature)] pub fn f();") + .file("a/src/lib.rs", "#[cfg(feature)] pub fn f();") .file( "b/Cargo.toml", r#" From 33718c7eef67b6f8c4b63922584bbc802811b2e9 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 28 Oct 2021 09:01:47 -0700 Subject: [PATCH 34/34] Fix repeated warning with two calls to to_package_id_specs --- src/cargo/ops/cargo_compile.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 8594a174b3d..046147f1910 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -369,20 +369,19 @@ pub fn create_bcx<'a, 'cfg>( let target_data = RustcTargetData::new(ws, &build_config.requested_kinds)?; let all_packages = &Packages::All; - let full_specs = if rustdoc_scrape_examples.is_some() { + let need_reverse_dependencies = rustdoc_scrape_examples.is_some(); + let full_specs = if need_reverse_dependencies { all_packages } else { spec }; - let specs = spec.to_package_id_specs(ws)?; let resolve_specs = full_specs.to_package_id_specs(ws)?; - let has_dev_units = - if filter.need_dev_deps(build_config.mode) || rustdoc_scrape_examples.is_some() { - HasDevUnits::Yes - } else { - HasDevUnits::No - }; + let has_dev_units = if filter.need_dev_deps(build_config.mode) || need_reverse_dependencies { + HasDevUnits::Yes + } else { + HasDevUnits::No + }; let resolve = ops::resolve_ws_with_opts( ws, &target_data, @@ -422,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 to a Vec. + 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.