From 7aa649ba2210a5536ba6888421f9ca6caebfeac1 Mon Sep 17 00:00:00 2001 From: bors Date: Thu, 26 Jan 2023 18:14:08 +0000 Subject: [PATCH] Auto merge of #11347 - kamirr:fix-split-debuginfo-windows, r=weihanglo 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 https://github.com/rust-lang/rust/pull/104104 rustc (approved, awaiting merge), it will properly detect supported values for `split-debuginfo` and only ignore those that are unsupported. --- .../compiler/build_context/target_info.rs | 34 +++++++++++++------ src/cargo/core/compiler/mod.rs | 15 +++++--- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 61398a7d66e..61d1f331615 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -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}; @@ -43,6 +44,8 @@ pub struct TargetInfo { crate_types: RefCell>>, /// `cfg` information extracted from `rustc --print=cfg`. cfg: Vec, + /// Supported values for `-Csplit-debuginfo=` flag, queried from rustc + support_split_debuginfo: Vec, /// Path to the sysroot. pub sysroot: PathBuf, /// Path to the "lib" or "bin" directory that rustc uses for its dynamic @@ -55,8 +58,6 @@ pub struct TargetInfo { pub rustflags: Vec, /// Extra flags to pass to `rustdoc`, see [`extra_args`]. pub rustdocflags: Vec, - /// 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`. @@ -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 @@ -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"); @@ -303,7 +308,7 @@ impl TargetInfo { Flags::Rustdoc, )?, cfg, - supports_split_debuginfo, + support_split_debuginfo, }); } } @@ -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 diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 8e477607171..0978b584bc3 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -980,10 +980,17 @@ fn build_base_args( cmd.args(<o_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)); } }