From 9eeb123a2b9b1c657a12c48ab5237b8d5a3fce43 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Mon, 27 Jul 2026 14:18:15 +0200 Subject: [PATCH 1/3] feat(samply-codspeed): bump submodule to forward signal exit code --- crates/samply-codspeed | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/samply-codspeed b/crates/samply-codspeed index bb8afef3f..4d425b659 160000 --- a/crates/samply-codspeed +++ b/crates/samply-codspeed @@ -1 +1 @@ -Subproject commit bb8afef3faf24e3d002751bdea011fc947d7b07a +Subproject commit 4d425b65939cd93afa59f86f8ab2d34a55b32945 From 89c2db0687f348789e8119df754874a9c0e4d027 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Mon, 27 Jul 2026 12:38:14 +0200 Subject: [PATCH 2/3] chore: fix self-exec failures in tests and ungate executor tests Tests no longer require sudo, or at least they dont after the kernel parameters are set, and the memory executor tests now run in a reasonable time thanks to the attach on demand, we dont need to gate anymore. Also add a way for the test executable to find the actual codspeed binary in order to have the self exec into samply work properly. It used to re-exec the test executable, which is not the behavior we want here. --- .github/workflows/ci.yml | 3 - Cargo.lock | 36 ++++++++---- Cargo.toml | 1 + build.rs | 6 -- src/cli/mod.rs | 16 +++++- src/executor/tests.rs | 121 ++++++++++++++++++++++++++++++--------- 6 files changed, 132 insertions(+), 51 deletions(-) delete mode 100644 build.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d6935beba..bbbb05a93 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,9 +41,6 @@ jobs: - name: Install memtrack run: | cargo install --path crates/memtrack --locked - - name: Install exec-harness - run: | - cargo install --path crates/exec-harness --locked - name: Grant memtrack file capabilities run: cargo r -- setup --mode memory diff --git a/Cargo.lock b/Cargo.lock index e107a92fc..c6f1a659f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -125,7 +125,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -136,7 +136,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -707,6 +707,7 @@ dependencies = [ "console", "crc32fast", "debugid", + "escargot", "exec-harness", "futures", "gimli", @@ -1021,7 +1022,7 @@ dependencies = [ "libc", "option-ext", "redox_users 0.5.2", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -1159,7 +1160,18 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.60.2", + "windows-sys 0.61.2", +] + +[[package]] +name = "escargot" +version = "0.5.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "11c3aea32bc97b500c9ca6a72b768a26e558264303d101d3409cf6d57a9ed0cf" +dependencies = [ + "log", + "serde", + "serde_json", ] [[package]] @@ -2560,7 +2572,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -3675,7 +3687,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -3733,7 +3745,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -4240,7 +4252,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a766e1110788c36f4fa1c2b71b387a7815aa65f88ce0229841826633d93723e" dependencies = [ "libc", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -4466,10 +4478,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.3.4", + "getrandom 0.4.2", "once_cell", "rustix 1.1.4", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -4488,7 +4500,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "230a1b821ccbd75b185820a1f1ff7b14d21da1e442e22c0863ea5f08771a8874" dependencies = [ "rustix 1.1.4", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -5325,7 +5337,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 89fec29b3..e3b198e36 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -88,6 +88,7 @@ shell-quote = "0.7.2" assert_cmd = "2.2" predicates = "3.1.4" strum = { version = "0.28.0", features = ["derive"] } +escargot = "0.5.15" [workspace] members = [ diff --git a/build.rs b/build.rs deleted file mode 100644 index 2750b7e60..000000000 --- a/build.rs +++ /dev/null @@ -1,6 +0,0 @@ -fn main() { - // Force a rebuild of the test target to be able to run the full test suite locally just by - // setting GITHUB_ACTIONS=1 in the environment. - // This is because `test_with` is evaluated at build time - println!("cargo::rerun-if-env-changed=GITHUB_ACTIONS"); -} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 15d31b25e..dae96ca58 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -112,13 +112,23 @@ pub(crate) enum InternalCommands { Samply(samply::SamplyArgs), } +/// Overrides the executable used to re-invoke internal subcommands. +/// +/// [`std::env::current_exe`] is not always a binary that can dispatch them: it +/// resolves to the host executable when this crate is linked into one, and to +/// a wrapper when the CLI is invoked through a launcher script. +pub(crate) const SELF_EXE_ENV_VAR: &str = "CODSPEED_SELF_EXE"; + impl InternalCommands { /// Build a [`CommandBuilder`] that re-execs the current binary into this /// internal subcommand. Each variant owns its own arg layout. pub fn get_command_builder(&self) -> Result { - let current_exe = std::env::current_exe() - .context("failed to resolve current executable for internal subcommand")?; - let mut builder = CommandBuilder::new(current_exe); + let self_exe = match std::env::var_os(SELF_EXE_ENV_VAR) { + Some(path) => PathBuf::from(path), + None => std::env::current_exe() + .context("failed to resolve current executable for internal subcommand")?, + }; + let mut builder = CommandBuilder::new(self_exe); match self { InternalCommands::Samply(args) => { builder.arg("samply"); diff --git a/src/executor/tests.rs b/src/executor/tests.rs index 77003df00..d562c6c90 100644 --- a/src/executor/tests.rs +++ b/src/executor/tests.rs @@ -1,8 +1,5 @@ -// Shared test helpers. On non-linux platforms, the only consumer is the `walltime` mod, which is -// gated behind `GITHUB_ACTIONS`. Apply the same gate here so dead-code lints don't fire on macOS -// without the env var. On linux, valgrind always uses these helpers, so no gate is needed. -#[cfg_attr(not(target_os = "linux"), test_with::env(GITHUB_ACTIONS))] mod helpers { + pub(crate) use crate::cli::SELF_EXE_ENV_VAR; pub use crate::executor::{ExecutionContext, Executor, ExecutorConfig}; pub use crate::system::SystemInfo; pub use rstest_reuse::{self, *}; @@ -120,6 +117,55 @@ fi #[case(ENV_TESTS[7])] pub fn env_test_cases(#[case] env_case: (&str, &str)) {} + /// Builds a workspace binary and returns its path. + /// + /// `CARGO_BIN_EXE_*` is only set for integration tests and benches, and the + /// build directory layout is not a stable interface, so the path is obtained + /// by invoking cargo. + async fn workspace_binary_path(package: &'static str, bin: &'static str) -> String { + tokio::task::spawn_blocking(move || { + let path = escargot::CargoBuild::new() + .package(package) + .bin(bin) + .current_target() + .run() + .unwrap_or_else(|e| panic!("failed to build the {bin} binary: {e}")) + .path() + .to_path_buf(); + path.into_os_string() + .into_string() + .unwrap_or_else(|_| panic!("{bin} binary path is not valid UTF-8")) + }) + .await + .unwrap_or_else(|e| panic!("{bin} binary build task panicked: {e}")) + } + + /// Path to the `codspeed` binary, built on first use. + /// + /// Code under test re-execs the running executable to reach internal + /// subcommands. That executable is this test binary, whose harness rejects + /// their arguments, so the tests point [`SELF_EXE_ENV_VAR`] at the real + /// binary instead. + pub async fn codspeed_binary_path() -> &'static str { + static BINARY: OnceCell = OnceCell::const_new(); + + BINARY + .get_or_init(|| workspace_binary_path("codspeed-runner", "codspeed")) + .await + } + + /// Path to the `exec-harness` binary, built on first use. + /// + /// Production runs install a pinned release and invoke it by name, which + /// would make the tests depend on what is installed on the machine. + pub async fn exec_harness_binary_path() -> &'static str { + static BINARY: OnceCell = OnceCell::const_new(); + + BINARY + .get_or_init(|| workspace_binary_path("exec-harness", "exec-harness")) + .await + } + pub async fn create_test_setup(config: ExecutorConfig) -> (ExecutionContext, TempDir) { let temp_dir = TempDir::new().unwrap(); @@ -205,7 +251,6 @@ mod valgrind { } } -#[test_with::env(GITHUB_ACTIONS)] mod walltime { use super::helpers::*; use crate::executor::wall_time::executor::WallTimeExecutor; @@ -250,11 +295,15 @@ mod walltime { let (_permit, mut executor) = get_walltime_executor().await; let config = walltime_config(cmd, enable_profiler); + let self_exe = codspeed_binary_path().await; // Unset GITHUB_ACTIONS to force LocalProvider which supports repository_override - temp_env::async_with_vars(&[("GITHUB_ACTIONS", None::<&str>)], async { - let (execution_context, _temp_dir) = create_test_setup(config).await; - executor.run(&execution_context, &None).await.unwrap(); - }) + temp_env::async_with_vars( + &[("GITHUB_ACTIONS", None), (SELF_EXE_ENV_VAR, Some(self_exe))], + async { + let (execution_context, _temp_dir) = create_test_setup(config).await; + executor.run(&execution_context, &None).await.unwrap(); + }, + ) .await; } @@ -268,8 +317,13 @@ mod walltime { let (_permit, mut executor) = get_walltime_executor().await; let (env_var, env_value) = env_case; + let self_exe = codspeed_binary_path().await; temp_env::async_with_vars( - &[(env_var, Some(env_value)), ("GITHUB_ACTIONS", None)], + &[ + (env_var, Some(env_value)), + ("GITHUB_ACTIONS", None), + (SELF_EXE_ENV_VAR, Some(self_exe)), + ], async { let cmd = env_var_validation_script(env_var, env_value); let config = walltime_config(&cmd, enable_profiler); @@ -304,11 +358,15 @@ fi ); std::fs::create_dir_all(config.working_directory.as_ref().unwrap()).unwrap(); + let self_exe = codspeed_binary_path().await; // Unset GITHUB_ACTIONS to force LocalProvider which supports repository_override - temp_env::async_with_vars(&[("GITHUB_ACTIONS", None::<&str>)], async { - let (execution_context, _temp_dir) = create_test_setup(config).await; - executor.run(&execution_context, &None).await.unwrap(); - }) + temp_env::async_with_vars( + &[("GITHUB_ACTIONS", None), (SELF_EXE_ENV_VAR, Some(self_exe))], + async { + let (execution_context, _temp_dir) = create_test_setup(config).await; + executor.run(&execution_context, &None).await.unwrap(); + }, + ) .await; } @@ -319,12 +377,16 @@ fi let (_permit, mut executor) = get_walltime_executor().await; let config = walltime_config("exit 1", enable_profiler); + let self_exe = codspeed_binary_path().await; // Unset GITHUB_ACTIONS to force LocalProvider which supports repository_override - temp_env::async_with_vars(&[("GITHUB_ACTIONS", None::<&str>)], async { - let (execution_context, _temp_dir) = create_test_setup(config).await; - let result = executor.run(&execution_context, &None).await; - assert!(result.is_err(), "Command should fail"); - }) + temp_env::async_with_vars( + &[("GITHUB_ACTIONS", None), (SELF_EXE_ENV_VAR, Some(self_exe))], + async { + let (execution_context, _temp_dir) = create_test_setup(config).await; + let result = executor.run(&execution_context, &None).await; + assert!(result.is_err(), "Command should fail"); + }, + ) .await; } // @@ -339,11 +401,12 @@ fi } fn wrap_with_exec_harness( + exec_harness: &str, walltime_args: &exec_harness::walltime::WalltimeExecutionArgs, command: &[String], ) -> String { shell_words::join( - std::iter::once(crate::executor::orchestrator::EXEC_HARNESS_COMMAND) + std::iter::once(exec_harness) .chain(walltime_args.to_cli_args().iter().map(|s| s.as_str())) .chain(command.iter().map(|s| s.as_str())), ) @@ -367,20 +430,24 @@ fi }; let cmd = cmd.split(" ").map(|s| s.to_owned()).collect::>(); - let wrapped_command = wrap_with_exec_harness(&walltime_args, &cmd); + let wrapped_command = + wrap_with_exec_harness(exec_harness_binary_path().await, &walltime_args, &cmd); + let self_exe = codspeed_binary_path().await; // Unset GITHUB_ACTIONS to force LocalProvider which supports repository_override - temp_env::async_with_vars(&[("GITHUB_ACTIONS", None::<&str>)], async { - let config = walltime_config(&wrapped_command, true); - let (execution_context, _temp_dir) = create_test_setup(config).await; - executor.run(&execution_context, &None).await.unwrap(); - }) + temp_env::async_with_vars( + &[("GITHUB_ACTIONS", None), (SELF_EXE_ENV_VAR, Some(self_exe))], + async { + let config = walltime_config(&wrapped_command, true); + let (execution_context, _temp_dir) = create_test_setup(config).await; + executor.run(&execution_context, &None).await.unwrap(); + }, + ) .await; } } #[cfg(target_os = "linux")] -#[test_with::env(GITHUB_ACTIONS)] mod memory { use super::helpers::*; use crate::executor::memory::executor::MemoryExecutor; From c6f4fe85884220e06462f82c4569cacf243a579f Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Mon, 27 Jul 2026 11:02:22 +0200 Subject: [PATCH 3/3] feat: make samply profiler default for all OSes --- src/executor/config.rs | 4 ++-- src/executor/wall_time/executor.rs | 10 +--------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/executor/config.rs b/src/executor/config.rs index cb7e014b9..0dc40678a 100644 --- a/src/executor/config.rs +++ b/src/executor/config.rs @@ -45,9 +45,9 @@ pub enum SimulationTool { /// The profiler to use for walltime mode. #[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] pub enum WalltimeProfiler { - /// Use perf to collect profiling data (Linux). + /// Use perf to collect profiling data. Perf, - /// Use samply to collect profiling data (macOS). + /// Use samply to collect profiling data. Samply, } diff --git a/src/executor/wall_time/executor.rs b/src/executor/wall_time/executor.rs index fc956e6d0..a15a96008 100644 --- a/src/executor/wall_time/executor.rs +++ b/src/executor/wall_time/executor.rs @@ -93,15 +93,7 @@ fn select_profiler(profiler_override: Option) -> Option Some(Box::new(PerfProfiler::new())), Some(WalltimeProfiler::Samply) => Some(Box::new(SamplyProfiler::new())), - None => { - if cfg!(target_os = "linux") { - Some(Box::new(PerfProfiler::new())) - } else if cfg!(target_os = "macos") { - Some(Box::new(SamplyProfiler::new())) - } else { - None - } - } + None => Some(Box::new(SamplyProfiler::new())), } }