Rollup merge of rust-lang#121754 - TimNN:split-target, r=Mark-Simulacrum

[bootstrap] Move the `split-debuginfo` setting to the per-target section

As described in rust-lang#112406, bootstrap currently applies the global `split-debuginfo` setting to all artifacts, irrespective of their target triple.

This doesn't cause problems when the "build" triple defaults `split-debuginfo` to `off` (as is the case on Linux, for example).

However, when the "build" triple has `split-debuginfo` enabled and additional target triples are configured, then artifacts for the additional triples will also be built with `split-debuginfo` (despite not necessarily supporting `split-debuginfo`).

rust-lang#112406 mentions `riscv32imc-unknown-none-elf` as one target where this happens, and I've run into this with Wasm as well.

This PR does **not** implement ``@ehuss's`` suggestion that "bootstrap not try to guess how to configure split-debuginfo, and instead use cargo profiles to set it", because that seemed like a lot more significant change.

---

After this PR, anyone explicitly setting `rust.split-debuginfo` should update their configuration to specify the setting in the `target.<triple>` section, though `rust.split-debuginfo` will still be honored for the "build" triple for now.

This PR changes the behavior when `rust.split-debuginfo` was **not** explicitly set **and** bootstrap is configured to cross-compile to a triple that has a different `split-debuginfo` than the "build" triple.

---

If there's a reasonable way to add additional tests for this, please let me know (I didn't find any tests checking cargo arguments in [`builder/tests.rs`](https://github.com/rust-lang/rust/blob/master/src/bootstrap/src/core/builder/tests.rs)).

Author: matthiaskrgr

config.example.toml

@@ -543,23 +543,15 @@
 # FIXME(#61117): Some tests fail when this option is enabled.
 #debuginfo-level-tests = 0
 
-# Should rustc be build with split debuginfo? Default is platform dependent.
-# Valid values are the same as those accepted by `-C split-debuginfo`
-# (`off`/`unpacked`/`packed`).
+# Should rustc and the standard library be built with split debuginfo? Default
+# is platform dependent.
 #
-# On Linux, split debuginfo is disabled by default.
+# This field is deprecated, use `target.<triple>.split-debuginfo` instead.
 #
-# On Apple platforms, unpacked split debuginfo is used by default. Unpacked
-# debuginfo does not run `dsymutil`, which packages debuginfo from disparate
-# object files into a single `.dSYM` file. `dsymutil` adds time to builds for
-# no clear benefit, and also makes it more difficult for debuggers to find
-# debug info. The compiler currently defaults to running `dsymutil` to preserve
-# its historical default, but when compiling the compiler itself, we skip it by
-# default since we know it's safe to do so in that case.
+# The value specified here is only used when targeting the `build.build` triple,
+# and is overridden by `target.<triple>.split-debuginfo` if specified.
 #
-# On Windows platforms, packed debuginfo is the only supported option,
-# producing a `.pdb` file.
-#split-debuginfo = if linux { off } else if windows { packed } else if apple { unpacked }
+#split-debuginfo = see target.<triple>.split-debuginfo
 
 # Whether or not `panic!`s generate backtraces (RUST_BACKTRACE)
 #backtrace = true
@@ -769,6 +761,26 @@
 # Setting this will override the `use-lld` option for Rust code when targeting MSVC.
 #linker = "cc" (path)
 
+# Should rustc and the standard library be built with split debuginfo? Default
+# is platform dependent.
+#
+# Valid values are the same as those accepted by `-C split-debuginfo`
+# (`off`/`unpacked`/`packed`).
+#
+# On Linux, split debuginfo is disabled by default.
+#
+# On Apple platforms, unpacked split debuginfo is used by default. Unpacked
+# debuginfo does not run `dsymutil`, which packages debuginfo from disparate
+# object files into a single `.dSYM` file. `dsymutil` adds time to builds for
+# no clear benefit, and also makes it more difficult for debuggers to find
+# debug info. The compiler currently defaults to running `dsymutil` to preserve
+# its historical default, but when compiling the compiler itself, we skip it by
+# default since we know it's safe to do so in that case.
+#
+# On Windows platforms, packed debuginfo is the only supported option,
+# producing a `.pdb` file.
+#split-debuginfo = if linux { off } else if windows { packed } else if apple { unpacked }
+
 # Path to the `llvm-config` binary of the installation of a custom LLVM to link
 # against. Note that if this is specified we don't compile LLVM at all for this
 # target.

src/bootstrap/src/core/builder.rs

@@ -1730,15 +1730,16 @@ impl<'a> Builder<'a> {
             },
         );
 
+        let split_debuginfo = self.config.split_debuginfo(target);
         let split_debuginfo_is_stable = target.contains("linux")
             || target.contains("apple")
-            || (target.is_msvc() && self.config.rust_split_debuginfo == SplitDebuginfo::Packed)
-            || (target.is_windows() && self.config.rust_split_debuginfo == SplitDebuginfo::Off);
+            || (target.is_msvc() && split_debuginfo == SplitDebuginfo::Packed)
+            || (target.is_windows() && split_debuginfo == SplitDebuginfo::Off);
 
         if !split_debuginfo_is_stable {
             rustflags.arg("-Zunstable-options");
         }
