Skip to content

Commit

Permalink
Allow Check targets needed for optional doc-scraping to fail without …
Browse files Browse the repository at this point in the history
…killing the build
  • Loading branch information
willcrichton committed Dec 3, 2022
1 parent 7b9069e commit 8e647ba
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 54 deletions.
5 changes: 1 addition & 4 deletions src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -812,10 +812,7 @@ impl<'cfg> DrainState<'cfg> {
debug!("end ({:?}): {:?}", unit, result);
match result {
Ok(()) => self.finish(id, &unit, artifact, cx)?,
Err(_)
if unit.mode.is_doc_scrape()
&& unit.target.doc_scrape_examples().is_unset() =>
{
Err(_) if cx.bcx.unit_can_fail_for_docscraping(&unit) => {
cx.failed_scrape_units
.lock()
.unwrap()
Expand Down
133 changes: 87 additions & 46 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,23 @@ fn compile<'cfg>(
Ok(())
}

/// Generates the warning message used when fallible doc-scrape units fail,
/// either for rustdoc or rustc.
fn make_failed_scrape_diagnostic(cx: &Context<'_, '_>, unit: &Unit, top_line: String) -> String {
let manifest_path = unit.pkg.manifest_path();
let relative_manifest_path = manifest_path
.strip_prefix(cx.bcx.ws.root())
.unwrap_or(&manifest_path)
.to_owned();
format!(
"\
{top_line}
Try running with `--verbose` to see the error message.
If an example or library should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` or `[lib]` definition in {}",
relative_manifest_path.display()
)
}

fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> CargoResult<Work> {
let mut rustc = prepare_rustc(cx, &unit.target.rustc_crate_types(), unit)?;
let build_plan = cx.bcx.build_config.build_plan;
Expand Down Expand Up @@ -265,6 +282,25 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
let is_local = unit.is_local();
let artifact = unit.artifact;

// If this unit is needed for doc-scraping, then we generate a diagnostic that
// describes the set of reverse-dependencies that cause the unit to be needed.
let target_desc = unit.target.description_named();
let mut for_scrape_units = cx
.bcx
.scrape_units_have_dep_on(unit)
.into_iter()
.map(|unit| unit.target.description_named())
.collect::<Vec<_>>();
for_scrape_units.sort();
let for_scrape_units = for_scrape_units.join(", ");
let failed_scrape_diagnostic = make_failed_scrape_diagnostic(cx, unit, format!("failed to check {target_desc} in package `{name}` as a prerequisite for scraping examples from: {for_scrape_units}"));

let hide_diagnostics_for_scrape_unit = cx.bcx.unit_can_fail_for_docscraping(unit)
&& !matches!(cx.bcx.config.shell().verbosity(), Verbosity::Verbose);
if hide_diagnostics_for_scrape_unit {
output_options.show_diagnostics = false;
}

return Ok(Work::new(move |state| {
// Artifacts are in a different location than typical units,
// hence we must assure the crate- and target-dependent
Expand Down Expand Up @@ -339,38 +375,48 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
if build_plan {
state.build_plan(buildkey, rustc.clone(), outputs.clone());
} else {
exec.exec(
&rustc,
package_id,
&target,
mode,
&mut |line| on_stdout_line(state, line, package_id, &target),
&mut |line| {
on_stderr_line(
state,
line,
package_id,
&manifest_path,
&target,
&mut output_options,
)
},
)
.map_err(verbose_if_simple_exit_code)
.with_context(|| {
// adapted from rustc_errors/src/lib.rs
let warnings = match output_options.warnings_seen {
0 => String::new(),
1 => "; 1 warning emitted".to_string(),
count => format!("; {} warnings emitted", count),
};
let errors = match output_options.errors_seen {
0 => String::new(),
1 => " due to previous error".to_string(),
count => format!(" due to {} previous errors", count),
};
format!("could not compile `{}`{}{}", name, errors, warnings)
})?;
let result = exec
.exec(
&rustc,
package_id,
&target,
mode,
&mut |line| on_stdout_line(state, line, package_id, &target),
&mut |line| {
on_stderr_line(
state,
line,
package_id,
&manifest_path,
&target,
&mut output_options,
)
},
)
.map_err(verbose_if_simple_exit_code)
.with_context(|| {
// adapted from rustc_errors/src/lib.rs
let warnings = match output_options.warnings_seen {
0 => String::new(),
1 => "; 1 warning emitted".to_string(),
count => format!("; {} warnings emitted", count),
};
let errors = match output_options.errors_seen {
0 => String::new(),
1 => " due to previous error".to_string(),
count => format!(" due to {} previous errors", count),
};
format!("could not compile `{}`{}{}", name, errors, warnings)
});

if let Err(e) = result {
if hide_diagnostics_for_scrape_unit {
state.warning(failed_scrape_diagnostic)?;
}

return Err(e);
}

// Exec should never return with success *and* generate an error.
debug_assert_eq!(output_options.errors_seen, 0);
}
Expand Down Expand Up @@ -713,20 +759,22 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
let build_script_outputs = Arc::clone(&cx.build_script_outputs);
let package_id = unit.pkg.package_id();
let manifest_path = PathBuf::from(unit.pkg.manifest_path());
let relative_manifest_path = manifest_path
.strip_prefix(cx.bcx.ws.root())
.unwrap_or(&manifest_path)
.to_owned();
let target = Target::clone(&unit.target);
let mut output_options = OutputOptions::new(cx, unit);
let script_metadata = cx.find_build_script_metadata(unit);

let failed_scrape_units = Arc::clone(&cx.failed_scrape_units);
let hide_diagnostics_for_scrape_unit = unit.mode.is_doc_scrape()
&& unit.target.doc_scrape_examples().is_unset()
let hide_diagnostics_for_scrape_unit = cx.bcx.unit_can_fail_for_docscraping(unit)
&& !matches!(cx.bcx.config.shell().verbosity(), Verbosity::Verbose);
let failed_scrape_diagnostic = make_failed_scrape_diagnostic(
cx,
unit,
format!("failed to scan {target_desc} in package `{name}` for example code usage"),
);
if hide_diagnostics_for_scrape_unit {
output_options.show_diagnostics = false;
}

Ok(Work::new(move |state| {
add_custom_flags(
&mut rustdoc,
Expand Down Expand Up @@ -775,14 +823,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {

if let Err(e) = result {
if hide_diagnostics_for_scrape_unit {
let diag = format!(
"\
failed to scan {target_desc} in package `{name}` for example code usage
Try running with `--verbose` to see the error message.
If this example should not be scanned, consider adding `doc-scrape-examples = false` to the `[[example]]` definition in {}",
relative_manifest_path.display()
);
state.warning(diag)?;
state.warning(failed_scrape_diagnostic)?;
}

return Err(e);
Expand Down
39 changes: 38 additions & 1 deletion src/cargo/core/compiler/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use crate::core::compiler::context::Context;
use crate::core::compiler::unit::Unit;
use crate::core::compiler::CompileKind;
use crate::core::compiler::{BuildContext, CompileKind};
use crate::sources::CRATES_IO_REGISTRY;
use crate::util::errors::{internal, CargoResult};
use cargo_util::ProcessBuilder;
Expand Down Expand Up @@ -209,3 +209,40 @@ impl RustdocScrapeExamples {
matches!(self, RustdocScrapeExamples::Unset)
}
}

impl BuildContext<'_, '_> {
/// Returns the set of Docscrape units that have a direct dependency on `unit`
pub fn scrape_units_have_dep_on<'a>(&'a self, unit: &'a Unit) -> Vec<&'a Unit> {
self.scrape_units
.iter()
.filter(|scrape_unit| {
self.unit_graph[scrape_unit]
.iter()
.any(|dep| &dep.unit == unit)
})
.collect::<Vec<_>>()
}

/// Returns true if this unit is needed for doing doc-scraping and is also
/// allowed to fail without killing the build.
pub fn unit_can_fail_for_docscraping(&self, unit: &Unit) -> bool {
// If the unit is not a Docscrape unit, e.g. a Lib target that is
// checked to scrape an Example target, then we need to get the doc-scrape-examples
// configuration for the reverse-dependent Example target.
let for_scrape_units = if unit.mode.is_doc_scrape() {
vec![unit]
} else {
self.scrape_units_have_dep_on(unit)
};

if for_scrape_units.is_empty() {
false
} else {
// All Docscrape units must have doc-scrape-examples unset. If any are true,
// then the unit is not allowed to fail.
for_scrape_units
.iter()
.all(|unit| unit.target.doc_scrape_examples().is_unset())
}
}
}
21 changes: 18 additions & 3 deletions tests/testsuite/docscrape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,17 +332,32 @@ fn no_fail_bad_lib() {
"#,
)
.file("src/lib.rs", "pub fn foo() { CRASH_THE_BUILD() }")
.file("examples/ex.rs", "fn main() { foo::foo(); }")
.file("examples/ex2.rs", "fn main() { foo::foo(); }")
.build();

p.cargo("doc -Zunstable-options -Z rustdoc-scrape-examples")
.masquerade_as_nightly_cargo(&["rustdoc-scrape-examples"])
.with_stderr(
.with_stderr_unordered(
"\
[CHECKING] foo v0.0.1 ([CWD])
[SCRAPING] foo v0.0.1 ([CWD])
warning: failed to scan lib in package `foo` for example code usage
Try running with `--verbose` to see the error message.
If this example should not be scanned, consider adding `doc-scrape-examples = false` to the `[[example]]` definition in Cargo.toml
If an example or library should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` or `[lib]` definition in Cargo.toml
warning: `foo` (lib) generated 1 warning
warning: failed to check lib in package `foo` as a prerequisite for scraping examples from: example \"ex\", example \"ex2\"
Try running with `--verbose` to see the error message.
If an example or library should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` or `[lib]` definition in Cargo.toml
warning: `foo` (lib) generated 1 warning
warning: failed to scan example \"ex\" in package `foo` for example code usage
Try running with `--verbose` to see the error message.
If an example or library should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` or `[lib]` definition in Cargo.toml
warning: `foo` (example \"ex\") generated 1 warning
warning: failed to scan example \"ex2\" in package `foo` for example code usage
Try running with `--verbose` to see the error message.
If an example or library should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` or `[lib]` definition in Cargo.toml
warning: `foo` (example \"ex2\") generated 1 warning
[DOCUMENTING] foo v0.0.1 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]",
)
Expand Down Expand Up @@ -374,7 +389,7 @@ fn no_fail_bad_example() {
[SCRAPING] foo v0.0.1 ([CWD])
warning: failed to scan example \"ex1\" in package `foo` for example code usage
Try running with `--verbose` to see the error message.
If this example should not be scanned, consider adding `doc-scrape-examples = false` to the `[[example]]` definition in Cargo.toml
If an example or library should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` or `[lib]` definition in Cargo.toml
warning: `foo` (example \"ex1\") generated 1 warning
[DOCUMENTING] foo v0.0.1 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]",
Expand Down

0 comments on commit 8e647ba

Please sign in to comment.