Skip to content

Commit

Permalink
Unrolled build for rust-lang#121754
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#121754 - TimNN:split-target, r=Mark-Simulacrum,onur-ozkan

[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)).
  • Loading branch information
rust-timer committed Mar 12, 2024
2 parents 7de1a1f + 0e354c9 commit c254d13
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 25 deletions.
40 changes: 26 additions & 14 deletions config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -773,6 +765,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 @@ -1731,15 +1731,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
42 changes: 34 additions & 8 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,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,
Expand Down Expand Up @@ -574,6 +574,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 @@ -1133,6 +1134,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 @@ -1627,11 +1629,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;
Expand Down Expand Up @@ -1853,10 +1862,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 @@ -1893,6 +1903,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 @@ -2291,6 +2307,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())
}
Expand Down
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 @@ -151,4 +151,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
severity: ChangeSeverity::Info,
summary: "New option `rust.llvm-bitcode-linker` that will build the llvm-bitcode-linker.",
},
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.",
},
];

0 comments on commit c254d13

Please sign in to comment.