-        match self.config.rust_split_debuginfo {
+        match split_debuginfo {
             SplitDebuginfo::Packed => rustflags.arg("-Csplit-debuginfo=packed"),
             SplitDebuginfo::Unpacked => rustflags.arg("-Csplit-debuginfo=unpacked"),
             SplitDebuginfo::Off => rustflags.arg("-Csplit-debuginfo=off"),

src/bootstrap/src/core/config/config.rs

@@ -255,7 +255,7 @@ pub struct Config {
     pub rust_debuginfo_level_std: DebuginfoLevel,
     pub rust_debuginfo_level_tools: DebuginfoLevel,
     pub rust_debuginfo_level_tests: DebuginfoLevel,
-    pub rust_split_debuginfo: SplitDebuginfo,
+    pub rust_split_debuginfo_for_build_triple: Option<SplitDebuginfo>, // FIXME: Deprecated field. Remove in Q3'24.
     pub rust_rpath: bool,
     pub rust_strip: bool,
     pub rust_frame_pointers: bool,
@@ -573,6 +573,7 @@ pub struct Target {
     pub ranlib: Option<PathBuf>,
     pub default_linker: Option<PathBuf>,
     pub linker: Option<PathBuf>,
+    pub split_debuginfo: Option<SplitDebuginfo>,
     pub sanitizers: Option<bool>,
     pub profiler: Option<StringOrBool>,
     pub rpath: Option<bool>,
@@ -1130,6 +1131,7 @@ define_config! {
         ranlib: Option<String> = "ranlib",
         default_linker: Option<PathBuf> = "default-linker",
         linker: Option<String> = "linker",
+        split_debuginfo: Option<String> = "split-debuginfo",
         llvm_config: Option<String> = "llvm-config",
         llvm_has_rust_patches: Option<bool> = "llvm-has-rust-patches",
         llvm_filecheck: Option<String> = "llvm-filecheck",
@@ -1622,11 +1624,18 @@ impl Config {
             debuginfo_level_tools = debuginfo_level_tools_toml;
             debuginfo_level_tests = debuginfo_level_tests_toml;
 
-            config.rust_split_debuginfo = split_debuginfo
+            config.rust_split_debuginfo_for_build_triple = split_debuginfo
                 .as_deref()
                 .map(SplitDebuginfo::from_str)
-                .map(|v| v.expect("invalid value for rust.split_debuginfo"))
-                .unwrap_or(SplitDebuginfo::default_for_platform(config.build));
+                .map(|v| v.expect("invalid value for rust.split-debuginfo"));
+
+            if config.rust_split_debuginfo_for_build_triple.is_some() {
+                println!(
+                    "WARNING: specifying `rust.split-debuginfo` is deprecated, use `target.{}.split-debuginfo` instead",
+                    config.build
+                );
+            }
+
             optimize = optimize_toml;
             omit_git_hash = omit_git_hash_toml;
             config.rust_new_symbol_mangling = new_symbol_mangling;
@@ -1847,10 +1856,11 @@ impl Config {
                 if let Some(ref s) = cfg.llvm_filecheck {
                     target.llvm_filecheck = Some(config.src.join(s));
                 }
-                target.llvm_libunwind = cfg
-                    .llvm_libunwind
-                    .as_ref()
-                    .map(|v| v.parse().expect("failed to parse rust.llvm-libunwind"));
+                target.llvm_libunwind = cfg.llvm_libunwind.as_ref().map(|v| {
+                    v.parse().unwrap_or_else(|_| {
+                        panic!("failed to parse target.{triple}.llvm-libunwind")
+                    })
+                });
                 if let Some(s) = cfg.no_std {
                     target.no_std = s;
                 }
@@ -1886,6 +1896,12 @@ impl Config {
                     }).collect());
                 }
 
+                target.split_debuginfo = cfg.split_debuginfo.as_ref().map(|v| {
+                    v.parse().unwrap_or_else(|_| {
+                        panic!("invalid value for target.{triple}.split-debuginfo")
+                    })
+                });
+
                 config.target_config.insert(TargetSelection::from_user(&triple), target);
             }
         }
@@ -2284,6 +2300,16 @@ impl Config {
             })
     }
 
+    pub fn split_debuginfo(&self, target: TargetSelection) -> SplitDebuginfo {
+        self.target_config
+            .get(&target)
+            .and_then(|t| t.split_debuginfo)
+            .or_else(|| {
+                if self.build == target { self.rust_split_debuginfo_for_build_triple } else { None }
+            })
+            .unwrap_or_else(|| SplitDebuginfo::default_for_platform(target))
+    }
+
     pub fn submodules(&self, rust_info: &GitInfo) -> bool {
         self.submodules.unwrap_or(rust_info.is_managed_git_subrepository())
     }

src/bootstrap/src/utils/change_tracker.rs

@@ -141,4 +141,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
         severity: ChangeSeverity::Info,
         summary: "A new `boostrap-cache-path` option has been introduced which can be utilized to modify the cache path for bootstrap.",
     },
+    ChangeInfo {
+        change_id: 121754,
+        severity: ChangeSeverity::Warning,
+        summary: "`rust.split-debuginfo` has been moved to `target.<triple>.split-debuginfo` and its default value is determined for each target individually.",
+    },
 ];