Skip to content

Commit

Permalink
Auto merge of #11347 - kamirr:fix-split-debuginfo-windows, r=<try>
Browse files Browse the repository at this point in the history
Fix split-debuginfo support detection

### What does this PR try to resolve?
cargo assumed that if `-Csplit-debuginfo=packed` worked, all values would be correct. This however is not the case -- as of Rust 1.65.0, rustc supports `packed`, but not `unpacked` or `off` on Windows. Because of this, having `split-debuginfo="unpacked`" on Windows has caused builds to fail, as cargo assumed that the option is fine (`split-debuginfo=packed` worked), but rustc then failed when being passed `-Csplit-debuginfo=unpacked`.

### How should we test and review this PR?
Consider an empty project with the following change to `Cargo.toml`:
```toml
[profile.dev]
split-debuginfo="unpacked"
```
`cargo +1.64.0 build` will work, but `cargo +1.65.0 build` will fail with the following error message:

```
PS C:\REDACTED> cargo build
   Compiling tmp v0.1.0 (C:\REDACTED)
error: `-Csplit-debuginfo=unpacked` is unstable on this platform

error: could not compile `tmp` due to previous error
```

With this patch and 1.65.0 rustc, cargo will ignore all `split-debuginfo` settings, but with rust-lang/rust#104104 rustc (approved, awaiting merge), it will properly detect supported values for `split-debuginfo` and only ignore those that are unsupported.
  • Loading branch information
bors committed Jan 26, 2023
2 parents 2ccd950 + fe95630 commit 641a4e1
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 15 deletions.
34 changes: 23 additions & 11 deletions src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::core::compiler::{
};
use crate::core::{Dependency, Package, Target, TargetKind, Workspace};
use crate::util::config::{Config, StringList, TargetConfig};
use crate::util::interning::InternedString;
use crate::util::{CargoResult, Rustc};
use anyhow::Context as _;
use cargo_platform::{Cfg, CfgExpr};
Expand Down Expand Up @@ -43,6 +44,8 @@ pub struct TargetInfo {
crate_types: RefCell<HashMap<CrateType, Option<(String, String)>>>,
/// `cfg` information extracted from `rustc --print=cfg`.
cfg: Vec<Cfg>,
/// Supported values for `-Csplit-debuginfo=` flag, queried from rustc
support_split_debuginfo: Vec<String>,
/// Path to the sysroot.
pub sysroot: PathBuf,
/// Path to the "lib" or "bin" directory that rustc uses for its dynamic
Expand All @@ -55,8 +58,6 @@ pub struct TargetInfo {
pub rustflags: Vec<String>,
/// Extra flags to pass to `rustdoc`, see [`extra_args`].
pub rustdocflags: Vec<String>,
/// Whether or not rustc supports the `-Csplit-debuginfo` flag.
pub supports_split_debuginfo: bool,
}

/// Kind of each file generated by a Unit, part of `FileType`.
Expand Down Expand Up @@ -167,6 +168,18 @@ impl TargetInfo {
loop {
let extra_fingerprint = kind.fingerprint_hash();

// Query rustc for supported -Csplit-debuginfo values
let support_split_debuginfo = rustc
.cached_output(
rustc.workspace_process().arg("--print=split-debuginfo"),
extra_fingerprint,
)
.unwrap_or_default()
.0
.lines()
.map(String::from)
.collect();

// Query rustc for several kinds of info from each line of output:
// 0) file-names (to determine output file prefix/suffix for given crate type)
// 1) sysroot
Expand Down Expand Up @@ -199,14 +212,6 @@ impl TargetInfo {
process.arg("--crate-type").arg(crate_type.as_str());
}

// An extra `rustc` call to determine `-Csplit-debuginfo=packed` support.
let supports_split_debuginfo = rustc
.cached_output(
process.clone().arg("-Csplit-debuginfo=packed"),
extra_fingerprint,
)
.is_ok();

process.arg("--print=sysroot");
process.arg("--print=cfg");

Expand Down Expand Up @@ -303,7 +308,7 @@ impl TargetInfo {
Flags::Rustdoc,
)?,
cfg,
supports_split_debuginfo,
support_split_debuginfo,
});
}
}
Expand Down Expand Up @@ -543,6 +548,13 @@ impl TargetInfo {
}
Ok((result, unsupported))
}

/// Checks if the debuginfo-split value is supported by this target
pub fn supports_debuginfo_split(&self, split: InternedString) -> bool {
self.support_split_debuginfo
.iter()
.any(|sup| sup.as_str() == split.as_str())
}
}

/// Takes rustc output (using specialized command line args), and calculates the file prefix and
Expand Down
15 changes: 11 additions & 4 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -980,10 +980,17 @@ fn build_base_args(
cmd.args(&lto_args(cx, unit));

// This is generally just an optimization on build time so if we don't pass
// it then it's ok. As of the time of this writing it's a very new flag, so
// we need to dynamically check if it's available.
if cx.bcx.target_data.info(unit.kind).supports_split_debuginfo {
if let Some(split) = split_debuginfo {
// it then it's ok. The values for the flag (off, packed, unpacked) may be supported
// or not depending on the platform, so availability is checked per-value.
// For example, at the time of writing this code, on Windows the only stable valid
// value for split-debuginfo is "packed", while on Linux "unpacked" is also stable.
if let Some(split) = split_debuginfo {
if cx
.bcx
.target_data
.info(unit.kind)
.supports_debuginfo_split(split)
{
cmd.arg("-C").arg(format!("split-debuginfo={}", split));
}
}
Expand Down

0 comments on commit 641a4e1

Please sign in to comment.