From de8a80be44eec3de0e55def873cff186f70bc44d Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Wed, 28 Feb 2024 18:00:16 +0100 Subject: [PATCH] [bootstrap] Move the `split-debuginfo` setting to the per-target section --- config.example.toml | 38 ++++++++++++----------- src/bootstrap/src/core/builder.rs | 7 +++-- src/bootstrap/src/core/config/config.rs | 38 +++++++++++++++++------ src/bootstrap/src/core/config/tests.rs | 6 ++++ src/bootstrap/src/utils/change_tracker.rs | 5 +++ 5 files changed, 63 insertions(+), 31 deletions(-) diff --git a/config.example.toml b/config.example.toml index cef33a7905a3b..9b6627655cf67 100644 --- a/config.example.toml +++ b/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. diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index e169cba3c13ae..db834df5f97fe 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/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"), diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 875a4efae02fc..0fa7f33a4b011 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/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, pub default_linker: Option, pub linker: Option, + pub split_debuginfo: Option, pub sanitizers: Option, pub profiler: Option, pub rpath: Option, @@ -1126,6 +1126,7 @@ define_config! { ranlib: Option = "ranlib", default_linker: Option = "default-linker", linker: Option = "linker", + split_debuginfo: Option = "split-debuginfo", llvm_config: Option = "llvm-config", llvm_has_rust_patches: Option = "llvm-has-rust-patches", llvm_filecheck: Option = "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..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()) } diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs index 6ac573c68df18..177482401841e 100644 --- a/src/bootstrap/src/core/config/tests.rs +++ b/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\""); +} diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs index 9a50ad4437e73..8fb64c0a20aa9 100644 --- a/src/bootstrap/src/utils/change_tracker.rs +++ b/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..split-debuginfo` and its default value is determined for each target individually.", + }, ];