Support rustc's -Z panic-abort-tests in Cargo

Recently added in rust-lang/rust#64158 the `-Z panic-abort-tests` flag
to the compiler itself will activate a mode in the `test` crate which
enables running tests even if they're compiled with `panic=abort`.  It
effectively runs a test-per-process.

This commit brings the same support to Cargo, adding a `-Z
panic-abort-tests` flag to Cargo which allows building tests in
`panic=abort` mode. While I wanted to be sure to add support for this in
Cargo before we stabilize the flag in `rustc`, I don't actually know how
we're going to stabilize this here. Today Cargo will automatically
switch test targets to `panic=unwind`, and so if we actually were to
stabilize this flag then this configuration would break:

    [profile.dev]
    panic = 'abort'

In that case tests would be compiled with `panic=unwind` (due to how
profiles work today) which would clash with crates also being compiled
with `panic=abort`. I'm hopeful though that we can perhaps either figure
out a solution for this and maybe even integrate it with the ongoing
profiles work.

Author: alexcrichton

src/cargo/core/compiler/mod.rs

@@ -799,6 +799,17 @@ fn build_base_args<'a, 'cfg>(
 
     if test && unit.target.harness() {
         cmd.arg("--test");
+
+        // Cargo has historically never compiled `--test` binaries with
+        // `panic=abort` because the `test` crate itself didn't support it.
+        // Support is now upstream, however, but requires an unstable flag to be
+        // passed when compiling the test. We require, in Cargo, an unstable
+        // flag to pass to rustc, so register that here. Eventually this flag
+        // will simply not be needed when the behavior is stabilized in the Rust
+        // compiler itself.
+        if *panic == PanicStrategy::Abort {
+            cmd.arg("-Zpanic-abort-tests");
+        }
     } else if test {
         cmd.arg("--cfg").arg("test");
     }

src/cargo/core/compiler/unit_dependencies.rs

@@ -161,7 +161,7 @@ fn deps_of_roots<'a, 'cfg>(roots: &[Unit<'a>], mut state: &mut State<'a, 'cfg>)
             // without, once for `--test`). In particular, the lib included for
             // Doc tests and examples are `Build` mode here.
             let unit_for = if unit.mode.is_any_test() || state.bcx.build_config.test() {
-                UnitFor::new_test()
+                UnitFor::new_test(state.bcx.config)
             } else if unit.target.is_custom_build() {
                 // This normally doesn't happen, except `clean` aggressively
                 // generates all units.

src/cargo/core/features.rs

@@ -340,6 +340,7 @@ pub struct CliUnstable {
     pub build_std: Option<Vec<String>>,
     pub timings: Option<Vec<String>>,
     pub doctest_xcompile: bool,
+    pub panic_abort_tests: bool,
 }
 
 impl CliUnstable {
@@ -399,6 +400,7 @@ impl CliUnstable {
             }
             "timings" => self.timings = Some(parse_timings(v)),
             "doctest-xcompile" => self.doctest_xcompile = true,
+            "panic-abort-tests" => self.panic_abort_tests = true,
             _ => failure::bail!("unknown `-Z` flag specified: {}", k),
         }
 

src/cargo/core/profiles.rs

@@ -357,9 +357,10 @@ impl Profiles {
             Some(r) => r,
         };
         let mut profile = maker.get_profile(Some(pkg_id), is_member, unit_for);
-        // `panic` should not be set for tests/benches, or any of their
-        // dependencies.
-        if !unit_for.is_panic_abort_ok() || mode.is_any_test() {
+        // `panic` may not be set for tests/benches, or any of their
+        // dependencies, so handle that here if that's what `UnitFor` tells us
+        // to do.
+        if !unit_for.is_panic_abort_ok() {
             profile.panic = PanicStrategy::Unwind;
         }
 
@@ -901,6 +902,8 @@ impl UnitFor {
     pub fn new_build() -> UnitFor {
         UnitFor {
             build: true,
+            // Force build scripts to always use `panic=unwind` for now to
+            // maximally share dependencies with procedural macros.
             panic_abort_ok: false,
         }
     }
@@ -909,22 +912,33 @@ impl UnitFor {
     pub fn new_compiler() -> UnitFor {
         UnitFor {
             build: false,
+            // Force plugins to use `panic=abort` so panics in the compiler do
+            // not abort the process but instead end with a reasonable error
+            // message that involves catching the panic in the compiler.
             panic_abort_ok: false,
         }
     }
 
     /// A unit for a test/bench target or their dependencies.
-    pub fn new_test() -> UnitFor {
+    ///
+    /// Note that `config` is taken here for unstable CLI features to detect
+    /// whether `panic=abort` is supported for tests. Historical versions of
+    /// rustc did not support this, but newer versions do with an unstable
+    /// compiler flag.
+    pub fn new_test(config: &Config) -> UnitFor {
         UnitFor {
             build: false,
-            panic_abort_ok: false,
+            panic_abort_ok: config.cli_unstable().panic_abort_tests,
         }
     }
 
     /// Creates a variant based on `for_host` setting.
     ///
-    /// When `for_host` is true, this clears `panic_abort_ok` in a sticky fashion so
-    /// that all its dependencies also have `panic_abort_ok=false`.
+    /// When `for_host` is true, this clears `panic_abort_ok` in a sticky
+    /// fashion so that all its dependencies also have `panic_abort_ok=false`.
+    /// This'll help ensure that once we start compiling for the host platform
+    /// (build scripts, plugins, proc macros, etc) we'll share the same build
+    /// graph where everything is `panic=unwind`.
     pub fn with_for_host(self, for_host: bool) -> UnitFor {
         UnitFor {
             build: self.build || for_host,
@@ -938,7 +952,8 @@ impl UnitFor {
         self.build
     }
 
-    /// Returns `true` if this unit is allowed to set the `panic` compiler flag.
+    /// Returns `true` if this unit is allowed to set the `panic=abort`
+    /// compiler flag.
     pub fn is_panic_abort_ok(self) -> bool {
         self.panic_abort_ok
     }

src/cargo/ops/cargo_compile.rs

@@ -642,7 +642,7 @@ fn generate_targets<'a>(
 ) -> CargoResult<Vec<Unit<'a>>> {
     // Helper for creating a `Unit` struct.
     let new_unit = |pkg: &'a Package, target: &'a Target, target_mode: CompileMode| {
-        let unit_for = if bcx.build_config.mode.is_any_test() {
+        let unit_for = if target_mode.is_any_test() {
             // NOTE: the `UnitFor` here is subtle. If you have a profile
             // with `panic` set, the `panic` flag is cleared for
             // tests/benchmarks and their dependencies. If this
@@ -661,7 +661,7 @@ fn generate_targets<'a>(
             //
             // Forcing the lib to be compiled three times during `cargo
             // test` is probably also not desirable.
-            UnitFor::new_test()
+            UnitFor::new_test(bcx.config)
         } else if target.for_host() {
             // Proc macro / plugin should not have `panic` set.
             UnitFor::new_compiler()

src/doc/src/reference/unstable.md

@@ -475,3 +475,17 @@ that information for change-detection (if any binary dependency changes, then
 the crate will be rebuilt). The primary use case is for building the compiler
 itself, which has implicit dependencies on the standard library that would
 otherwise be untracked for change-detection.
+
+### panic-abort-tests
+
+The `-Z panic-abort-tests` flag will enable nightly support to compile test
+harness crates with `-Cpanic=abort`. Without this flag Cargo will compile tests,
+and everything they depend on, with `-Cpanic=unwind` because it's the only way
+`test`-the-crate knows how to operate. As of [rust-lang/rust#64158], however,
+the `test` crate supports `-C panic=abort` with a test-per-process, and can help
+avoid compiling crate graphs multiple times.
+
+It's currently unclear how this feature will be stabilized in Cargo, but we'd
+like to stabilize it somehow!
+
+[rust-lang/rust#64158]: https://github.com/rust-lang/rust/pull/64158

tests/testsuite/test.rs

@@ -3872,3 +3872,132 @@ fn cargo_test_doctest_xcompile_no_runner() {
     )
     .run();
 }
+
+#[cargo_test]
+fn panic_abort_tests() {
+    if !is_nightly() {
+        // -Zpanic-abort-tests in rustc is unstable
+        return;
+    }
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = 'foo'
+                version = '0.1.0'
+
+                [dependencies]
+                a = { path = 'a' }
+
+                [profile.dev]
+                panic = 'abort'
+                [profile.test]
+                panic = 'abort'
+            "#,
+        )
+        .file(
+            "src/lib.rs",
+            r#"
+                #[test]
+                fn foo() {
+                    a::foo();
+                }
+            "#,
+        )
+        .file("a/Cargo.toml", &basic_lib_manifest("a"))
+        .file("a/src/lib.rs", "pub fn foo() {}")
+        .build();
+
+    p.cargo("test -Z panic-abort-tests -v")
+        .with_stderr_contains("[..]--crate-name a [..]-C panic=abort[..]")
+        .with_stderr_contains("[..]--crate-name foo [..]-C panic=abort[..]")
+        .with_stderr_contains("[..]--crate-name foo [..]-C panic=abort[..]--test[..]")
+        .masquerade_as_nightly_cargo()
+        .run();
+}
+
+#[cargo_test]
+fn panic_abort_only_test() {
+    if !is_nightly() {
+        // -Zpanic-abort-tests in rustc is unstable
+        return;
+    }
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = 'foo'
+                version = '0.1.0'
+
+                [dependencies]
+                a = { path = 'a' }
+
+                [profile.test]
+                panic = 'abort'
+            "#,
+        )
+        .file(
+            "src/lib.rs",
+            r#"
+                #[test]
+                fn foo() {
+                    a::foo();
+                }
+            "#,
+        )
+        .file("a/Cargo.toml", &basic_lib_manifest("a"))
+        .file("a/src/lib.rs", "pub fn foo() {}")
+        .build();
+
+    p.cargo("test -Z panic-abort-tests -v")
+        .with_stderr_does_not_contain("[..]--crate-name a [..]-C panic=abort[..]")
+        .with_stderr_contains("[..]--crate-name foo [..]-C panic=abort[..]--test[..]")
+        .masquerade_as_nightly_cargo()
+        .run();
+}
+
+#[cargo_test]
+fn panic_abort_invalid() {
+    if !is_nightly() {
+        // -Zpanic-abort-tests in rustc is unstable
+        return;
+    }
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = 'foo'
+                version = '0.1.0'
+
+                [dependencies]
+                a = { path = 'a' }
+
+                [profile.dev]
+                panic = 'abort'
+            "#,
+        )
+        .file(
+            "src/lib.rs",
+            r#"
+                #[test]
+                fn foo() {
+                    a::foo();
+                }
+            "#,
+        )
+        .file("a/Cargo.toml", &basic_lib_manifest("a"))
+        .file("a/src/lib.rs", "pub fn foo() {}")
+        .build();
+
+    p.cargo("test -Z panic-abort-tests -v")
+        .masquerade_as_nightly_cargo()
+        .with_status(101)
+        .with_stderr_contains("[..]incompatible with this crate's strategy[..]")
+        .run();
+}