From 7aa649ba2210a5536ba6888421f9ca6caebfeac1 Mon Sep 17 00:00:00 2001 From: bors Date: Thu, 26 Jan 2023 18:14:08 +0000 Subject: [PATCH 1/5] 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)); } } From bd3c324bfaf1f3125a3af266cc48e1add286e469 Mon Sep 17 00:00:00 2001 From: bors Date: Sat, 28 Jan 2023 20:03:00 +0000 Subject: [PATCH 2/5] Auto merge of #11633 - weihanglo:reduce-rustc-query-calls, r=ehuss Reduce target info rustc query calls --- .../compiler/build_context/target_info.rs | 68 ++++++++++++------- tests/testsuite/cfg.rs | 40 +++++++++++ 2 files changed, 82 insertions(+), 26 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 61d1f331615..1ec0d9b5b4e 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -168,22 +168,11 @@ 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 - // 2) cfg + // 2) split-debuginfo + // 3) cfg // // Search `--print` to see what we query so far. let mut process = rustc.workspace_process(); @@ -213,6 +202,8 @@ impl TargetInfo { } 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 @@ -228,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) { @@ -251,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) @@ -590,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( + request: &str, + cmd: &ProcessBuilder, + stdout: &str, + stderr: &str, +) -> CargoResult { + 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); diff --git a/tests/testsuite/cfg.rs b/tests/testsuite/cfg.rs index 3f79db77275..aac5c02b65a 100644 --- a/tests/testsuite/cfg.rs +++ b/tests/testsuite/cfg.rs @@ -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"); } @@ -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(); @@ -430,6 +468,8 @@ command was: `[..]compiler[..]--crate-type [..]` [..]___[..] [..]___[..] [..] +[..],[..] +___ 123 From f19479a62480eb802d3d5e81900228934793725a Mon Sep 17 00:00:00 2001 From: bors Date: Tue, 24 Jan 2023 04:52:00 +0000 Subject: [PATCH 3/5] Auto merge of #11619 - epage:test, r=ehuss test: Update for clap 4.1.3 The latest clap release fixed a bug with the algorithm for providing suggestions, leading this suggestion to change. --- Cargo.toml | 2 +- tests/testsuite/cargo_add/invalid_arg/stderr.log | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c91b2d95278..324f3a53654 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -68,7 +68,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" diff --git a/tests/testsuite/cargo_add/invalid_arg/stderr.log b/tests/testsuite/cargo_add/invalid_arg/stderr.log index 8274b20ad1b..b5ee3cca39c 100644 --- a/tests/testsuite/cargo_add/invalid_arg/stderr.log +++ b/tests/testsuite/cargo_add/invalid_arg/stderr.log @@ -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] [@] ... cargo add [OPTIONS] --path ... From 5bd7a2c07eeac35eae513b24c2c232f860fbc8e4 Mon Sep 17 00:00:00 2001 From: bors Date: Sat, 21 Jan 2023 22:56:05 +0000 Subject: [PATCH 4/5] Auto merge of #11609 - ehuss:libgit2-sys-pin, r=weihanglo Temporarily pin libgit2-sys. There are some issues with the most recent libgit2-sys 0.14.2 not working on Windows (https://github.com/libgit2/libgit2/issues/6453 and https://github.com/libgit2/libgit2/issues/6454). Until we figure out what to do with it, this pins the release to the previous version. --- Cargo.toml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 324f3a53654..cf3bfb0d968 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" From 6fe6437ac8a0a69a101f71bda7358d4d6e3ac694 Mon Sep 17 00:00:00 2001 From: bors Date: Sat, 21 Jan 2023 21:15:35 +0000 Subject: [PATCH 5/5] Auto merge of #11610 - ehuss:disable-public-network-windows, r=epage Disable network SSH tests on windows. These tests have a high failure rate on Windows, so this disables them for now. I don't know exactly why they are failing. If I had to take a wild guess, I would suspect the use of WinCNG for the SSH backend, which may mean it is out of our control. --- tests/testsuite/ssh.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/testsuite/ssh.rs b/tests/testsuite/ssh.rs index b0fe9ffff63..e8f5ce7556c 100644 --- a/tests/testsuite/ssh.rs +++ b/tests/testsuite/ssh.rs @@ -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. @@ -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. //