Inherit panic settings in test/bench profiles

We've always ignored the `panic` settings configured in test/bench
profiles, so this just continues to ignore them but in a slightly
different way.

Author: alexcrichton

src/cargo/core/profiles.rs

@@ -314,7 +314,7 @@ impl Profiles {
         mode: CompileMode,
         profile_kind: ProfileKind,
     ) -> Profile {
-        let profile_name = if !self.named_profiles_enabled {
+        let (profile_name, inherits) = if !self.named_profiles_enabled {
             // With the feature disabled, we degrade `--profile` back to the
             // `--release` and `--debug` predicates, and convert back from
             // ProfileKind::Custom instantiation.
@@ -328,9 +328,9 @@ impl Profiles {
             match mode {
                 CompileMode::Test | CompileMode::Bench => {
                     if release {
-                        "bench"
+                        ("bench", Some("release"))
                     } else {
-                        "test"
+                        ("test", Some("dev"))
                     }
                 }
                 CompileMode::Build
@@ -342,26 +342,33 @@ impl Profiles {
                     // ancestor's profile. However, `cargo clean -p` can hit this
                     // path.
                     if release {
-                        "release"
+                        ("release", None)
                     } else {
-                        "dev"
+                        ("dev", None)
                     }
                 }
-                CompileMode::Doc { .. } => "doc",
+                CompileMode::Doc { .. } => ("doc", None),
             }
         } else {
-            profile_kind.name()
+            (profile_kind.name(), None)
         };
         let maker = match self.by_name.get(profile_name) {
             None => panic!("Profile {} undefined", profile_name),
             Some(r) => r,
         };
         let mut profile = maker.get_profile(Some(pkg_id), is_member, unit_for);
-        // `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;
+
+        // Dealing with `panic=abort` and `panic=unwind` requires some special
+        // treatment. Be sure to process all the various options here.
+        match unit_for.panic_setting() {
+            PanicSetting::AlwaysUnwind => profile.panic = PanicStrategy::Unwind,
+            PanicSetting::ReadProfile => {}
+            PanicSetting::Inherit => {
+                if let Some(inherits) = inherits {
+                    let maker = self.by_name.get(inherits).unwrap();
+                    profile.panic = maker.get_profile(Some(pkg_id), is_member, unit_for).panic;
+                }
+            }
         }
 
         // Incremental can be globally overridden.
@@ -881,11 +888,25 @@ pub struct UnitFor {
     /// any of its dependencies. This enables `build-override` profiles for
     /// these targets.
     build: bool,
-    /// This is true if it is *allowed* to set the `panic=abort` flag. Currently
-    /// this is false for test/bench targets and all their dependencies, and
-    /// "for_host" units such as proc macro and custom build scripts and their
-    /// dependencies.
-    panic_abort_ok: bool,
+    /// How Cargo processes the `panic` setting or profiles. This is done to
+    /// handle test/benches inheriting from dev/release, as well as forcing
+    /// `for_host` units to always unwind.
+    panic_setting: PanicSetting,
+}
+
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
+enum PanicSetting {
+    /// Used to force a unit to always be compiled with the `panic=unwind`
+    /// strategy, notably for build scripts, proc macros, etc.
+    AlwaysUnwind,
+
+    /// Indicates that this unit will read its `profile` setting and use
+    /// whatever is configured there.
+    ReadProfile,
+
+    /// This unit will ignore its `panic` setting in its profile and will
+    /// instead inherit it from the `dev` or `release` profile, as appropriate.
+    Inherit,
 }
 
 impl UnitFor {
@@ -894,7 +915,7 @@ impl UnitFor {
     pub fn new_normal() -> UnitFor {
         UnitFor {
             build: false,
-            panic_abort_ok: true,
+            panic_setting: PanicSetting::ReadProfile,
         }
     }
 
@@ -904,7 +925,7 @@ impl UnitFor {
             build: true,
             // Force build scripts to always use `panic=unwind` for now to
             // maximally share dependencies with procedural macros.
-            panic_abort_ok: false,
+            panic_setting: PanicSetting::AlwaysUnwind,
         }
     }
 
@@ -915,7 +936,7 @@ impl UnitFor {
             // 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,
+            panic_setting: PanicSetting::AlwaysUnwind,
         }
     }
 
@@ -928,7 +949,15 @@ impl UnitFor {
     pub fn new_test(config: &Config) -> UnitFor {
         UnitFor {
             build: false,
-            panic_abort_ok: config.cli_unstable().panic_abort_tests,
+            // We're testing out an unstable feature (`-Zpanic-abort-tests`)
+            // which inherits the panic setting from the dev/release profile
+            // (basically avoid recompiles) but historical defaults required
+            // that we always unwound.
+            panic_setting: if config.cli_unstable().panic_abort_tests {
+                PanicSetting::Inherit
+            } else {
+                PanicSetting::AlwaysUnwind
+            },
         }
     }
 
@@ -942,7 +971,11 @@ impl UnitFor {
     pub fn with_for_host(self, for_host: bool) -> UnitFor {
         UnitFor {
             build: self.build || for_host,
-            panic_abort_ok: self.panic_abort_ok && !for_host,
+            panic_setting: if for_host {
+                PanicSetting::AlwaysUnwind
+            } else {
+                self.panic_setting
+            },
         }
     }
 
@@ -952,29 +985,32 @@ impl UnitFor {
         self.build
     }
 
-    /// 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
+    /// Returns how `panic` settings should be handled for this profile
+    fn panic_setting(self) -> PanicSetting {
+        self.panic_setting
     }
 
     /// All possible values, used by `clean`.
     pub fn all_values() -> &'static [UnitFor] {
-        static ALL: [UnitFor; 3] = [
+        static ALL: &[UnitFor] = &[
             UnitFor {
                 build: false,
-                panic_abort_ok: true,
+                panic_setting: PanicSetting::ReadProfile,
             },
             UnitFor {
                 build: true,
-                panic_abort_ok: false,
+                panic_setting: PanicSetting::AlwaysUnwind,
+            },
+            UnitFor {
+                build: false,
+                panic_setting: PanicSetting::AlwaysUnwind,
             },
             UnitFor {
                 build: false,
-                panic_abort_ok: false,
+                panic_setting: PanicSetting::Inherit,
             },
         ];
-        &ALL
+        ALL
     }
 }
 

tests/testsuite/test.rs

@@ -3954,14 +3954,13 @@ fn panic_abort_only_test() {
         .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[..]")
+        .with_stderr_contains("warning: `panic` setting is ignored for `test` profile")
         .masquerade_as_nightly_cargo()
         .run();
 }
 
 #[cargo_test]
-fn panic_abort_invalid() {
+fn panic_abort_test_profile_inherits() {
     if !is_nightly() {
         // -Zpanic-abort-tests in rustc is unstable
         return;
@@ -3997,7 +3996,6 @@ fn panic_abort_invalid() {
 
     p.cargo("test -Z panic-abort-tests -v")
         .masquerade_as_nightly_cargo()
-        .with_status(101)
-        .with_stderr_contains("[..]incompatible with this crate's strategy[..]")
+        .with_status(0)
         .run();
 }