Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[beta-1.68] Backport fixes of split-debuginfo detection #11649

Merged
merged 5 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ jobserver = "0.1.24"
lazycell = "1.2.0"
libc = "0.2"
log = "0.4.6"
libgit2-sys = "0.14.1"
# Temporarily pin libgit2-sys due to some issues with SSH not working on
# Windows.
libgit2-sys = "=0.14.1"
memchr = "2.1.3"
opener = "0.5"
os_info = "3.5.0"
Expand All @@ -68,7 +70,7 @@ toml_edit = { version = "0.15.0", features = ["serde", "easy", "perf"] }
unicode-xid = "0.2.0"
url = "2.2.2"
walkdir = "2.2"
clap = "4.1.1"
clap = "4.1.3"
unicode-width = "0.1.5"
openssl = { version = '0.10.11', optional = true }
im-rc = "15.0.0"
Expand Down
78 changes: 53 additions & 25 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 @@ -170,7 +171,8 @@ impl TargetInfo {
// 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
// 2) cfg
// 2) split-debuginfo
// 3) cfg
//
// Search `--print` to see what we query so far.
let mut process = rustc.workspace_process();
Expand Down Expand Up @@ -199,15 +201,9 @@ 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=split-debuginfo");
process.arg("--print=crate-name"); // `___` as a delimiter.
process.arg("--print=cfg");

let (output, error) = rustc
Expand All @@ -223,13 +219,8 @@ impl TargetInfo {
map.insert(crate_type.clone(), out);
}

let line = match lines.next() {
Some(line) => line,
None => anyhow::bail!(
"output of --print=sysroot missing when learning about \
target-specific information from rustc\n{}",
output_err_info(&process, &output, &error)
),
let Some(line) = lines.next() else {
return error_missing_print_output("sysroot", &process, &output, &error);
};
let sysroot = PathBuf::from(line);
let sysroot_host_libdir = if cfg!(windows) {
Expand All @@ -246,6 +237,26 @@ impl TargetInfo {
});
sysroot_target_libdir.push("lib");

let support_split_debuginfo = {
// HACK: abuse `--print=crate-name` to use `___` as a delimiter.
let mut res = Vec::new();
loop {
match lines.next() {
Some(line) if line == "___" => break,
Some(line) => res.push(line.into()),
None => {
return error_missing_print_output(
"split-debuginfo",
&process,
&output,
&error,
)
}
}
}
res
};

let cfg = lines
.map(|line| Ok(Cfg::from_str(line)?))
.filter(TargetInfo::not_user_specific_cfg)
Expand Down Expand Up @@ -303,7 +314,7 @@ impl TargetInfo {
Flags::Rustdoc,
)?,
cfg,
supports_split_debuginfo,
support_split_debuginfo,
});
}
}
Expand Down Expand Up @@ -543,6 +554,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 Expand Up @@ -578,17 +596,27 @@ fn parse_crate_type(
};
let mut parts = line.trim().split("___");
let prefix = parts.next().unwrap();
let suffix = match parts.next() {
Some(part) => part,
None => anyhow::bail!(
"output of --print=file-names has changed in the compiler, cannot parse\n{}",
output_err_info(cmd, output, error)
),
let Some(suffix) = parts.next() else {
return error_missing_print_output("file-names", cmd, output, error);
};

Ok(Some((prefix.to_string(), suffix.to_string())))
}

/// Helper for creating an error message for missing output from a certain `--print` request.
fn error_missing_print_output<T>(
request: &str,
cmd: &ProcessBuilder,
stdout: &str,
stderr: &str,
) -> CargoResult<T> {
let err_info = output_err_info(cmd, stdout, stderr);
anyhow::bail!(
"output of --print={request} missing when learning about \
target-specific information from rustc\n{err_info}",
)
}

/// Helper for creating an error message when parsing rustc output fails.
fn output_err_info(cmd: &ProcessBuilder, stdout: &str, stderr: &str) -> String {
let mut result = format!("command was: {}\n", cmd);
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
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_add/invalid_arg/stderr.log
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
error: unexpected argument '--flag' found

note: to pass '--flag' as a value, use '-- --flag'
note: argument '--tag' exists

Usage: cargo add [OPTIONS] <DEP>[@<VERSION>] ...
cargo add [OPTIONS] --path <PATH> ...
Expand Down
40 changes: 40 additions & 0 deletions tests/testsuite/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,22 @@ fn bad_cfg_discovery() {
return;
}
println!("{}", sysroot);

if mode == "no-split-debuginfo" {
return;
}
loop {
let line = lines.next().unwrap();
if line == "___" {
println!("\n{line}");
break;
} else {
// As the number split-debuginfo options varies,
// concat them into one line.
print!("{line},");
}
};

if mode != "bad-cfg" {
panic!("unexpected");
}
Expand Down Expand Up @@ -412,6 +428,28 @@ command was: `[..]compiler[..]--crate-type [..]`
[..]___[..]
[..]___[..]

",
)
.run();

p.cargo("build")
.env("RUSTC", &funky_rustc)
.env("FUNKY_MODE", "no-split-debuginfo")
.with_status(101)
.with_stderr(
"\
[ERROR] output of --print=split-debuginfo missing when learning about target-specific information from rustc
command was: `[..]compiler[..]--crate-type [..]`

--- stdout
[..]___[..]
[..]___[..]
[..]___[..]
[..]___[..]
[..]___[..]
[..]___[..]
[..]

",
)
.run();
Expand All @@ -430,6 +468,8 @@ command was: `[..]compiler[..]--crate-type [..]`
[..]___[..]
[..]___[..]
[..]
[..],[..]
___
123


Expand Down
8 changes: 8 additions & 0 deletions tests/testsuite/ssh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,10 @@ Caused by:
}

#[cargo_test(public_network_test)]
// For unknown reasons, this test occasionally fails on Windows with a
// LIBSSH2_ERROR_KEY_EXCHANGE_FAILURE error:
// failed to start SSH session: Unable to exchange encryption keys; class=Ssh (23)
#[cfg_attr(windows, ignore = "test is flaky on windows")]
fn invalid_github_key() {
// A key for github.com in known_hosts should override the built-in key.
// This uses a bogus key which should result in an error.
Expand Down Expand Up @@ -417,6 +421,10 @@ fn invalid_github_key() {
}

#[cargo_test(public_network_test)]
// For unknown reasons, this test occasionally fails on Windows with a
// LIBSSH2_ERROR_KEY_EXCHANGE_FAILURE error:
// failed to start SSH session: Unable to exchange encryption keys; class=Ssh (23)
#[cfg_attr(windows, ignore = "test is flaky on windows")]
fn bundled_github_works() {
// The bundled key for github.com works.
//
Expand Down