Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Check targets needed for optional doc-scraping to fail without killing the build #11450

Merged
merged 2 commits into from
Dec 11, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
willcrichton marked this conversation as resolved.
Show resolved Hide resolved
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();
willcrichton marked this conversation as resolved.
Show resolved Hide resolved
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;
}
weihanglo marked this conversation as resolved.
Show resolved Hide resolved

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(
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
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<'_, '_> {
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
/// 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