diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 40ab6aaf38e9..086781f69f04 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -77,6 +77,13 @@ pub struct Context<'a, 'cfg> { /// 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, + + /// Map that tracks whether a unit completed successfully. Used in conjuction + /// with the `Unit::can_fail` flag, so jobs can dynamically track at runtime + /// whether their dependencies succeeded or failed. Currently used for + /// the Rustdoc scrape-examples feature to allow Rustdoc to proceed even if + /// examples fail to compile. + pub completed_units: Arc>>, } impl<'a, 'cfg> Context<'a, 'cfg> { @@ -115,6 +122,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { rustc_clients: HashMap::new(), lto: HashMap::new(), metadata_for_doc_units: HashMap::new(), + completed_units: Arc::new(Mutex::new(HashMap::new())), }) } diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index a63949b69d12..b8f9dc27c06d 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -126,6 +126,10 @@ struct DrainState<'cfg> { total_units: usize, queue: DependencyQueue, + /// Dependency map that is like JobQueue::dep_map, except with Job information removed. + /// Used to determine if a unit's dependencies have failed, see + /// [`DrainState::spawn_work_if_possible`]. + dep_map: HashMap>, messages: Arc>, /// Diagnostic deduplication support. diag_dedupe: DiagDedupe<'cfg>, @@ -506,8 +510,15 @@ impl<'cfg> JobQueue<'cfg> { self.queue.queue_finished(); let progress = Progress::with_style("Building", ProgressStyle::Ratio, cx.bcx.config); + let dep_map = self + .queue + .dep_map() + .iter() + .map(|(unit, (deps, _))| (unit.clone(), deps.clone())) + .collect(); let state = DrainState { total_units: self.queue.len(), + dep_map, queue: self.queue, // 100 here is somewhat arbitrary. It is a few screenfulls of // output, and hopefully at most a few megabytes of memory for @@ -578,6 +589,32 @@ impl<'cfg> DrainState<'cfg> { // start requesting job tokens. Each job after the first needs to // request a token. while let Some((unit, job)) = self.queue.dequeue() { + // First, we handle the special case of fallible units. If + // this unit is allowed to fail, and any one of its dependencies + // has failed, then we should immediately mark it as failed and + // skip executing it. + if unit.can_fail { + let mut completed_units = cx.completed_units.lock().unwrap(); + let failed_deps = self.dep_map[&unit] + .iter() + .filter(|(dep_unit, _)| { + let dep_meta = cx.files().metadata(dep_unit); + !completed_units[&dep_meta] + }) + .map(|(_, artifact)| artifact) + .collect::>(); + if !failed_deps.is_empty() { + // TODO: should put a warning here saying which units were skipped + // due to failed dependencies. + for artifact in failed_deps { + self.queue.finish(&unit, artifact); + } + let unit_meta = cx.files().metadata(&unit); + completed_units.insert(unit_meta, false); + continue; + } + } + self.pending_queue.push((unit, job)); if self.active.len() + self.pending_queue.len() > 1 { jobserver_helper.request_token(); @@ -713,7 +750,8 @@ impl<'cfg> DrainState<'cfg> { }; debug!("end ({:?}): {:?}", unit, result); match result { - Ok(()) => self.finish(id, &unit, artifact, cx)?, + Ok(()) => self.finish(id, &unit, artifact, cx, true)?, + Err(_) if unit.can_fail => self.finish(id, &unit, artifact, cx, false)?, Err(error) => { let msg = "The following warnings were emitted during compilation:"; self.emit_warnings(Some(msg), &unit, cx)?; @@ -1161,6 +1199,7 @@ impl<'cfg> DrainState<'cfg> { unit: &Unit, artifact: Artifact, cx: &mut Context<'_, '_>, + success: bool, ) -> CargoResult<()> { if unit.mode.is_run_custom_build() && unit.show_warnings(cx.bcx.config) { self.emit_warnings(None, unit, cx)?; @@ -1170,6 +1209,11 @@ impl<'cfg> DrainState<'cfg> { Artifact::All => self.timings.unit_finished(id, unlocked), Artifact::Metadata => self.timings.unit_rmeta_finished(id, unlocked), } + cx.completed_units + .lock() + .unwrap() + .insert(cx.files().metadata(unit), success); + Ok(()) } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index bfab9cd65812..86bfcd33d9b7 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -22,7 +22,7 @@ mod unit; pub mod unit_dependencies; pub mod unit_graph; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::env; use std::ffi::{OsStr, OsString}; use std::fs::{self, File}; @@ -639,9 +639,9 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { let metadata = cx.metadata_for_doc_units[unit]; rustdoc.arg("-C").arg(format!("metadata={}", metadata)); - let scrape_output_path = |unit: &Unit| -> CargoResult { + let scrape_output_path = |unit: &Unit| -> PathBuf { let output_dir = cx.files().deps_dir(unit); - Ok(output_dir.join(format!("{}.examples", unit.buildkey()))) + output_dir.join(format!("{}.examples", unit.buildkey())) }; if unit.mode.is_doc_scrape() { @@ -651,7 +651,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { rustdoc .arg("--scrape-examples-output-path") - .arg(scrape_output_path(unit)?); + .arg(scrape_output_path(unit)); // Only scrape example for items from crates in the workspace, to reduce generated file size for pkg in cx.bcx.ws.members() { @@ -664,18 +664,18 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { rustdoc.arg("--scrape-examples-target-crate").arg(name); } } - } else if cx.bcx.scrape_units.len() > 0 && cx.bcx.ws.unit_needs_doc_scrape(unit) { - // We only pass scraped examples to packages in the workspace - // since examples are only coming from reverse-dependencies of workspace packages + } + let should_include_scrape_units = + cx.bcx.scrape_units.len() > 0 && cx.bcx.ws.unit_needs_doc_scrape(unit); + let scrape_outputs = should_include_scrape_units.then(|| { rustdoc.arg("-Zunstable-options"); - - for scrape_unit in &cx.bcx.scrape_units { - rustdoc - .arg("--with-examples") - .arg(scrape_output_path(scrape_unit)?); - } - } + cx.bcx + .scrape_units + .iter() + .map(|unit| (cx.files().metadata(unit), scrape_output_path(unit))) + .collect::>() + }); build_deps_args(&mut rustdoc, cx, unit)?; rustdoc::add_root_urls(cx, unit, &mut rustdoc)?; @@ -693,12 +693,27 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { let target = Target::clone(&unit.target); let mut output_options = OutputOptions::new(cx, unit); let script_metadata = cx.find_build_script_metadata(unit); + let completed_units = Arc::clone(&cx.completed_units); Ok(Work::new(move |state| { add_custom_flags( &mut rustdoc, &build_script_outputs.lock().unwrap(), script_metadata, )?; + + // Add the output of scraped examples to the rustdoc command. + // This action must happen after the unit's dependencies have finished, + // because some of those deps may be Docscrape units which have failed. + // So we dynamically determine which `--with-examples` flags to pass here. + if let Some(scrape_outputs) = scrape_outputs { + let completed_units = completed_units.lock().unwrap(); + for (metadata, output_path) in &scrape_outputs { + if completed_units[metadata] { + rustdoc.arg("--with-examples").arg(output_path); + } + } + } + let crate_dir = doc_dir.join(&crate_name); if crate_dir.exists() { // Remove output from a previous build. This ensures that stale @@ -706,6 +721,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { debug!("removing pre-existing doc directory {:?}", crate_dir); paths::remove_dir_all(crate_dir)?; } + state.running(&rustdoc); rustdoc diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index e0baebd516b8..3d03dabb3d3c 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/src/cargo/core/compiler/standard_lib.rs @@ -217,6 +217,7 @@ pub fn generate_std_roots( /*is_std*/ true, /*dep_hash*/ 0, IsArtifact::No, + false, )); } } diff --git a/src/cargo/core/compiler/unit.rs b/src/cargo/core/compiler/unit.rs index 32a98cb8cd70..5578de87d90b 100644 --- a/src/cargo/core/compiler/unit.rs +++ b/src/cargo/core/compiler/unit.rs @@ -72,6 +72,7 @@ pub struct UnitInner { /// This value initially starts as 0, and then is filled in via a /// second-pass after all the unit dependencies have been computed. pub dep_hash: u64, + pub can_fail: bool, } impl UnitInner { @@ -141,6 +142,7 @@ impl fmt::Debug for Unit { .field("artifact", &self.artifact.is_true()) .field("is_std", &self.is_std) .field("dep_hash", &self.dep_hash) + .field("can_fail", &self.can_fail) .finish() } } @@ -184,6 +186,7 @@ impl UnitInterner { is_std: bool, dep_hash: u64, artifact: IsArtifact, + can_fail: bool, ) -> Unit { let target = match (is_std, target.kind()) { // This is a horrible hack to support build-std. `libstd` declares @@ -216,6 +219,7 @@ impl UnitInterner { is_std, dep_hash, artifact, + can_fail, }); Unit { inner } } diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index 4bd8aedf2b25..570b531521a2 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -888,6 +888,7 @@ fn new_unit_dep_with_profile( state.is_std, /*dep_hash*/ 0, artifact.map_or(IsArtifact::No, |_| IsArtifact::Yes), + parent.can_fail, ); Ok(UnitDep { unit, diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 2a9b32e8ca3e..0a810e9d5877 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -663,6 +663,7 @@ unstable_cli_options!( // TODO(wcrichto): move scrape example configuration into Cargo.toml before stabilization // See: https://github.com/rust-lang/cargo/pull/9525#discussion_r728470927 rustdoc_scrape_examples: Option = ("Allow rustdoc to scrape examples from reverse-dependencies for documentation"), + ignore_scrape_failures: bool = ("When scraping examples for Rustdoc, don't stop compilation if an example fails"), skip_rustdoc_fingerprint: bool = (HIDDEN), ); @@ -936,6 +937,7 @@ impl CliUnstable { ) } } + "ignore-scrape-failures" => self.ignore_scrape_failures = parse_empty(k, v)?, "skip-rustdoc-fingerprint" => self.skip_rustdoc_fingerprint = parse_empty(k, v)?, "compile-progress" => stabilized_warn(k, "1.30", STABILIZED_COMPILE_PROGRESS), "offline" => stabilized_err(k, "1.36", STABILIZED_OFFLINE)?, diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index ee74c31a4de8..ef0022bca6e0 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -1517,7 +1517,7 @@ impl<'cfg> Workspace<'cfg> { // (not documented) or proc macros (have no scrape-able exports). Additionally, // naively passing a proc macro's unit_for to new_unit_dep will currently cause // Cargo to panic, see issue #10545. - self.is_member(&unit.pkg) && !unit.target.for_host() + self.is_member(&unit.pkg) && !unit.target.for_host() && unit.mode.is_doc() } } diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 5d273295ae35..dd2753739926 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -1069,6 +1069,7 @@ fn generate_targets( /*is_std*/ false, /*dep_hash*/ 0, IsArtifact::No, + mode.is_doc_scrape() && ws.config().cli_unstable().ignore_scrape_failures, ); units.insert(unit); } @@ -1631,6 +1632,7 @@ fn traverse_and_share( unit.is_std, new_dep_hash, unit.artifact, + unit.can_fail, ); assert!(memo.insert(unit.clone(), new_unit.clone()).is_none()); new_graph.entry(new_unit.clone()).or_insert(new_deps); @@ -1872,6 +1874,7 @@ fn override_rustc_crate_types( unit.is_std, unit.dep_hash, unit.artifact, + unit.can_fail, ) }; units[0] = match unit.target.kind() { diff --git a/src/cargo/util/dependency_queue.rs b/src/cargo/util/dependency_queue.rs index 5a3289eb7ee7..208ba311a512 100644 --- a/src/cargo/util/dependency_queue.rs +++ b/src/cargo/util/dependency_queue.rs @@ -170,6 +170,10 @@ impl DependencyQueue { self.dep_map.len() } + pub fn dep_map(&self) -> &HashMap, V)> { + &self.dep_map + } + /// Indicate that something has finished. /// /// Calling this function indicates that the `node` has produced `edge`. All diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index bd799d1d1b80..47dbf94508ea 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -2645,6 +2645,112 @@ fn scrape_examples_issue_10545() { .run(); } +#[cargo_test] +fn scrape_examples_no_fail_bad_example() { + if !is_nightly() { + // -Z rustdoc-scrape-examples is unstable + return; + } + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + "#, + ) + .file("examples/ex1.rs", "DOES NOT COMPILE") + .file("examples/ex2.rs", "fn main() { foo::foo(); }") + .file("src/lib.rs", "pub fn foo(){}") + .build(); + + p.cargo( + "doc -v -Zunstable-options -Z rustdoc-scrape-examples=examples -Z ignore-scrape-failures", + ) + .masquerade_as_nightly_cargo() + .with_stderr_unordered( + "\ +[CHECKING] foo v0.0.1 ([CWD]) +[RUNNING] `rustc --crate-name foo[..] +[RUNNING] `rustdoc[..] --crate-name ex1[..] +[RUNNING] `rustdoc[..] --crate-name ex2[..] +error: expected one of `!` or `::`, found `NOT` + --> examples/ex1.rs:1:6 + | +1 | DOES NOT COMPILE + | ^^^ expected one of `!` or `::` + +[DOCUMENTING] foo v0.0.1 ([CWD]) +[RUNNING] `rustdoc[..] --crate-name foo[..] +[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")); +} + +#[cargo_test] +fn scrape_examples_no_fail_bad_dependency() { + if !is_nightly() { + // -Z rustdoc-scrape-examples is unstable + return; + } + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [workspace] + members = ["dep"] + + [dev-dependencies] + dep = {path = "dep"} + "#, + ) + .file("examples/ex.rs", "fn main() { foo::foo(); }") + .file("src/lib.rs", "pub fn foo(){}\npub fn bar() { foo(); }") + .file( + "dep/Cargo.toml", + r#" + [package] + name = "dep" + version = "0.0.1" + authors = [] + "#, + ) + .file("dep/src/lib.rs", "DOES NOT COMPILE") + .build(); + + p.cargo("doc -Zunstable-options -Z rustdoc-scrape-examples=all -Z ignore-scrape-failures") + .masquerade_as_nightly_cargo() + .with_stderr_unordered( + "\ +[CHECKING] dep v0.0.1 ([CWD]/dep) +error: expected one of `!` or `::`, found `NOT` + --> dep/src/lib.rs:1:6 + | +1 | DOES NOT COMPILE + | ^^^ expected one of `!` or `::` + +[CHECKING] foo v0.0.1 ([CWD]) +[DOCUMENTING] 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")); +} + #[cargo_test] fn lib_before_bin() { // Checks that the library is documented before the binary.