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 e9fe30d81..bf4e683ac 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 0fd7df06f..c6a7c5d99 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/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 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/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/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; 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())), } }