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

Author: TimNN

config.example.toml

@@ -539,24 +539,6 @@
 # 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`).
-#
-# 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 }
-
 # Whether or not `panic!`s generate backtraces (RUST_BACKTRACE)
 #backtrace = true
 
@@ -765,6 +747,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

@@ -254,7 +254,6 @@ 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_rpath: bool,
     pub rust_strip: bool,
     pub rust_frame_pointers: bool,
@@ -570,6 +569,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>,
@@ -1126,6 +1126,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",
@@ -1616,11 +1617,14 @@ impl Config {
             debuginfo_level_tools = debuginfo_level_tools_toml;
             debuginfo_level_tests = debuginfo_level_tests_toml;
 
-            config.rust_split_debuginfo = 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));
+            // FIXME: Remove this and the `Rust.split_debuginfo` field after people had time to
+            // migrate. (The changelog can only be shown if the config parses successfully, so keep
+            // the field around and provide a useful error message).
+            assert!(
+                split_debuginfo.is_none(),
+                "`rust.split_debuginfo` is no longer supported, specify `target.<triple>.split_debuginfo` instead."
+            );
+
             optimize = optimize_toml;
             omit_git_hash = omit_git_hash_toml;
             config.rust_new_symbol_mangling = new_symbol_mangling;
@@ -1841,10 +1845,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;
                 }
@@ -1880,6 +1885,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);
             }
         }
@@ -2278,6 +2289,13 @@ impl Config {
             })
     }
 
+    pub fn split_debuginfo(&self, target: TargetSelection) -> SplitDebuginfo {
+        self.target_config
+            .get(&target)
+            .and_then(|t| t.split_debuginfo)
+            .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/core/config/tests.rs

@@ -237,3 +237,9 @@ fn rust_lld() {
     assert!(matches!(parse("rust.use-lld = true").lld_mode, LldMode::External));
     assert!(matches!(parse("rust.use-lld = false").lld_mode, LldMode::Unused));
 }
+
+#[test]
+#[should_panic(expected = "`rust.split_debuginfo` is no longer supported")]
+fn split_debuginfo_wrong_location() {
+    parse("rust.split-debuginfo = \"off\"");
+}

src/bootstrap/src/utils/change_tracker.rs

@@ -136,4 +136,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
         severity: ChangeSeverity::Info,
         summary: "`x install` now skips providing tarball sources (under 'build/dist' path) to speed up the installation process.",
     },
+    ChangeInfo {
+        change_id: 0, // TODO
+        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.",
+    },
 ];