Skip to content

Commit

Permalink
[bootstrap] Move the split-debuginfo setting to the per-target section
Browse files Browse the repository at this point in the history
  • Loading branch information
TimNN committed Feb 28, 2024
1 parent ef32456 commit de8a80b
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 31 deletions.
38 changes: 20 additions & 18 deletions config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down
7 changes: 4 additions & 3 deletions src/bootstrap/src/core/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
38 changes: 28 additions & 10 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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())
}
Expand Down
6 changes: 6 additions & 0 deletions src/bootstrap/src/core/config/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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\"");
}
5 changes: 5 additions & 0 deletions src/bootstrap/src/utils/change_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
},
];

0 comments on commit de8a80b

Please sign in to comment.