diff --git a/src/run/runner/helpers/run_with_sudo.rs b/src/run/runner/helpers/run_with_sudo.rs index c497edb7..01704803 100644 --- a/src/run/runner/helpers/run_with_sudo.rs +++ b/src/run/runner/helpers/run_with_sudo.rs @@ -4,6 +4,16 @@ use std::{ process::{Command, Stdio}, }; +fn is_sudo_available() -> bool { + Command::new("sudo") + .arg("--version") + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .map(|status| status.success()) + .unwrap_or(false) +} + /// Validate sudo access, prompting the user for their password if necessary fn validate_sudo_access() -> Result<()> { let needs_password = IsTerminal::is_terminal(&std::io::stdout()) @@ -37,11 +47,12 @@ fn validate_sudo_access() -> Result<()> { Ok(()) })?; } + debug!("Sudo access validated"); Ok(()) } /// Creates the base sudo command after validating sudo access -pub fn validated_sudo_command() -> Result { +fn validated_sudo_command() -> Result { validate_sudo_access()?; let mut cmd = Command::new("sudo"); // Password prompt should not appear here since it has already been validated @@ -49,20 +60,35 @@ pub fn validated_sudo_command() -> Result { Ok(cmd) } -/// Run a command with sudo after validating sudo access +/// Build a command wrapped with sudo if possible +pub fn build_command_with_sudo(command_args: &[&str]) -> Result { + let command_str = command_args.join(" "); + if is_sudo_available() { + debug!("Sudo is required for command: {command_str}"); + let mut c = validated_sudo_command()?; + c.args(command_args); + Ok(c) + } else { + debug!("Running command without sudo: {command_str}"); + let mut c = Command::new(command_args[0]); + c.args(&command_args[1..]); + Ok(c) + } +} + +/// Run a command with sudo after validating sudo access (if possible) pub fn run_with_sudo(command_args: &[&str]) -> Result<()> { + let mut cmd = build_command_with_sudo(command_args)?; let command_str = command_args.join(" "); - debug!("Running command with sudo: {command_str}"); - let output = validated_sudo_command()? - .args(command_args) + let output = cmd .stdout(Stdio::piped()) .output() - .map_err(|_| anyhow!("Failed to execute command with sudo: {command_str}"))?; + .map_err(|_| anyhow!("Failed to execute: {command_str}"))?; if !output.status.success() { info!("stdout: {}", String::from_utf8_lossy(&output.stdout)); error!("stderr: {}", String::from_utf8_lossy(&output.stderr)); - bail!("Failed to execute command with sudo: {command_str}"); + bail!("Failed to execute command: {command_str}"); } Ok(()) diff --git a/src/run/runner/wall_time/executor.rs b/src/run/runner/wall_time/executor.rs index 1f424cbd..e0be5056 100644 --- a/src/run/runner/wall_time/executor.rs +++ b/src/run/runner/wall_time/executor.rs @@ -8,7 +8,7 @@ use crate::run::runner::helpers::get_bench_command::get_bench_command; use crate::run::runner::helpers::introspected_golang; use crate::run::runner::helpers::introspected_nodejs; use crate::run::runner::helpers::run_command_with_log_pipe::run_command_with_log_pipe; -use crate::run::runner::helpers::run_with_sudo::validated_sudo_command; +use crate::run::runner::helpers::run_with_sudo::build_command_with_sudo; use crate::run::runner::{ExecutorName, RunData}; use crate::run::{check_system::SystemInfo, config::Config}; use async_trait::async_trait; @@ -100,7 +100,7 @@ impl WallTimeExecutor { fn walltime_bench_cmd( config: &Config, run_data: &RunData, - ) -> Result<(NamedTempFile, NamedTempFile, String)> { + ) -> Result<(NamedTempFile, NamedTempFile, Vec)> { let bench_cmd = get_bench_command(config)?; let system_env = get_exported_system_env()?; @@ -150,11 +150,24 @@ impl WallTimeExecutor { // - We have to pass the environment variables because `--scope` only inherits the system and not the user environment variables. let uid = nix::unistd::Uid::current().as_raw(); let gid = nix::unistd::Gid::current().as_raw(); - let cmd = format!( - "systemd-run {quiet_flag} --scope --slice=codspeed.slice --same-dir --uid={uid} --gid={gid} -- bash {}", - script_file.path().display() - ); - Ok((env_file, script_file, cmd)) + let script_path = script_file.path().to_string_lossy().to_string(); + let cmd = [ + "systemd-run", + quiet_flag, + "--scope", + "--slice=codspeed.slice", + "--same-dir", + &format!("--uid={uid}"), + &format!("--gid={gid}"), + "--", + "bash", + &script_path, + ]; + Ok(( + env_file, + script_file, + cmd.into_iter().map(|s| s.to_string()).collect(), + )) } } @@ -179,25 +192,24 @@ impl Executor for WallTimeExecutor { run_data: &RunData, _mongo_tracer: &Option, ) -> Result<()> { - let mut cmd = validated_sudo_command()?; - - if let Some(cwd) = &config.working_directory { - let abs_cwd = canonicalize(cwd)?; - cmd.current_dir(abs_cwd); - } - let status = { let _guard = HookScriptsGuard::setup(); + // Keep the temporary files alive for the duration of the command execution let (_env_file, _script_file, bench_cmd) = Self::walltime_bench_cmd(config, run_data)?; if let Some(perf) = &self.perf && config.enable_perf { - perf.run(cmd, &bench_cmd, config).await + perf.run(bench_cmd, config).await } else { - cmd.args(["sh", "-c", &bench_cmd]); + let mut cmd = build_command_with_sudo( + &bench_cmd.iter().map(|s| s.as_str()).collect::>(), + )?; debug!("cmd: {cmd:?}"); - + if let Some(cwd) = &config.working_directory { + let abs_cwd = canonicalize(cwd)?; + cmd.current_dir(abs_cwd); + } run_command_with_log_pipe(cmd).await } }; diff --git a/src/run/runner/wall_time/perf/mod.rs b/src/run/runner/wall_time/perf/mod.rs index ad76f06d..624d4f9c 100644 --- a/src/run/runner/wall_time/perf/mod.rs +++ b/src/run/runner/wall_time/perf/mod.rs @@ -5,6 +5,7 @@ use crate::run::UnwindingMode; use crate::run::config::Config; use crate::run::runner::helpers::env::is_codspeed_debug_enabled; use crate::run::runner::helpers::run_command_with_log_pipe::run_command_with_log_pipe_and_callback; +use crate::run::runner::helpers::run_with_sudo::build_command_with_sudo; use crate::run::runner::helpers::run_with_sudo::run_with_sudo; use crate::run::runner::valgrind::helpers::ignored_objects_path::get_objects_path_to_ignore; use crate::run::runner::valgrind::helpers::perf_maps::harvest_perf_maps_for_pids; @@ -24,8 +25,8 @@ use runner_shared::fifo::MarkerType; use runner_shared::metadata::PerfMetadata; use runner_shared::unwind_data::UnwindData; use std::collections::HashSet; +use std::fs::canonicalize; use std::path::{Path, PathBuf}; -use std::process::Command; use std::time::Duration; use std::{cell::OnceCell, collections::HashMap, process::ExitStatus}; @@ -84,12 +85,7 @@ impl PerfRunner { } } - pub async fn run( - &self, - mut cmd: Command, - bench_cmd: &str, - config: &Config, - ) -> anyhow::Result { + pub async fn run(&self, bench_cmd: Vec, config: &Config) -> anyhow::Result { let perf_fifo = PerfFifo::new()?; let runner_fifo = RunnerFifo::new()?; @@ -111,11 +107,11 @@ impl PerfRunner { (UnwindingMode::Dwarf, None) }; - let cg_mode = match cg_mode { - UnwindingMode::FramePointer => "fp", - UnwindingMode::Dwarf => &format!("dwarf,{}", stack_size.unwrap_or(8192)), + let cg_mode_str = match cg_mode { + UnwindingMode::FramePointer => "fp".to_string(), + UnwindingMode::Dwarf => format!("dwarf,{}", stack_size.unwrap_or(8192)), }; - debug!("Using call graph mode: {cg_mode:?}"); + debug!("Using call graph mode: {cg_mode_str:?}"); let quiet_flag = if !is_codspeed_debug_enabled() { "--quiet" @@ -126,6 +122,7 @@ impl PerfRunner { let working_perf_executable = get_working_perf_executable().context("Failed to find a working perf executable")?; + let bench_str = bench_cmd.join(" "); let perf_args = [ working_perf_executable.as_str(), "record", @@ -138,7 +135,7 @@ impl PerfRunner { "--delay=-1", "-g", "--user-callchains", - &format!("--call-graph={cg_mode}"), + &format!("--call-graph={cg_mode_str}"), &format!( "--control=fifo:{},{}", perf_fifo.ctl_fifo_path.to_string_lossy(), @@ -146,12 +143,15 @@ impl PerfRunner { ), &format!("--output={PERF_DATA_PATH}"), "--", - bench_cmd, + &bench_str, ]; - - cmd.args(["sh", "-c", &perf_args.join(" ")]); - debug!("cmd: {cmd:?}"); - + let perf_cmd = perf_args.join(" "); + debug!("raw perf cmd: {perf_cmd:?}"); + let mut cmd = build_command_with_sudo(&["sh", "-c", &perf_cmd])?; + if let Some(cwd) = &config.working_directory { + let abs_cwd = canonicalize(cwd)?; + cmd.current_dir(abs_cwd); + } let on_process_started = async |_| -> anyhow::Result<()> { let data = Self::handle_fifo(runner_fifo, perf_fifo).await?; let _ = self.benchmark_data.set(data);