From f360184f33fe295163945d4e8b3c37979b7de1d9 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Thu, 21 Sep 2023 14:10:55 -0400 Subject: [PATCH 1/3] detect-targets: add debug tracing See #1375. --- Cargo.lock | 2 ++ crates/detect-targets/Cargo.toml | 2 ++ crates/detect-targets/src/detect.rs | 11 ++++--- crates/detect-targets/src/detect/linux.rs | 37 +++++++++++++++++------ crates/detect-targets/src/main.rs | 5 +++ 5 files changed, 43 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6fb46ca3e..91b743ff2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -833,6 +833,8 @@ dependencies = [ "cfg-if", "guess_host_triple", "tokio", + "tracing", + "tracing-subscriber", "windows-dll", "windows-sys", ] diff --git a/crates/detect-targets/Cargo.toml b/crates/detect-targets/Cargo.toml index 9e3613ba2..c7d7a766d 100644 --- a/crates/detect-targets/Cargo.toml +++ b/crates/detect-targets/Cargo.toml @@ -11,6 +11,8 @@ license = "Apache-2.0 OR MIT" [dependencies] tokio = { version = "1.28.2", features = ["rt", "process", "sync"], default-features = false } +tracing = "0.1.37" +tracing-subscriber = { version = "0.3.17", features = ["fmt"], default-features = false } cfg-if = "1.0.0" guess_host_triple = "0.1.3" diff --git a/crates/detect-targets/src/detect.rs b/crates/detect-targets/src/detect.rs index d9fb9df6f..b24364617 100644 --- a/crates/detect-targets/src/detect.rs +++ b/crates/detect-targets/src/detect.rs @@ -7,6 +7,7 @@ use std::{ use cfg_if::cfg_if; use tokio::process::Command; +use tracing::debug; cfg_if! { if #[cfg(target_os = "linux")] { @@ -32,10 +33,12 @@ cfg_if! { /// Check [this issue](https://github.com/ryankurte/cargo-binstall/issues/155) /// for more information. pub async fn detect_targets() -> Vec { - let target = get_target_from_rustc().await.unwrap_or_else(|| { - guess_host_triple::guess_host_triple() - .unwrap_or(crate::TARGET) - .to_string() + let target = get_target_from_rustc().await; + debug!("get_target_from_rustc()={target:?}"); + let target = target.unwrap_or_else(|| { + let target = guess_host_triple::guess_host_triple(); + debug!("guess_host_triple::guess_host_triple()={target:?}"); + target.unwrap_or(crate::TARGET).to_string() }); cfg_if! { diff --git a/crates/detect-targets/src/detect/linux.rs b/crates/detect-targets/src/detect/linux.rs index a945c881e..a47836c79 100644 --- a/crates/detect-targets/src/detect/linux.rs +++ b/crates/detect-targets/src/detect/linux.rs @@ -4,6 +4,7 @@ use std::{ }; use tokio::{process::Command, task}; +use tracing::debug; pub(super) async fn detect_targets(target: String) -> Vec { let (prefix, postfix) = target @@ -79,12 +80,18 @@ async fn get_ld_flavor(cmd: &str) -> Option { status, stdout, stderr, - } = Command::new(cmd) + } = match Command::new(cmd) .arg("--version") .stdin(Stdio::null()) .output() .await - .ok()?; + { + Ok(output) => output, + Err(err) => { + debug!("{cmd}: err={err:?}"); + return None; + } + }; const ALPINE_GCOMPAT: &str = r#"This is the gcompat ELF interpreter stub. You are not meant to run this directly. @@ -93,20 +100,26 @@ You are not meant to run this directly. if status.success() { // Executing glibc ldd or /lib/ld-linux-{cpu_arch}.so.1 will always // succeeds. - String::from_utf8_lossy(&stdout) - .contains("GLIBC") - .then_some(Libc::Gnu) + let stdout = String::from_utf8_lossy(&stdout); + debug!("{cmd}: stdout={stdout}"); + stdout.contains("GLIBC").then_some(Libc::Gnu) } else if status.code() == Some(1) { // On Alpine, executing both the gcompat glibc and the ldd and // /lib/ld-musl-{cpu_arch}.so.1 will fail with exit status 1. - if str::from_utf8(&stdout).as_deref() == Ok(ALPINE_GCOMPAT) { + let stdout = str::from_utf8(&stdout); + debug!("{cmd}: stdout={stdout:?}"); + if stdout == Ok(ALPINE_GCOMPAT) { // Alpine's gcompat package will output ALPINE_GCOMPAT to stdout Some(Libc::Gnu) - } else if String::from_utf8_lossy(&stderr).contains("musl libc") { - // Alpine/s ldd and musl dynlib will output to stderr - Some(Libc::Musl) } else { - None + let stderr = String::from_utf8_lossy(&stderr); + debug!("{cmd}: stderr={stderr:?}"); + if stderr.contains("musl libc") { + // Alpine/s ldd and musl dynlib will output to stderr + Some(Libc::Musl) + } else { + None + } } } else if status.code() == Some(127) { // On Ubuntu 20.04 (glibc 2.31), the `--version` flag is not supported @@ -122,6 +135,10 @@ You are not meant to run this directly. status.success().then_some(Libc::Gnu) } else { + let code = status.code(); + let stdout = str::from_utf8(&stdout); + let stderr = str::from_utf8(&stderr); + debug!("{cmd}: code={code:?} stdout={stdout:?} stderr={stderr:?}"); None } } diff --git a/crates/detect-targets/src/main.rs b/crates/detect-targets/src/main.rs index fbaa187fe..1a0b03550 100644 --- a/crates/detect-targets/src/main.rs +++ b/crates/detect-targets/src/main.rs @@ -4,6 +4,11 @@ use detect_targets::detect_targets; use tokio::runtime; fn main() -> io::Result<()> { + tracing_subscriber::fmt::fmt() + .with_max_level(tracing::Level::TRACE) + .with_writer(std::io::stderr) + .init(); + let targets = runtime::Builder::new_current_thread() .enable_all() .build()? From bf909c4fa6054ac5f3b6df75df8fb1566210a63c Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sat, 23 Sep 2023 19:52:14 +1000 Subject: [PATCH 2/3] Add new feature `tracing` & `cli-tracing` to `detect-targets Signed-off-by: Jiahao XU --- crates/bin/src/logging.rs | 1 + crates/binstalk/Cargo.toml | 2 +- crates/detect-targets/Cargo.toml | 8 +++++-- crates/detect-targets/src/detect.rs | 3 +++ crates/detect-targets/src/detect/linux.rs | 27 ++++++++++++----------- crates/detect-targets/src/main.rs | 1 + 6 files changed, 26 insertions(+), 16 deletions(-) diff --git a/crates/bin/src/logging.rs b/crates/bin/src/logging.rs index 267fb8d68..320f266ab 100644 --- a/crates/bin/src/logging.rs +++ b/crates/bin/src/logging.rs @@ -202,6 +202,7 @@ pub fn logging(log_level: LevelFilter, json_output: bool) { "binstalk_registry", "cargo_binstall", "cargo_toml_workspace", + "detect_targets", "simple_git", ]); diff --git a/crates/binstalk/Cargo.toml b/crates/binstalk/Cargo.toml index affb61965..8f40bf4fa 100644 --- a/crates/binstalk/Cargo.toml +++ b/crates/binstalk/Cargo.toml @@ -18,7 +18,7 @@ binstalk-types = { version = "0.5.0", path = "../binstalk-types" } cargo-toml-workspace = { version = "1.0.0", path = "../cargo-toml-workspace" } command-group = { version = "2.1.0", features = ["with-tokio"] } compact_str = { version = "0.7.0", features = ["serde"] } -detect-targets = { version = "0.1.11", path = "../detect-targets" } +detect-targets = { version = "0.1.11", path = "../detect-targets", features = ["tracing"] } either = "1.8.1" itertools = "0.11.0" jobslot = { version = "0.2.11", features = ["tokio"] } diff --git a/crates/detect-targets/Cargo.toml b/crates/detect-targets/Cargo.toml index c7d7a766d..c3528b641 100644 --- a/crates/detect-targets/Cargo.toml +++ b/crates/detect-targets/Cargo.toml @@ -11,11 +11,15 @@ license = "Apache-2.0 OR MIT" [dependencies] tokio = { version = "1.28.2", features = ["rt", "process", "sync"], default-features = false } -tracing = "0.1.37" -tracing-subscriber = { version = "0.3.17", features = ["fmt"], default-features = false } +tracing = { version = "0.1.37", optional = true } +tracing-subscriber = { version = "0.3.17", features = ["fmt"], default-features = false, optional = true } cfg-if = "1.0.0" guess_host_triple = "0.1.3" +[features] +tracing = ["dep:tracing"] +cli-logging = ["tracing", "dep:tracing-subscriber"] + [target.'cfg(target_os = "windows")'.dependencies] windows-sys = { version = "0.48.0", features = ["Win32_System_Threading", "Win32_System_SystemInformation", "Win32_Foundation"] } windows-dll = { version = "0.4.1", features = ["windows"], default-features = false } diff --git a/crates/detect-targets/src/detect.rs b/crates/detect-targets/src/detect.rs index b24364617..15d8a1252 100644 --- a/crates/detect-targets/src/detect.rs +++ b/crates/detect-targets/src/detect.rs @@ -7,6 +7,7 @@ use std::{ use cfg_if::cfg_if; use tokio::process::Command; +#[cfg(feature = "tracing")] use tracing::debug; cfg_if! { @@ -34,9 +35,11 @@ cfg_if! { /// for more information. pub async fn detect_targets() -> Vec { let target = get_target_from_rustc().await; + #[cfg(feature = "tracing")] debug!("get_target_from_rustc()={target:?}"); let target = target.unwrap_or_else(|| { let target = guess_host_triple::guess_host_triple(); + #[cfg(feature = "tracing")] debug!("guess_host_triple::guess_host_triple()={target:?}"); target.unwrap_or(crate::TARGET).to_string() }); diff --git a/crates/detect-targets/src/detect/linux.rs b/crates/detect-targets/src/detect/linux.rs index a47836c79..6fc3a5a1f 100644 --- a/crates/detect-targets/src/detect/linux.rs +++ b/crates/detect-targets/src/detect/linux.rs @@ -4,6 +4,7 @@ use std::{ }; use tokio::{process::Command, task}; +#[cfg(feature = "tracing")] use tracing::debug; pub(super) async fn detect_targets(target: String) -> Vec { @@ -87,12 +88,19 @@ async fn get_ld_flavor(cmd: &str) -> Option { .await { Ok(output) => output, - Err(err) => { - debug!("{cmd}: err={err:?}"); + Err(_err) => { + #[cfg(feature = "tracing")] + debug!("Running `{cmd} --version`: err={_err:?}"); return None; } }; + let stdout = String::from_utf8_lossy(&stdout); + let stderr = String::from_utf8_lossy(&stderr); + + #[cfg(feature = "tracing")] + debug!("`{cmd} --version`: status={status}, stdout='{stdout}', stderr='{stderr}'"); + const ALPINE_GCOMPAT: &str = r#"This is the gcompat ELF interpreter stub. You are not meant to run this directly. "#; @@ -100,20 +108,14 @@ You are not meant to run this directly. if status.success() { // Executing glibc ldd or /lib/ld-linux-{cpu_arch}.so.1 will always // succeeds. - let stdout = String::from_utf8_lossy(&stdout); - debug!("{cmd}: stdout={stdout}"); stdout.contains("GLIBC").then_some(Libc::Gnu) } else if status.code() == Some(1) { // On Alpine, executing both the gcompat glibc and the ldd and // /lib/ld-musl-{cpu_arch}.so.1 will fail with exit status 1. - let stdout = str::from_utf8(&stdout); - debug!("{cmd}: stdout={stdout:?}"); - if stdout == Ok(ALPINE_GCOMPAT) { + if stdout == ALPINE_GCOMPAT { // Alpine's gcompat package will output ALPINE_GCOMPAT to stdout Some(Libc::Gnu) } else { - let stderr = String::from_utf8_lossy(&stderr); - debug!("{cmd}: stderr={stderr:?}"); if stderr.contains("musl libc") { // Alpine/s ldd and musl dynlib will output to stderr Some(Libc::Musl) @@ -133,12 +135,11 @@ You are not meant to run this directly. .await .ok()?; + #[cfg(feature = "tracing")] + debug!("`{cmd} --version`: status={status}"); + status.success().then_some(Libc::Gnu) } else { - let code = status.code(); - let stdout = str::from_utf8(&stdout); - let stderr = str::from_utf8(&stderr); - debug!("{cmd}: code={code:?} stdout={stdout:?} stderr={stderr:?}"); None } } diff --git a/crates/detect-targets/src/main.rs b/crates/detect-targets/src/main.rs index 1a0b03550..2623c077e 100644 --- a/crates/detect-targets/src/main.rs +++ b/crates/detect-targets/src/main.rs @@ -4,6 +4,7 @@ use detect_targets::detect_targets; use tokio::runtime; fn main() -> io::Result<()> { + #[cfg(feature = "cli-logging")] tracing_subscriber::fmt::fmt() .with_max_level(tracing::Level::TRACE) .with_writer(std::io::stderr) From b554019e93b0f7e3f1d4db1853bcc0d6fa456f91 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sat, 23 Sep 2023 20:24:38 +1000 Subject: [PATCH 3/3] Fix clippy lints Signed-off-by: Jiahao XU --- crates/detect-targets/src/detect/linux.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/detect-targets/src/detect/linux.rs b/crates/detect-targets/src/detect/linux.rs index 6fc3a5a1f..bba7fe60d 100644 --- a/crates/detect-targets/src/detect/linux.rs +++ b/crates/detect-targets/src/detect/linux.rs @@ -115,13 +115,11 @@ You are not meant to run this directly. if stdout == ALPINE_GCOMPAT { // Alpine's gcompat package will output ALPINE_GCOMPAT to stdout Some(Libc::Gnu) + } else if stderr.contains("musl libc") { + // Alpine/s ldd and musl dynlib will output to stderr + Some(Libc::Musl) } else { - if stderr.contains("musl libc") { - // Alpine/s ldd and musl dynlib will output to stderr - Some(Libc::Musl) - } else { - None - } + None } } else if status.code() == Some(127) { // On Ubuntu 20.04 (glibc 2.31), the `--version` flag is not supported