Skip to content

Commit

Permalink
feat(warnings) add build.warnings option
Browse files Browse the repository at this point in the history
  • Loading branch information
arlosi committed Sep 12, 2024
1 parent 643a025 commit c3bad93
Show file tree
Hide file tree
Showing 12 changed files with 346 additions and 53 deletions.
4 changes: 4 additions & 0 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ pub struct Compilation<'gctx> {
target_runners: HashMap<CompileKind, Option<(PathBuf, Vec<String>)>>,
/// The linker to use for each host or target.
target_linkers: HashMap<CompileKind, Option<PathBuf>>,

/// The total number of warnings emitted by the compilation.
pub warning_count: usize,
}

impl<'gctx> Compilation<'gctx> {
Expand Down Expand Up @@ -169,6 +172,7 @@ impl<'gctx> Compilation<'gctx> {
.chain(Some(&CompileKind::Host))
.map(|kind| Ok((*kind, target_linker(bcx, *kind)?)))
.collect::<CargoResult<HashMap<_, _>>>()?,
warning_count: 0,
})
}

Expand Down
28 changes: 18 additions & 10 deletions src/cargo/core/compiler/job_queue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ use crate::core::compiler::future_incompat::{
};
use crate::core::resolver::ResolveBehavior;
use crate::core::{PackageId, Shell, TargetKind};
use crate::util::context::WarningHandling;
use crate::util::diagnostic_server::{self, DiagnosticPrinter};
use crate::util::errors::AlreadyPrintedError;
use crate::util::machine_message::{self, Message as _};
Expand Down Expand Up @@ -601,6 +602,7 @@ impl<'gctx> DrainState<'gctx> {
plan: &mut BuildPlan,
event: Message,
) -> Result<(), ErrorToHandle> {
let warning_handling = build_runner.bcx.gctx.warning_handling()?;
match event {
Message::Run(id, cmd) => {
build_runner
Expand Down Expand Up @@ -638,7 +640,9 @@ impl<'gctx> DrainState<'gctx> {
}
}
Message::Warning { id, warning } => {
build_runner.bcx.gctx.shell().warn(warning)?;
if warning_handling != WarningHandling::Allow {
build_runner.bcx.gctx.shell().warn(warning)?;
}
self.bump_warning_count(id, true, false);
}
Message::WarningCount {
Expand Down Expand Up @@ -826,6 +830,9 @@ impl<'gctx> DrainState<'gctx> {
// `display_error` inside `handle_error`.
Some(anyhow::Error::new(AlreadyPrintedError::new(error)))
} else if self.queue.is_empty() && self.pending_queue.is_empty() {
build_runner.compilation.warning_count +=
self.warning_count.values().map(|c| c.total).sum::<usize>();

let profile_link = build_runner.bcx.gctx.shell().err_hyperlink(
"https://doc.rust-lang.org/cargo/reference/profiles.html#default-profiles",
);
Expand Down Expand Up @@ -963,32 +970,32 @@ impl<'gctx> DrainState<'gctx> {
}

fn emit_warnings(
&mut self,
&self,
msg: Option<&str>,
unit: &Unit,
build_runner: &mut BuildRunner<'_, '_>,
build_runner: &BuildRunner<'_, '_>,
) -> CargoResult<()> {
let outputs = build_runner.build_script_outputs.lock().unwrap();
let Some(metadata) = build_runner.find_build_script_metadata(unit) else {
return Ok(());
};
let bcx = &mut build_runner.bcx;
let gctx = build_runner.bcx.gctx;
if let Some(output) = outputs.get(metadata) {
if !output.warnings.is_empty() {
if let Some(msg) = msg {
writeln!(bcx.gctx.shell().err(), "{}\n", msg)?;
writeln!(gctx.shell().err(), "{}\n", msg)?;
}

for warning in output.warnings.iter() {
let warning_with_package =
format!("{}@{}: {}", unit.pkg.name(), unit.pkg.version(), warning);

bcx.gctx.shell().warn(warning_with_package)?;
gctx.shell().warn(warning_with_package)?;
}

if msg.is_some() {
// Output an empty line.
writeln!(bcx.gctx.shell().err())?;
writeln!(gctx.shell().err())?;
}
}
}
Expand Down Expand Up @@ -1022,13 +1029,13 @@ impl<'gctx> DrainState<'gctx> {
gctx: &GlobalContext,
id: JobId,
rustc_workspace_wrapper: &Option<PathBuf>,
) {
let count = match self.warning_count.remove(&id) {
) -> usize {
let count = match self.warning_count.get(&id) {
// An error could add an entry for a `Unit`
// with 0 warnings but having fixable
// warnings be disallowed
Some(count) if count.total > 0 => count,
None | Some(_) => return,
None | Some(_) => return 0,
};
let unit = &self.active[&id];
let mut message = descriptive_pkg_name(&unit.pkg.name(), &unit.target, &unit.mode);
Expand Down Expand Up @@ -1089,6 +1096,7 @@ impl<'gctx> DrainState<'gctx> {
// Errors are ignored here because it is tricky to handle them
// correctly, and they aren't important.
let _ = gctx.shell().warn(message);
count.total
}

fn finish(
Expand Down
10 changes: 8 additions & 2 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ pub use crate::core::compiler::unit::{Unit, UnitInterner};
use crate::core::manifest::TargetSourcePath;
use crate::core::profiles::{PanicStrategy, Profile, StripInner};
use crate::core::{Feature, PackageId, Target, Verbosity};
use crate::util::context::WarningHandling;
use crate::util::errors::{CargoResult, VerboseError};
use crate::util::interning::InternedString;
use crate::util::machine_message::{self, Message};
Expand Down Expand Up @@ -201,13 +202,16 @@ fn compile<'gctx>(
} else {
// We always replay the output cache,
// since it might contain future-incompat-report messages
let show_diagnostics = unit.show_warnings(bcx.gctx)
&& build_runner.bcx.gctx.warning_handling()?
!= WarningHandling::Allow;
let work = replay_output_cache(
unit.pkg.package_id(),
PathBuf::from(unit.pkg.manifest_path()),
&unit.target,
build_runner.files().message_cache_path(unit),
build_runner.bcx.build_config.message_format,
unit.show_warnings(bcx.gctx),
show_diagnostics,
);
// Need to link targets on both the dirty and fresh.
work.then(link_targets(build_runner, unit, true)?)
Expand Down Expand Up @@ -1665,10 +1669,12 @@ impl OutputOptions {
// Remove old cache, ignore ENOENT, which is the common case.
drop(fs::remove_file(&path));
let cache_cell = Some((path, LazyCell::new()));
let show_diagnostics =
build_runner.bcx.gctx.warning_handling().unwrap_or_default() != WarningHandling::Allow;
OutputOptions {
format: build_runner.bcx.build_config.message_format,
cache_cell,
show_diagnostics: true,
show_diagnostics,
warnings_seen: 0,
errors_seen: 0,
}
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,7 @@ unstable_cli_options!(
target_applies_to_host: bool = ("Enable the `target-applies-to-host` key in the .cargo/config.toml file"),
trim_paths: bool = ("Enable the `trim-paths` option in profiles"),
unstable_options: bool = ("Allow the usage of unstable options"),
warnings: bool = ("Allow use of the build.warnings config key"),
);

const STABILIZED_COMPILE_PROGRESS: &str = "The progress bar is now always \
Expand Down Expand Up @@ -1293,6 +1294,7 @@ impl CliUnstable {
"script" => self.script = parse_empty(k, v)?,
"target-applies-to-host" => self.target_applies_to_host = parse_empty(k, v)?,
"unstable-options" => self.unstable_options = parse_empty(k, v)?,
"warnings" => self.warnings = parse_empty(k, v)?,
_ => bail!("\
unknown `-Z` flag specified: {k}\n\n\
For available unstable features, see https://doc.rust-lang.org/nightly/cargo/reference/unstable.html\n\
Expand Down
24 changes: 13 additions & 11 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1169,12 +1169,13 @@ impl<'gctx> Workspace<'gctx> {
}
}

pub fn emit_warnings(&self) -> CargoResult<()> {
pub fn emit_warnings(&self) -> CargoResult<usize> {
let mut warning_count = 0;
for (path, maybe_pkg) in &self.packages.packages {
let path = path.join("Cargo.toml");
if let MaybePackage::Package(pkg) = maybe_pkg {
if self.gctx.cli_unstable().cargo_lints {
self.emit_lints(pkg, &path)?
warning_count += self.emit_lints(pkg, &path)?;
}
}
let warnings = match maybe_pkg {
Expand All @@ -1199,11 +1200,11 @@ impl<'gctx> Workspace<'gctx> {
}
}
}
Ok(())
Ok(warning_count)
}

pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
let mut error_count = 0;
pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<usize> {
let mut counts = Default::default();
let toml_lints = pkg
.manifest()
.normalized_toml()
Expand Down Expand Up @@ -1235,16 +1236,17 @@ impl<'gctx> Workspace<'gctx> {
self.root_manifest(),
self.gctx,
)?;
check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
check_implicit_features(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
unused_dependencies(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
if error_count > 0 {
check_im_a_teapot(pkg, &path, &cargo_lints, &mut counts, self.gctx)?;
check_implicit_features(pkg, &path, &cargo_lints, &mut counts, self.gctx)?;
unused_dependencies(pkg, &path, &cargo_lints, &mut counts, self.gctx)?;
if counts.error_count > 0 {
Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
"encountered {error_count} errors(s) while running lints"
"encountered {error_count} errors(s) while running lints",
error_count = counts.error_count
))
.into())
} else {
Ok(())
Ok(counts.warning_count)
}
}

Expand Down
14 changes: 11 additions & 3 deletions src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ use crate::core::{PackageId, PackageSet, SourceId, TargetKind, Workspace};
use crate::drop_println;
use crate::ops;
use crate::ops::resolve::WorkspaceResolve;
use crate::util::context::GlobalContext;
use crate::util::context::{GlobalContext, WarningHandling};
use crate::util::interning::InternedString;
use crate::util::{CargoResult, StableHasher};

Expand Down Expand Up @@ -137,8 +137,16 @@ pub fn compile_with_exec<'a>(
options: &CompileOptions,
exec: &Arc<dyn Executor>,
) -> CargoResult<Compilation<'a>> {
ws.emit_warnings()?;
compile_ws(ws, options, exec)
let cargo_warning_count = ws.emit_warnings()?;
let compilation = compile_ws(ws, options, exec)?;
let total_warnings = cargo_warning_count + compilation.warning_count;

if ws.gctx().warning_handling()? == WarningHandling::Deny && total_warnings > 0 {
anyhow::bail!(
"warnings are denied by `build.warnings` configuration"
)
}
Ok(compilation)
}

/// Like [`compile_with_exec`] but without warnings from manifest parsing.
Expand Down
23 changes: 23 additions & 0 deletions src/cargo/util/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2004,6 +2004,15 @@ impl GlobalContext {
})?;
Ok(deferred.borrow_mut())
}

/// Get the global [`WarningHandling`] configuration.
pub fn warning_handling(&self) -> CargoResult<WarningHandling> {
if self.unstable_flags.warnings {
Ok(self.build_config()?.warnings.unwrap_or_default())
} else {
Ok(Default::default())
}
}
}

/// Internal error for serde errors.
Expand Down Expand Up @@ -2615,6 +2624,20 @@ pub struct CargoBuildConfig {
// deprecated alias for artifact-dir
pub out_dir: Option<ConfigRelativePath>,
pub artifact_dir: Option<ConfigRelativePath>,
pub warnings: Option<WarningHandling>,
}

/// Whether warnings should warn, be allowed, or cause an error.
#[derive(Debug, Copy, Clone, PartialEq, Eq, Deserialize, Default)]
#[serde(rename_all = "kebab-case")]
pub enum WarningHandling {
#[default]
/// Output warnings.
Warn,
/// Allow warnings (do not output them).
Allow,
/// Error if warnings are emitted.
Deny,
}

/// Configuration for `build.target`.
Expand Down
Loading

0 comments on commit c3bad93

Please sign in to comment.