Skip to content

Commit

Permalink
feat: add build.warnings config option to control rustc warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
arlosi committed Sep 13, 2024
1 parent 36cbb68 commit b7b9f87
Show file tree
Hide file tree
Showing 9 changed files with 128 additions and 25 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
11 changes: 9 additions & 2 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 @@ -1023,7 +1030,7 @@ impl<'gctx> DrainState<'gctx> {
id: JobId,
rustc_workspace_wrapper: &Option<PathBuf>,
) {
let count = match self.warning_count.remove(&id) {
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
Expand Down
9 changes: 7 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,15 @@ 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 +1668,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
8 changes: 6 additions & 2 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 @@ -138,7 +138,11 @@ pub fn compile_with_exec<'a>(
exec: &Arc<dyn Executor>,
) -> CargoResult<Compilation<'a>> {
ws.emit_warnings()?;
compile_ws(ws, options, exec)
let compilation = compile_ws(ws, options, exec)?;
if ws.gctx().warning_handling()? == WarningHandling::Deny && compilation.warning_count > 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
20 changes: 20 additions & 0 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ Each new feature described below should explain how to use it.
* [lockfile-path](#lockfile-path) --- Allows to specify a path to lockfile other than the default path `<workspace_root>/Cargo.lock`.
* [package-workspace](#package-workspace) --- Allows for packaging and publishing multiple crates in a workspace.
* [native-completions](#native-completions) --- Move cargo shell completions to native completions.
* [warnings](#warnings) --- controls warning behavior; options for allowing or denying warnings.

## allow-features

Expand Down Expand Up @@ -1957,3 +1958,22 @@ default behavior.

See the [build script documentation](build-scripts.md#rustc-check-cfg) for information
about specifying custom cfgs.

## warnings

The `-Z warnings` feature enables the `build.warnings` configuration option to control how
Cargo handles warnings. If the `-Z warnings` unstable flag is not enabled, then
the `build.warnings` config will be ignored.

### `build.warnings`
* Type: string
* Default: `warn`
* Environment: `CARGO_BUILD_WARNINGS`

Controls how Cargo handles warnings. Allowed values are:
* `warn`: warnings are emitted as warnings (default).
* `allow`: warnings are hidden.
* `deny`: if warnings are emitted, an error will be raised at the end of the operation and the process will exit with a failure exit code.

This setting currently only applies to rustc warnings. It may apply to additional warnings (such as Cargo lints or Cargo warnings)
in the future.
14 changes: 8 additions & 6 deletions tests/testsuite/cargo/z_help/stdout.term.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
62 changes: 49 additions & 13 deletions tests/testsuite/warning_override.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,13 @@ use snapbox::data::Inline;
const ALLOW_CLEAN: LazyLock<Inline> = LazyLock::new(|| {
str![[r#"
[CHECKING] foo v0.0.1 ([ROOT]/foo)
[WARNING] unused variable: `x`
...
[WARNING] `foo` (bin "foo") generated 1 warning
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
"#]]
});

const ALLOW_CACHED: LazyLock<Inline> = LazyLock::new(|| {
str![[r#"
[WARNING] unused variable: `x`
...
[WARNING] `foo` (bin "foo") generated 1 warning
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
"#]]
Expand All @@ -44,6 +38,7 @@ const DENY: LazyLock<Inline> = LazyLock::new(|| {
...
[WARNING] `foo` (bin "foo") generated 1 warning
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[ERROR] warnings are denied by `build.warnings` configuration
"#]]
});
Expand All @@ -68,33 +63,74 @@ fn make_project(main_src: &str) -> Project {
#[cargo_test]
fn rustc_caching_allow_first() {
let p = make_project("let x = 3;");
p.cargo("check").with_stderr_data(ALLOW_CLEAN.clone()).run();
p.cargo("check")
.masquerade_as_nightly_cargo(&["warnings"])
.arg("-Zwarnings")
.arg("--config")
.arg("build.warnings='allow'")
.with_stderr_data(ALLOW_CLEAN.clone())
.run();

p.cargo("check").with_stderr_data(DENY.clone()).run();
p.cargo("check")
.masquerade_as_nightly_cargo(&["warnings"])
.arg("-Zwarnings")
.arg("--config")
.arg("build.warnings='deny'")
.with_stderr_data(DENY.clone())
.with_status(101)
.run();
}

#[cargo_test]
fn rustc_caching_deny_first() {
let p = make_project("let x = 3;");
p.cargo("check").with_stderr_data(DENY.clone()).run();
p.cargo("check")
.masquerade_as_nightly_cargo(&["warnings"])
.arg("-Zwarnings")
.arg("--config")
.arg("build.warnings='deny'")
.with_stderr_data(DENY.clone())
.with_status(101)
.run();

p.cargo("check")
.masquerade_as_nightly_cargo(&["warnings"])
.arg("-Zwarnings")
.arg("--config")
.arg("build.warnings='allow'")
.with_stderr_data(ALLOW_CACHED.clone())
.run();
}

#[cargo_test]
fn config() {
let p = make_project("let x = 3;");
p.cargo("check").with_stderr_data(DENY.clone()).run();
p.cargo("check")
.masquerade_as_nightly_cargo(&["warnings"])
.arg("-Zwarnings")
.env("CARGO_BUILD_WARNINGS", "deny")
.with_stderr_data(DENY.clone())
.with_status(101)
.run();

// CLI has precedence over env.
p.cargo("check").with_stderr_data(WARN.clone()).run();
// CLI has precedence over env
p.cargo("check")
.masquerade_as_nightly_cargo(&["warnings"])
.arg("-Zwarnings")
.arg("--config")
.arg("build.warnings='warn'")
.env("CARGO_BUILD_WARNINGS", "deny")
.with_stderr_data(WARN.clone())
.run();
}

#[cargo_test]
fn requires_nightly() {
// build.warnings has no effect without -Zwarnings.
let p = make_project("let x = 3;");
p.cargo("check").with_stderr_data(WARN.clone()).run();
p.cargo("check")
.arg("--config")
.arg("build.warnings='deny'")
.with_stderr_data(WARN.clone())
.run();
}

0 comments on commit b7b9f87

Please sign in to comment.