Auto merge of #148332 - jyn514:cargo-deny-warnings, r=<try>

bootstrap: Use cargo's `build.warnings=deny` rather than -Dwarnings

try-job: test-various

Author: rust-bors[bot]

src/bootstrap/bootstrap.py

@@ -1113,6 +1113,19 @@ def build_bootstrap_cmd(self, env):
         else:
             env["RUSTFLAGS"] = "-Zallow-features="
 
+        if not os.path.isfile(self.cargo()):
+            raise Exception("no cargo executable found at `{}`".format(self.cargo()))
+        args = [
+            self.cargo(),
+            "build",
+            "--jobs=" + self.jobs,
+            "--manifest-path",
+            os.path.join(self.rust_root, "src/bootstrap/Cargo.toml"),
+            "-Zroot-dir=" + self.rust_root,
+        ]
+        # verbose cargo output is very noisy, so only enable it with -vv
+        args.extend("--verbose" for _ in range(self.verbose - 1))
+
         target_features = []
         if self.get_toml("crt-static", build_section) == "true":
             target_features += ["+crt-static"]
@@ -1131,28 +1144,15 @@ def build_bootstrap_cmd(self, env):
         else:
             deny_warnings = self.warnings == "deny"
         if deny_warnings:
-            env["RUSTFLAGS"] += " -Dwarnings"
+            args += ["-Zwarnings"]
+            env["CARGO_BUILD_WARNINGS"] = "deny"
 
         # Add RUSTFLAGS_BOOTSTRAP to RUSTFLAGS for bootstrap compilation.
         # Note that RUSTFLAGS_BOOTSTRAP should always be added to the end of
-        # RUSTFLAGS to be actually effective (e.g., if we have `-Dwarnings` in
-        # RUSTFLAGS, passing `-Awarnings` from RUSTFLAGS_BOOTSTRAP should override it).
+        # RUSTFLAGS, since that causes RUSTFLAGS_BOOTSTRAP to override RUSTFLAGS.
         if "RUSTFLAGS_BOOTSTRAP" in env:
             env["RUSTFLAGS"] += " " + env["RUSTFLAGS_BOOTSTRAP"]
 
-        if not os.path.isfile(self.cargo()):
-            raise Exception("no cargo executable found at `{}`".format(self.cargo()))
-        args = [
-            self.cargo(),
-            "build",
-            "--jobs=" + self.jobs,
-            "--manifest-path",
-            os.path.join(self.rust_root, "src/bootstrap/Cargo.toml"),
-            "-Zroot-dir=" + self.rust_root,
-        ]
-        # verbose cargo output is very noisy, so only enable it with -vv
-        args.extend("--verbose" for _ in range(self.verbose - 1))
-
         if "BOOTSTRAP_TRACING" in env:
             args.append("--features=tracing")
 

src/bootstrap/bootstrap_test.py

@@ -244,8 +244,10 @@ def test_warnings(self):
             if toml_warnings is not None:
                 configure_args = ["--set", "rust.deny-warnings=" + toml_warnings]
 
-            _, env = self.build_args(configure_args, args=["--warnings=warn"])
-            self.assertFalse("-Dwarnings" in env["RUSTFLAGS"])
+            args, env = self.build_args(configure_args, args=["--warnings=warn"])
+            self.assertFalse("CARGO_BUILD_WARNINGS" in env)
+            self.assertFalse("-Zwarnings" in args)
 
-            _, env = self.build_args(configure_args, args=["--warnings=deny"])
-            self.assertTrue("-Dwarnings" in env["RUSTFLAGS"])
+            args, env = self.build_args(configure_args, args=["--warnings=deny"])
+            self.assertEqual("deny", env["CARGO_BUILD_WARNINGS"])
+            self.assertTrue("-Zwarnings" in args)

src/bootstrap/src/core/builder/cargo.rs

@@ -459,6 +459,11 @@ impl Builder<'_> {
         let out_dir = self.stage_out(compiler, mode);
         cargo.env("CARGO_TARGET_DIR", &out_dir);
 
+        // Set this unconditionally. Cargo silently ignores `CARGO_BUILD_WARNINGS` when `-Z
+        // warnings` isn't present, which is hard to debug, and it's not worth the effort to keep
+        // them in sync.
+        cargo.arg("-Zwarnings");
+
         // Bootstrap makes a lot of assumptions about the artifacts produced in the target
         // directory. If users override the "build directory" using `build-dir`
         // (https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#build-dir), then
@@ -1159,8 +1164,10 @@ impl Builder<'_> {
             lint_flags.push("-Wunused_lifetimes");
 
             if self.config.deny_warnings {
-                lint_flags.push("-Dwarnings");
-                rustdocflags.arg("-Dwarnings");
+                // We use this instead of `lint_flags` so that we don't have to rebuild all
+                // workspace dependencies when `deny-warnings` changes, but we still get an error
+                // immediately instead of having to wait until the next rebuild.
+                cargo.env("CARGO_BUILD_WARNINGS", "deny");
             }
 
             rustdocflags.arg("-Wrustdoc::invalid_codeblock_attributes");