diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 000000000..9f36b44dc --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,45 @@ +# What a musl build of the bundled `memtrack` needs. Refs COD-3440. +# +# `libbpf-sys` vendors elfutils, whose `configure` unconditionally looks for +# `argp`, `obstack` and `fts`. musl ships none of them, so the checks fail and +# the build stops before it reaches libelf — even though a libelf-only build +# never calls into any of them. Pre-seeding autoconf's cache skips the three +# checks. "none required" is the answer a glibc host reaches on its own, so +# these are unconditional rather than per-target; on gnu they only save three +# `configure` probes. +# +# Applies to everything below: cargo does *not* override a variable already set +# in the environment unless the entry carries `force = true`. A shell exporting +# `CFLAGS` or `CPATH` therefore loses these values, and the musl build fails on +# a missing or . They are left unforced so a caller who +# sets them deliberately keeps them; no CI job does. +[env] +ac_cv_search_argp_parse = "none required" +ac_cv_search__obstack_free = "none required" +ac_cv_search_fts_close = "none required" + +# Where the `argp.h` stub lives. `CPATH` rather than `CFLAGS -I`, because +# `relative = true` can only make a *bare* path absolute and a `CFLAGS` value +# has nowhere to put the `-I`. It resolves against the project root — the +# directory holding `.cargo/`, not `.cargo/` itself. +# +# Not target-scoped, so the stub is on the gnu build's include path too; the +# header defers to the real whenever it detects glibc. +CPATH = { value = "crates/memtrack/musl", relative = true } + +# libbpf includes and . Debian's musl-gcc runs with +# -nostdinc and only sees /usr/include/-linux-musl, so the kernel UAPI +# headers from linux-libc-dev have to be added back. `-idirafter` puts them last, +# behind musl's own, which is what keeps a glibc build unaffected. +# +# Both Debian multiarch triplets are listed because `[env]` cannot branch on the +# host architecture. A `-idirafter` naming a directory that does not exist is +# ignored silently, so the wrong one does nothing — as do both off Debian. +CFLAGS = "-idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include/aarch64-linux-gnu -idirafter /usr/include" + +# rustc links with `-nodefaultlibs`, so gcc does not pull in libgcc. On aarch64, +# libbpf's C code needs the outline-atomic helpers (`__aarch64_ldadd4_sync` and +# friends) that live there, and the link fails without it. x86_64 has no such +# helpers and needs nothing. +[target.aarch64-unknown-linux-musl] +rustflags = ["-C", "link-arg=-lgcc"] diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f8fe75eac..1226106ef 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,6 +19,9 @@ jobs: - uses: ./.github/actions/install-rust with: components: rustfmt, clippy + # Building the runner builds memtrack's vendored libbpf-sys with it. + - uses: ./.github/actions/install-bpf-deps + if: matrix.os == 'ubuntu-latest' - uses: j178/prek-action@bdca6f102f98e2b4c7029491a53dfd366469e33d # v2.0.4 with: extra-args: --all-files @@ -36,15 +39,10 @@ jobs: - uses: ./.github/actions/install-rust - # Install memtrack for the memory integration tests - uses: ./.github/actions/install-bpf-deps - - name: Install memtrack - run: | - cargo install --path crates/memtrack --locked - - - name: Grant memtrack file capabilities - run: cargo r -- setup --mode memory + # No `setup --mode memory` here: the memory tests grant the capabilities + # themselves, pointed at the binary they actually re-exec. - run: cargo test --all --exclude memtrack --exclude exec-harness exec-harness-tests: @@ -64,6 +62,7 @@ jobs: with: submodules: true - uses: ./.github/actions/install-rust + - uses: ./.github/actions/install-bpf-deps - name: Run tests run: cargo run -- exec -m simulation,walltime,memory --warmup-time 0s --max-rounds 5 -- sleep 1 @@ -74,10 +73,6 @@ jobs: with: submodules: true - uses: ./.github/actions/install-rust - - name: Install exec-harness - run: | - cargo install --path crates/exec-harness --locked - - name: Run tests env: # Profiling system commands (e.g. `ls`) with samply is not yet supported on MacOS @@ -153,6 +148,56 @@ jobs: mode: ${{ matrix.mode }} run: cargo codspeed run -p runner-shared + # The released Linux artifacts are musl, and nothing else here builds them, so + # a break in the argp stub, the kernel-header paths or the aarch64 `-lgcc` + # link flag would otherwise surface for the first time in a release. + musl-build: + runs-on: ${{ matrix.runner }} + strategy: + fail-fast: false + matrix: + include: + - runner: ubuntu-latest + target: x86_64-unknown-linux-musl + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + submodules: true + - uses: ./.github/actions/install-rust + with: + cache-key: ${{ matrix.target }} + - uses: ./.github/actions/install-bpf-deps + - name: Install the musl toolchain + run: | + sudo apt-get install -y musl-tools linux-libc-dev + rustup target add "${{ matrix.target }}" + + # No environment variables: the whole recipe lives in `.cargo/config.toml`, + # and a plain build is what proves it still stands on its own. + - name: Build + run: cargo build --bin codspeed --target "${{ matrix.target }}" + + - name: Assert the artifact is static and carries both subcommands + run: | + BIN=target/${{ matrix.target }}/debug/codspeed + file "$BIN" + # Asserted through readelf rather than a `file` string: rustc emits a + # static-PIE for x86_64 musl, which `file` spells differently from the + # aarch64 one. What matters is that nothing is loaded at runtime. + if readelf -d "$BIN" 2>/dev/null | grep -qE 'NEEDED|RPATH|RUNPATH'; then + echo "the musl binary has a dynamic dependency" + exit 1 + fi + if readelf -lW "$BIN" 2>/dev/null | grep -q 'INTERP'; then + echo "the musl binary requests a dynamic loader" + exit 1 + fi + # These answer only if the two CLIs really are linked in. + "$BIN" exec-harness --version + "$BIN" memtrack --version + check: runs-on: ubuntu-latest if: always() @@ -163,6 +208,7 @@ jobs: - basic-run-test - macos-basic-run-test - bpf-tests + - musl-build - benchmarks steps: - uses: re-actors/alls-green@release/v1 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c47e5ace9..9d04a714a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,50 +10,28 @@ prek install ## Release Process -This repository is a Cargo workspace containing multiple crates. The release process differs depending on which crate you're releasing. +This repository is a Cargo workspace containing multiple crates, but only one of them is released: the main runner. Everything else is linked into its binary. ### Workspace Structure - **`codspeed-runner`**: The main CLI binary (`codspeed`) -- **`memtrack`**: Memory tracking binary (`codspeed-memtrack`) -- **`exec-harness`**: Execution harness binary +- **`memtrack`**: Memory tracker, built into `codspeed` and reached as `codspeed memtrack` +- **`exec-harness`**: Execution harness, built into `codspeed` and reached as `codspeed exec-harness` - **`runner-shared`**: Shared library used by other crates -### Releasing Support Crates (memtrack, exec-harness, runner-shared) +`memtrack` and `exec-harness` are **not released on their own**. They are linked into the +`codspeed` binary and invoked as hidden subcommands, so one tag produces one artifact set and +there is no version for the runner to be out of step with. Their `[[bin]]` targets remain for +development and for the tests, which build them to exercise the standalone path. -For any crate other than the main runner: - -```bash -cargo release -p --execute -``` - -Where `` is one of: `alpha`, `beta`, `patch`, `minor`, or `major`. - -**Examples:** - -```bash -# Release a new patch version of memtrack -cargo release -p memtrack --execute patch - -# Release a beta version of exec-harness -cargo release -p exec-harness --execute beta -``` - -#### Post-Release: Update Version References - -After releasing `memtrack` or `exec-harness`, you **must** update the version references in the runner code: - -1. **For memtrack**: Update the `MEMTRACK_INSTALLER` pin record in `src/binary_pins.rs` (see [Pinned binary hashes](#pinned-binary-hashes) below). - -2. **For exec-harness**: Update the `EXEC_HARNESS_INSTALLER` pin record in `src/binary_pins.rs`. - -These constants are used by the runner to download and install the correct versions of the binaries from GitHub releases. +Both still keep their own `version` in `Cargo.toml` — that is what +`codspeed exec-harness --version` reports — but bumping it is a plain edit, not a release. ### Pinned binary hashes Every binary the runner downloads at install time is SHA-256-pinned. The pins live in two places: -- **`src/binary_pins.rs`** — the patched valgrind `.deb`, the memtrack installer, the exec-harness installer, and the mongo-tracer installer. Each artifact keeps its version, URL template, and hash together in a pin record. +- **`src/binary_pins.rs`** — the patched valgrind `.deb` and the mongo-tracer installer. Each artifact keeps its version, URL template, and hash together in a pin record. - **`src/executor/helpers/introspected_golang/go.sh`** — the go-runner installer published by [CodSpeedHQ/codspeed-go](https://github.com/CodSpeedHQ/codspeed-go), one ` ` row per release in the `GO_RUNNER_INSTALLER_SHA256S` table. `DEFAULT_GO_RUNNER_VERSION` (just below the table) selects the row used by default. When you bump a pinned version (or add a new go-runner row), update the matching pin record / table row with the new version and its SHA-256. @@ -84,16 +62,12 @@ These tests also run in CI, but running them locally before opening the PR avoid ### Releasing the Main Runner -The main runner (`codspeed-runner`) should be released after ensuring all dependency versions are correct. +The main runner (`codspeed-runner`) is the only crate that is released. #### Pre-Release Check -**Verify binary version references**: Check that version constants in the runner code match the released versions: - -- `MEMTRACK_VERSION` in `src/binary_pins.rs` -- `EXEC_HARNESS_VERSION` in `src/binary_pins.rs` - -Also confirm the SHA-256 entries in the pin records in `src/binary_pins.rs` match the released artifacts. +Confirm the SHA-256 entries in the pin records in `src/binary_pins.rs` match the released +artifacts they point at. #### Release Command diff --git a/Cargo.lock b/Cargo.lock index 69c918e5c..87b3f0b37 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1179,13 +1179,11 @@ name = "exec-harness" version = "1.3.0" dependencies = [ "anyhow", - "cc", "clap", "env_logger", "humantime", "instrument-hooks-bindings", "log", - "object", "runner-shared", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 7709542c4..d88c8eea2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -74,7 +74,9 @@ samply = { path = "crates/samply-codspeed/samply" } [target.'cfg(target_os = "linux")'.dependencies] procfs = "0.18" caps = "0.5" -memtrack = { path = "crates/memtrack", default-features = false } +# Default features on purpose: `ebpf` carries the tracker itself, which the +# bundled `memtrack` subcommand needs, not just the IPC types. +memtrack = { path = "crates/memtrack" } ipc-channel = { workspace = true } [dev-dependencies] @@ -139,3 +141,18 @@ targets = ["aarch64-apple-darwin", "aarch64-unknown-linux-musl", "x86_64-unknown binaries.aarch64-apple-darwin = ["codspeed"] binaries.aarch64-unknown-linux-musl = ["codspeed"] binaries.x86_64-unknown-linux-musl = ["codspeed"] + +# Linking memtrack in pulls its vendored libbpf/elfutils build into this +# package, so releasing the CLI needs memtrack's build toolchain. +[package.metadata.dist.dependencies.apt] +build-essential = "*" +pkgconf = "*" +zlib1g-dev = "*" +libbpf-dev = "*" +musl-tools = "*" +linux-libc-dev = "*" + +# Required for the vendored feature +autopoint = "*" +bison = "*" +flex = "*" diff --git a/crates/exec-harness/Cargo.toml b/crates/exec-harness/Cargo.toml index f73c631c8..04a168ed3 100644 --- a/crates/exec-harness/Cargo.toml +++ b/crates/exec-harness/Cargo.toml @@ -20,10 +20,7 @@ serde = { workspace = true } humantime = "2.3" runner-shared = { path = "../runner-shared" } tempfile = { workspace = true } -object = { workspace = true } -[build-dependencies] -cc = "1" - -[package.metadata.dist] -targets = ["aarch64-unknown-linux-gnu", "x86_64-unknown-linux-gnu"] +# Deliberately no `[package.metadata.dist]`: exec-harness ships inside the +# `codspeed` binary and is not released on its own. The `[[bin]]` stays for +# development and for the tests, which exercise the standalone path. diff --git a/crates/exec-harness/build.rs b/crates/exec-harness/build.rs index bf65ef7e5..8988c463a 100644 --- a/crates/exec-harness/build.rs +++ b/crates/exec-harness/build.rs @@ -1,170 +1,18 @@ //! Build script for exec-harness //! -//! This script compiles the `libcodspeed_preload.so` shared library that is used -//! to inject instrumentation into child processes via LD_PRELOAD. -//! -//! The library is built using the `core.c` and headers from the `instrument-hooks-bindings` -//! crate's `instrument-hooks` directory. - -use std::env; -use std::path::PathBuf; +//! Exports the constants shared between the crate's modules as environment +//! variables, so `src/constants.rs` can read them through `env!()` and there is +//! a single source of truth for the integration identity reported to CodSpeed. -/// Shared constants for the preload library. -/// These are passed as C defines during compilation and exported as environment -/// variables for the Rust code to use via `env!()`. -struct PreloadConstants { - /// Environment variable name for the benchmark URI. - uri_env: &'static str, - /// Integration name reported to CodSpeed. - integration_name: &'static str, - /// Integration version reported to CodSpeed. - integration_version: &'static str, - /// Filename for the preload shared library. - preload_lib_filename: &'static str, -} +/// Integration name reported to CodSpeed. +const INTEGRATION_NAME: &str = "exec-harness"; fn main() { - println!("cargo:rerun-if-changed=preload/codspeed_preload.c"); - println!("cargo:rerun-if-env-changed=CODSPEED_INSTRUMENT_HOOKS_DIR"); - - let preload_constants: PreloadConstants = PreloadConstants::default(); + println!("cargo:rerun-if-changed=build.rs"); - // Export constants as environment variables for the Rust code - println!( - "cargo:rustc-env=CODSPEED_URI_ENV={}", - preload_constants.uri_env - ); - println!( - "cargo:rustc-env=CODSPEED_INTEGRATION_NAME={}", - preload_constants.integration_name - ); + println!("cargo:rustc-env=CODSPEED_INTEGRATION_NAME={INTEGRATION_NAME}"); println!( "cargo:rustc-env=CODSPEED_INTEGRATION_VERSION={}", - preload_constants.integration_version - ); - println!( - "cargo:rustc-env=CODSPEED_PRELOAD_LIB_FILENAME={}", - preload_constants.preload_lib_filename + env!("CARGO_PKG_VERSION") ); - - let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap()); - - // Try to get the instrument-hooks directory from the environment variable first, - // otherwise use the one from the instrument-hooks-bindings crate - let instrument_hooks_dir = manifest_dir - .parent() - .unwrap() - .join("instrument-hooks-bindings/instrument-hooks"); - - // Build the preload shared library - let paths = PreloadBuildPaths { - preload_c: manifest_dir.join("preload/codspeed_preload.c"), - core_c: instrument_hooks_dir.join("dist/core.c"), - includes_dir: instrument_hooks_dir.join("includes"), - }; - println!("cargo:rerun-if-changed={}", paths.core_c.display()); - paths.check_sources_exist(); - build_shared_library(&paths, &preload_constants); -} - -/// Build the shared library using the cc crate -fn build_shared_library(paths: &PreloadBuildPaths, constants: &PreloadConstants) { - let uri_env_val = format!("\"{}\"", constants.uri_env); - let integration_name_val = format!("\"{}\"", constants.integration_name); - let integration_version_val = format!("\"{}\"", constants.integration_version); - let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap()); - let out_file = out_dir.join(constants.preload_lib_filename); - - let mut build = cc::Build::new(); - build - .file(&paths.preload_c) - .file(&paths.core_c) - .include(&paths.includes_dir) - .pic(true) - .opt_level(3) - // There's no need to output cargo metadata as we are just building a shared library - // that will be copied to disk and loaded through LD_PRELOAD at runtime - .cargo_metadata(false) - // Pass constants as C defines - .define("CODSPEED_URI_ENV", uri_env_val.as_str()) - .define("CODSPEED_INTEGRATION_NAME", integration_name_val.as_str()) - .define( - "CODSPEED_INTEGRATION_VERSION", - integration_version_val.as_str(), - ) - .std("gnu11") // need gnu11 instead of just c11 for setenv - // Suppress warnings from generated Zig code - .flag("-Wno-format") - .flag("-Wno-format-security") - .flag("-Wno-unused-but-set-variable") - .flag("-Wno-unused-const-variable") - .flag("-Wno-type-limits") - .flag("-Wno-uninitialized") - .flag("-Wno-overflow") - .flag("-Wno-unused-function") - .flag("-Wno-unterminated-string-initialization"); - - // Compile source files to object files - let objects = build.compile_intermediates(); - - // Link object files into shared library - let compiler = build.get_compiler(); - let mut link_cmd = compiler.to_command(); - link_cmd - .arg("-shared") - .arg("-o") - .arg(&out_file) - .args(&objects) - .arg("-lpthread"); - - let status = link_cmd.status().expect("Failed to run linker"); - if !status.success() { - panic!("Failed to link libcodspeed_preload.so"); - } -} - -impl Default for PreloadConstants { - fn default() -> Self { - Self { - uri_env: "CODSPEED_BENCH_URI", - integration_name: "exec-harness", - integration_version: env!("CARGO_PKG_VERSION"), - preload_lib_filename: "libcodspeed_preload.so", - } - } -} - -/// Paths required to build the preload shared library. -struct PreloadBuildPaths { - /// Path to the preload C source file (codspeed_preload.c). - preload_c: PathBuf, - /// Path to the core C source file from instrument-hooks. - core_c: PathBuf, - /// Path to the includes directory from instrument-hooks. - includes_dir: PathBuf, -} - -impl PreloadBuildPaths { - /// Verify that all required source files and directories exist. - /// Panics with a descriptive message if any path is missing. - fn check_sources_exist(&self) { - if !self.core_c.exists() { - panic!( - "core.c not found at {}. Make sure the instrument hooks submodule is available.", - self.core_c.display() - ); - } - if !self.includes_dir.exists() { - panic!( - "includes directory not found at {}. instrument hooks submodule is available.", - self.includes_dir.display() - ); - } - if !self.preload_c.exists() { - panic!( - "codspeed_preload.c not found at {}", - self.preload_c.display() - ); - } - } } diff --git a/crates/exec-harness/preload/codspeed_preload.c b/crates/exec-harness/preload/codspeed_preload.c deleted file mode 100644 index 418af1430..000000000 --- a/crates/exec-harness/preload/codspeed_preload.c +++ /dev/null @@ -1,87 +0,0 @@ -// LD_PRELOAD library for enabling Valgrind instrumentation in child processes -// -// This library is loaded via LD_PRELOAD into benchmark processes spawned by -// exec-harness. It enables callgrind instrumentation on load and disables it on -// exit, allowing exec-harness to measure arbitrary commands without requiring -// them to link against instrument-hooks. -// -// Environment variables: -// CODSPEED_BENCH_URI - The benchmark URI to report (required) - -#include -#include - -#include "core.h" - -#ifndef RUNNING_ON_VALGRIND -// If somehow the core.h did not include the valgrind header, something is -// wrong, but still have a fallback -#warning "RUNNING_ON_VALGRIND not defined, headers may be missing" -#define RUNNING_ON_VALGRIND 0 -#endif - -// These constants are defined by the build script (build.rs) via -D flags -#ifndef CODSPEED_URI_ENV -#error "CODSPEED_URI_ENV must be defined by the build system" -#endif -#ifndef CODSPEED_INTEGRATION_NAME -#error "CODSPEED_INTEGRATION_NAME must be defined by the build system" -#endif -#ifndef CODSPEED_INTEGRATION_VERSION -#error "CODSPEED_INTEGRATION_VERSION must be defined by the build system" -#endif - -static const char *URI_ENV = CODSPEED_URI_ENV; -static const char *INTEGRATION_NAME = CODSPEED_INTEGRATION_NAME; -static const char *INTEGRATION_VERSION = CODSPEED_INTEGRATION_VERSION; - -static InstrumentHooks *g_hooks = NULL; -static const char *g_bench_uri = NULL; - -__attribute__((constructor)) static void codspeed_preload_init(void) { - // Skip initialization if not running under Valgrind yet. - // When using LD_PRELOAD with Valgrind, the constructor runs twice: - // once before Valgrind takes over, and once after. We only want to - // initialize when Valgrind is active. - // - // This is purely empirical, and is not (yet) backed up by documented - // behavior. - if (!RUNNING_ON_VALGRIND) { - return; - } - - g_bench_uri = getenv(URI_ENV); - if (!g_bench_uri) { - return; - } - - g_hooks = instrument_hooks_init(); - if (!g_hooks) { - return; - } - - instrument_hooks_set_integration(g_hooks, INTEGRATION_NAME, - INTEGRATION_VERSION); - - if (instrument_hooks_start_benchmark_inline(g_hooks) != 0) { - instrument_hooks_deinit(g_hooks); - g_hooks = NULL; - return; - } -} - -__attribute__((destructor)) static void codspeed_preload_fini(void) { - // If the process is not the owner of the lock, this means g_hooks was not - // initialized - if (!g_hooks) { - return; - } - - instrument_hooks_stop_benchmark_inline(g_hooks); - - int32_t pid = getpid(); - instrument_hooks_set_executed_benchmark(g_hooks, pid, g_bench_uri); - - instrument_hooks_deinit(g_hooks); - g_hooks = NULL; -} diff --git a/crates/exec-harness/src/analysis/ld_preload_check.rs b/crates/exec-harness/src/analysis/ld_preload_check.rs deleted file mode 100644 index 702f18d2a..000000000 --- a/crates/exec-harness/src/analysis/ld_preload_check.rs +++ /dev/null @@ -1,120 +0,0 @@ -use crate::prelude::*; -use std::fs; -use std::path::Path; - -/// Checks if the given executable will honor LD_PRELOAD. -/// -/// Returns `Ok(())` if LD_PRELOAD will work, or an error with a descriptive message if not. -/// -/// LD_PRELOAD works for: -/// - Dynamically linked ELF binaries -/// - Scripts (the interpreter is typically dynamically linked) -/// -/// LD_PRELOAD does NOT work for: -/// - Statically linked ELF binaries (no dynamic linker involved) -pub fn check_ld_preload_compatible(executable: &str) -> Result<()> { - let path = resolve_executable(executable)?; - let data = fs::read(&path) - .with_context(|| format!("Failed to read executable: {}", path.display()))?; - - // Check ELF magic bytes - if data.len() >= 4 && &data[0..4] == b"\x7FELF" { - check_elf_is_dynamic(&data, &path) - } else { - // Not an ELF file - likely a script with a shebang. - // Scripts use an interpreter which is typically dynamically linked. - Ok(()) - } -} - -/// Resolve executable name to its full path using PATH lookup. -fn resolve_executable(executable: &str) -> Result { - let path = Path::new(executable); - - // If it's already an absolute or relative path, use it directly - if path.is_absolute() || executable.contains('/') { - return Ok(path.to_path_buf()); - } - - // Search in PATH - if let Ok(path_env) = std::env::var("PATH") { - for dir in path_env.split(':') { - let candidate = Path::new(dir).join(executable); - if candidate.is_file() { - return Ok(candidate); - } - } - } - - bail!("Executable not found in PATH: {executable}") -} - -/// Check if an ELF binary is dynamically linked. -fn check_elf_is_dynamic(data: &[u8], path: &Path) -> Result<()> { - use object::Endianness; - use object::read::elf::ElfFile; - - // Try parsing as 64-bit ELF first, then 32-bit - if let Ok(elf) = ElfFile::>::parse(data) { - return check_elf_has_interp(elf, path); - } - - if let Ok(elf) = ElfFile::>::parse(data) { - return check_elf_has_interp(elf, path); - } - - bail!("Failed to parse ELF file: {}", path.display()) -} - -/// Check if an ELF file has a PT_INTERP or PT_DYNAMIC segment, indicating dynamic linking. -fn check_elf_has_interp<'data, Elf>( - elf: object::read::elf::ElfFile<'data, Elf>, - path: &Path, -) -> Result<()> -where - Elf: object::read::elf::FileHeader, -{ - use object::read::elf::ProgramHeader; - - let endian = elf.endian(); - - for segment in elf.elf_program_headers() { - let p_type = segment.p_type(endian); - // Either PT_INTERP or PT_DYNAMIC indicates a dynamically linked binary - if p_type == object::elf::PT_INTERP || p_type == object::elf::PT_DYNAMIC { - return Ok(()); - } - } - - // No PT_INTERP found - this is a statically linked binary - bail!( - "The codspeed CLI in CPU Simulation mode does not support statically linked binaries.\n\n\ - Executable '{}' is statically linked.\n\n\ - Please either:\n\ - - Use a dynamically linked executable, or\n\ - - Use a different measurement mode, or\n\ - - Use one of the CodSpeed framework benchmark integrations", - path.display() - ) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_dynamic_binary() { - // /bin/sh or similar should be dynamically linked on most systems - let result = check_ld_preload_compatible("sh"); - assert!( - result.is_ok(), - "sh should be dynamically linked: {result:?}" - ); - } - - #[test] - fn test_nonexistent_binary() { - let result = check_ld_preload_compatible("nonexistent_binary_12345"); - assert!(result.is_err()); - } -} diff --git a/crates/exec-harness/src/analysis/mod.rs b/crates/exec-harness/src/analysis/mod.rs index 8bb4eaf44..21f91d1e0 100644 --- a/crates/exec-harness/src/analysis/mod.rs +++ b/crates/exec-harness/src/analysis/mod.rs @@ -1,25 +1,60 @@ +use crate::MeasurementMode; use crate::constants::INTEGRATION_NAME; use crate::constants::INTEGRATION_VERSION; use crate::prelude::*; use crate::BenchmarkCommand; -use crate::constants; use crate::uri; use instrument_hooks_bindings::InstrumentHooks; use std::process::Command; -mod ld_preload_check; -mod preload_lib_file; - -pub fn perform(commands: Vec) -> Result<()> { +/// Executes the given benchmark commands, measuring each one through the +/// instrument hooks. +/// +/// Instrumentation is toggled in *this* process, around the spawn of each +/// benchmark command. Under Valgrind, the benchmarked child inherits the live +/// instrumentation state across `fork`/`exec`, and callgrind records the spawn +/// edge on the dump part that is live at fork time — the same part that +/// [`InstrumentHooks::set_executed_benchmark`] then names with the benchmark +/// URI. The backend walks that edge to attribute the child's trace to the +/// benchmark, so the measurement covers the whole spawned process tree. +/// +/// Nothing is injected into the benchmarked executable, so statically linked +/// ones work. +pub fn perform(commands: Vec, mode: MeasurementMode) -> Result<()> { let hooks = InstrumentHooks::instance(INTEGRATION_NAME, INTEGRATION_VERSION); + if !hooks.is_instrumented() { + // Every way this mode can go wrong is silent: the harness runs, the + // benchmark completes, and the measurement is empty. Fail loudly + // instead. + // + // Note this only catches the absence of *any* instrument (no + // instrument-hooks support compiled in, or nothing to attach to). It + // cannot tell whether Valgrind will actually honour the instrumentation + // toggles, which depends on the `--instr-atstart` the runner passes. + bail!( + "exec-harness found no instrument to report to, so nothing would be measured.\n\ + This binary is meant to be run by the CodSpeed CLI, which sets up the \ + instrumentation around it." + ); + } + for benchmark_cmd in commands { let name_and_uri = uri::generate_name_and_uri(&benchmark_cmd.name, &benchmark_cmd.command); name_and_uri.print_executing(); let mut cmd = Command::new(&benchmark_cmd.command[0]); cmd.args(&benchmark_cmd.command[1..]); + + if mode == MeasurementMode::Simulation { + // Make sure python and node processes output perf maps, so the + // runner can resolve JIT-ed frames afterwards. For python this is + // usually done by `pytest-codspeed`. + cmd.env("PYTHONPERFSUPPORT", "1"); + crate::node::set_node_options(&mut cmd); + } + hooks.start_benchmark().unwrap(); let status = cmd.status(); hooks.stop_benchmark().unwrap(); @@ -34,40 +69,3 @@ pub fn perform(commands: Vec) -> Result<()> { Ok(()) } - -/// Executes the given benchmark commands using a preload based trick to handle valgrind control. -/// -/// This function is only supported on Unix-like platforms, as it relies on the -/// `LD_PRELOAD` environment variable and Unix file permissions for shared libraries. -/// It will not work on non-Unix platforms or with statically linked binaries. -pub fn perform_with_valgrind(commands: Vec) -> Result<()> { - let preload_lib_path = preload_lib_file::get_preload_lib_path()?; - - for benchmark_cmd in commands { - // Check if the executable will honor LD_PRELOAD before running - ld_preload_check::check_ld_preload_compatible(&benchmark_cmd.command[0])?; - - let name_and_uri = uri::generate_name_and_uri(&benchmark_cmd.name, &benchmark_cmd.command); - name_and_uri.print_executing(); - - let mut cmd = Command::new(&benchmark_cmd.command[0]); - cmd.args(&benchmark_cmd.command[1..]); - // Use LD_PRELOAD to inject instrumentation into the child process - cmd.env("LD_PRELOAD", preload_lib_path); - // Make sure python processes output perf maps. This is usually done by `pytest-codspeed` - cmd.env("PYTHONPERFSUPPORT", "1"); - cmd.env(constants::URI_ENV, &name_and_uri.uri); - - crate::node::set_node_options(&mut cmd); - - let mut child = cmd.spawn().context("Failed to spawn command")?; - - let status = child.wait().context("Failed to execute command")?; - - if !status.success() { - bail!("Command exited with non-zero status: {status}"); - } - } - - Ok(()) -} diff --git a/crates/exec-harness/src/analysis/preload_lib_file.rs b/crates/exec-harness/src/analysis/preload_lib_file.rs deleted file mode 100644 index 2d53804cb..000000000 --- a/crates/exec-harness/src/analysis/preload_lib_file.rs +++ /dev/null @@ -1,46 +0,0 @@ -use crate::prelude::*; - -use std::io::Write; -use std::sync::OnceLock; - -/// Filename for the preload shared library. -const PRELOAD_LIB_FILENAME: &str = env!("CODSPEED_PRELOAD_LIB_FILENAME"); - -/// The preload library binary embedded at compile time. -const PRELOAD_LIB_BYTES: &[u8] = include_bytes!(concat!( - env!("OUT_DIR"), - "/", - env!("CODSPEED_PRELOAD_LIB_FILENAME") -)); - -/// Lazily initialized temp file containing the extracted preload library. -/// Kept in a static to prevent cleanup until process exit. -static PRELOAD_LIB_FILE: OnceLock = OnceLock::new(); - -/// Extracts the preload library to a temp file. -fn extract_preload_lib() -> Result { - let mut file = tempfile::Builder::new() - .suffix(PRELOAD_LIB_FILENAME) - .tempfile() - .context("Failed to create temp file for preload library")?; - - file.write_all(PRELOAD_LIB_BYTES) - .context("Failed to write preload library to temp file")?; - - debug!( - "Extracted preload library to temp file: {}", - file.path().display() - ); - - Ok(file) -} - -/// Returns the path to the preload library, extracting it to a temp file if needed. -pub(super) fn get_preload_lib_path() -> Result<&'static std::path::Path> { - if let Some(file) = PRELOAD_LIB_FILE.get() { - return Ok(file.path()); - } - - let file = extract_preload_lib()?; - Ok(PRELOAD_LIB_FILE.get_or_init(|| file).path()) -} diff --git a/crates/exec-harness/src/cli.rs b/crates/exec-harness/src/cli.rs new file mode 100644 index 000000000..166315189 --- /dev/null +++ b/crates/exec-harness/src/cli.rs @@ -0,0 +1,65 @@ +//! exec-harness's command line, shared by the standalone `exec-harness` binary +//! and by the `codspeed` CLI that bundles it as a hidden subcommand. +//! +//! Keeping the parser and the dispatch here rather than in `main.rs` is what +//! stops the two paths from drifting: the standalone binary is a wrapper over +//! [`run_cli`] and nothing else. It deliberately does **not** install a logger — +//! only one global logger can exist per process, and when exec-harness runs +//! bundled the host CLI has already installed its own. + +use crate::prelude::*; +use crate::walltime::WalltimeExecutionArgs; +use crate::{BenchmarkCommand, MeasurementMode, execute_benchmarks, read_commands_from_stdin}; +use clap::Parser; +use std::ffi::OsString; + +#[derive(Parser, Debug)] +#[command(name = "exec-harness")] +#[command( + version, + about = "CodSpeed exec harness - wraps commands with performance instrumentation" +)] +struct Args { + /// Optional benchmark name, else the command will be used as the name + #[arg(long)] + name: Option, + + /// Set by the runner, should be coherent with the executor being used + #[arg(short, long, global = true, env = "CODSPEED_RUNNER_MODE", hide = true)] + measurement_mode: Option, + + #[command(flatten)] + walltime_args: WalltimeExecutionArgs, + + /// The command and arguments to execute. + /// Use "-" as the only argument to read a JSON payload from stdin. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + command: Vec, +} + +/// Parse `argv` and run the requested benchmarks. +pub fn run_cli(argv: I) -> Result<()> +where + I: IntoIterator, + T: Into + Clone, +{ + debug!("Starting exec-harness with pid {}", std::process::id()); + + let args = Args::parse_from(argv); + let measurement_mode = args.measurement_mode; + + // Determine if we're in stdin mode or CLI mode + let commands = match args.command.as_slice() { + [single] if single == "-" => read_commands_from_stdin()?, + [] => bail!("No command provided"), + _ => vec![BenchmarkCommand { + command: args.command, + name: args.name, + walltime_args: args.walltime_args, + }], + }; + + execute_benchmarks(commands, measurement_mode)?; + + Ok(()) +} diff --git a/crates/exec-harness/src/constants.rs b/crates/exec-harness/src/constants.rs index 9a47591ce..f982f3954 100644 --- a/crates/exec-harness/src/constants.rs +++ b/crates/exec-harness/src/constants.rs @@ -1,11 +1,8 @@ //! Shared constants for the exec-harness crate. //! //! These constants are defined in the build script (build.rs) and exported as -//! environment variables. The same values are passed to the C preload library -//! as compiler defines, ensuring both Rust and C code use the same source of truth. - -/// Environment variable name for the benchmark URI. -pub const URI_ENV: &str = env!("CODSPEED_URI_ENV"); +//! environment variables, so that the integration identity reported to CodSpeed +//! has a single source of truth. /// Integration name reported to CodSpeed. pub const INTEGRATION_NAME: &str = env!("CODSPEED_INTEGRATION_NAME"); diff --git a/crates/exec-harness/src/lib.rs b/crates/exec-harness/src/lib.rs index 30cb21b46..52954b782 100644 --- a/crates/exec-harness/src/lib.rs +++ b/crates/exec-harness/src/lib.rs @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize}; use std::io::{self, BufRead}; pub mod analysis; +/// The command line, shared by the standalone binary and the bundled subcommand. +pub mod cli; pub mod constants; pub mod node; pub mod prelude; @@ -74,11 +76,8 @@ pub fn execute_benchmarks( Some(MeasurementMode::Walltime) | None => { walltime::perform(commands)?; } - Some(MeasurementMode::Memory) => { - analysis::perform(commands)?; - } - Some(MeasurementMode::Simulation) => { - analysis::perform_with_valgrind(commands)?; + Some(mode @ (MeasurementMode::Memory | MeasurementMode::Simulation)) => { + analysis::perform(commands, mode)?; } } diff --git a/crates/exec-harness/src/main.rs b/crates/exec-harness/src/main.rs index 99cbf7cd2..89bbe8c84 100644 --- a/crates/exec-harness/src/main.rs +++ b/crates/exec-harness/src/main.rs @@ -1,33 +1,12 @@ -use clap::Parser; +//! The standalone `exec-harness` binary. +//! +//! Everything it does lives in [`exec_harness::cli::run_cli`], which the +//! `codspeed` CLI also calls when exec-harness runs as a bundled subcommand. +//! All that is left here is the global logger, which only makes sense when +//! exec-harness owns the whole process. + +use exec_harness::cli::run_cli; use exec_harness::prelude::*; -use exec_harness::walltime::WalltimeExecutionArgs; -use exec_harness::{ - BenchmarkCommand, MeasurementMode, execute_benchmarks, read_commands_from_stdin, -}; - -#[derive(Parser, Debug)] -#[command(name = "exec-harness")] -#[command( - version, - about = "CodSpeed exec harness - wraps commands with performance instrumentation" -)] -struct Args { - /// Optional benchmark name, else the command will be used as the name - #[arg(long)] - name: Option, - - /// Set by the runner, should be coherent with the executor being used - #[arg(short, long, global = true, env = "CODSPEED_RUNNER_MODE", hide = true)] - measurement_mode: Option, - - #[command(flatten)] - walltime_args: WalltimeExecutionArgs, - - /// The command and arguments to execute. - /// Use "-" as the only argument to read a JSON payload from stdin. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - command: Vec, -} fn main() -> Result<()> { env_logger::builder() @@ -38,23 +17,5 @@ fn main() -> Result<()> { }) .init(); - debug!("Starting exec-harness with pid {}", std::process::id()); - - let args = Args::parse(); - let measurement_mode = args.measurement_mode; - - // Determine if we're in stdin mode or CLI mode - let commands = match args.command.as_slice() { - [single] if single == "-" => read_commands_from_stdin()?, - [] => bail!("No command provided"), - _ => vec![BenchmarkCommand { - command: args.command, - name: args.name, - walltime_args: args.walltime_args, - }], - }; - - execute_benchmarks(commands, measurement_mode)?; - - Ok(()) + run_cli(std::env::args_os()) } diff --git a/crates/instrument-hooks-bindings/build.rs b/crates/instrument-hooks-bindings/build.rs index 63b46664e..eb6611e85 100644 --- a/crates/instrument-hooks-bindings/build.rs +++ b/crates/instrument-hooks-bindings/build.rs @@ -1,3 +1,5 @@ +use std::env; + fn main() { println!("cargo:rustc-check-cfg=cfg(use_instrument_hooks)"); @@ -35,6 +37,24 @@ fn main() { Err(e) => { let compiler = build.try_get_compiler().expect("Failed to get C compiler"); + // Falling back to the noop implementation makes every hook a + // no-op, so a build that lands there runs benchmarks and reports + // no measurement at all, at exit code 0. Linux is where we + // actually measure, so fail the build instead of emitting a + // warning nobody reads. + if env::var("CARGO_CFG_TARGET_OS").as_deref() == Ok("linux") { + panic!( + "Failed to compile the instrument-hooks native library with cc-rs.\n\ + A Linux build must not fall back to the noop implementation: it \ + would run benchmarks and measure nothing.\n\ + Make sure a C compiler for the target is installed and reachable \ + by cc-rs (for musl targets, `musl-tools` provides \ + `-linux-musl-gcc`).\n\ + Compiler information: {compiler:?}\n\ + Compilation error: {e}" + ); + } + eprintln!("\n\nWARNING: Failed to compile instrument-hooks native library with cc-rs."); eprintln!( "The library will still compile, but instrument-hooks functionality will be disabled." diff --git a/crates/memtrack/Cargo.toml b/crates/memtrack/Cargo.toml index d97ed4f79..c8e241ebf 100644 --- a/crates/memtrack/Cargo.toml +++ b/crates/memtrack/Cargo.toml @@ -50,17 +50,6 @@ test-log = { workspace = true } insta = { workspace = true, features = ["json", "redactions"] } test-with = { workspace = true } -[package.metadata.dist] -targets = ["aarch64-unknown-linux-gnu", "x86_64-unknown-linux-gnu"] -features = ["libbpf-rs/static"] - -[package.metadata.dist.dependencies.apt] -build-essential = "*" -pkgconf = "*" -zlib1g-dev = "*" -libbpf-dev = "*" - -# Required for the vendored feature -autopoint = "*" -bison = "*" -flex = "*" +# Deliberately no `[package.metadata.dist]`: memtrack ships inside the +# `codspeed` binary and is not released on its own. Its apt build dependencies +# belong to the root package, which is what cargo-dist builds. diff --git a/crates/memtrack/musl/argp.h b/crates/memtrack/musl/argp.h new file mode 100644 index 000000000..87dbd8fde --- /dev/null +++ b/crates/memtrack/musl/argp.h @@ -0,0 +1,60 @@ +/* crates/memtrack/musl/argp.h — stub for musl builds of libbpf-sys' vendored elfutils. + Declarations only: a libelf-only build never calls into argp, but the + elfutils sources still `#include `, which musl does not ship. + If compilation complains about a missing type or macro, add it here. + + This directory is on `CPATH` for *every* build, gnu included, so the header + has to defer to a real wherever one exists. + + It branches on the libc rather than on the include path, and the difference + matters: `.cargo/config.toml` also puts `-idirafter /usr/include` on the musl + build, for libbpf's kernel UAPI headers, which makes glibc's argp.h reachable + from a musl compilation. `__has_include_next` would find it and the build + would die on `__THROW`. is included only to pull in , + which defines `__GLIBC__`. */ +#include +#if defined(__GLIBC__) +#include_next +#else + +#ifndef CODSPEED_STUB_ARGP_H +#define CODSPEED_STUB_ARGP_H + +#include + +typedef int error_t; + +struct argp_option { + const char *name; + int key; + const char *arg; + int flags; + const char *doc; + int group; +}; + +struct argp_state { + const char *name; +}; + +typedef error_t (*argp_parser_t)(int key, char *arg, struct argp_state *state); + +struct argp { + const struct argp_option *options; + argp_parser_t parser; + const char *args_doc; + const char *doc; + const void *children; + void *help_filter; + const char *argp_domain; +}; + +#define OPTION_ARG_OPTIONAL 0x1 +#define ARGP_HELP_SEE 0x40 +#define ARGP_ERR_UNKNOWN 1 + +int argp_help(const struct argp *argp, FILE *stream, unsigned int flags, char *name); + +#endif /* CODSPEED_STUB_ARGP_H */ + +#endif /* __GLIBC__ */ diff --git a/crates/memtrack/src/cli.rs b/crates/memtrack/src/cli.rs new file mode 100644 index 000000000..86ca66ab9 --- /dev/null +++ b/crates/memtrack/src/cli.rs @@ -0,0 +1,201 @@ +//! memtrack's command line, shared by the standalone `codspeed-memtrack` +//! binary and by the `codspeed` CLI that bundles it as a hidden subcommand. +//! +//! Keeping the parser and the work here rather than in `main.rs` is what stops +//! the two paths from drifting: the standalone binary is a wrapper over +//! [`run_cli`] and nothing else. It deliberately does **not** install a logger — +//! only one global logger can exist per process, and when memtrack runs bundled +//! the host CLI has already installed its own. `main.rs` installs one because +//! there it is the only thing in the process. + +use crate::prelude::*; +use crate::{MemtrackIpcMessage, Tracker, handle_ipc_message}; +use clap::Parser; +use ipc_channel::ipc; +use runner_shared::artifacts::{ArtifactExt, MemtrackArtifact, encode_events}; +use std::ffi::OsString; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::Arc; +use std::thread; + +#[derive(Parser)] +#[command(name = "memtrack")] +#[command(version, about = "Track memory allocations using eBPF", long_about = None)] +struct Cli { + #[command(subcommand)] + command: Commands, +} + +#[derive(Parser)] +enum Commands { + /// Track memory allocations for a command + Track { + /// Command to execute and track + command: String, + + /// Output folder for the allocations data + #[arg(short, long, default_value = ".")] + output: PathBuf, + + /// Optional IPC server name for receiving control commands + #[arg(long)] + ipc_server: Option, + }, +} + +/// Parse `argv` and run the requested subcommand, returning the exit code the +/// process should end with. +/// +/// The code is returned rather than passed to [`std::process::exit`] so that the +/// caller stays in charge of teardown. Both callers do exit on it: the tracked +/// command's status is the only meaningful result of a `track` run. +pub fn run_cli(argv: I) -> Result +where + I: IntoIterator, + T: Into + Clone, +{ + let cli = Cli::parse_from(argv); + + match cli.command { + Commands::Track { + command, + output: out_dir, + ipc_server, + } => { + debug!("Starting memtrack for command: {command}"); + + let status = + track_command(&command, ipc_server, &out_dir).context("Failed to track command")?; + + Ok(status.code().unwrap_or(1)) + } + } +} + +/// Get the original user's UID and GID when running under sudo. +/// Returns None if not running under sudo or if the environment variables are not set. +fn get_user_uid_gid() -> Option<(u32, u32)> { + let uid = std::env::var("SUDO_UID").ok()?.parse().ok()?; + let gid = std::env::var("SUDO_GID").ok()?.parse().ok()?; + Some((uid, gid)) +} + +fn track_command( + cmd_string: &str, + ipc_server_name: Option, + out_dir: &Path, +) -> anyhow::Result { + // First, establish IPC connection if needed to avoid timeouts on the runner because + // creating the Tracker instance takes some time. + let ipc_channel = if let Some(server_name) = ipc_server_name { + debug!("Connecting to IPC server: {server_name}"); + + let (tx, rx) = ipc::channel::()?; + let sender = ipc::IpcSender::connect(server_name)?; + sender.send(tx)?; + + Some(rx) + } else { + None + }; + + let tracker = Arc::new(Tracker::new()?); + + // Spawn IPC handler thread with the now-available tracker + let ipc_handle = if let Some(rx) = ipc_channel { + let tracker = tracker.clone(); + Some(thread::spawn(move || { + while let Ok(msg) = rx.recv() { + handle_ipc_message(msg, &tracker); + } + })) + } else { + // Without IPC, nothing toggles the tracking_enabled map, so allocator + // events would be dropped by the eBPF is_enabled() check. Enable it up + // front. + tracker.enable_tracking()?; + None + }; + + // Run the target command through bash to handle shell syntax. Drop + // privileges if running under sudo to avoid permission issues when the + // target accesses files owned by the original user. + let mut cmd = Command::new("bash"); + cmd.arg("-c").arg(cmd_string); + let uid_gid = get_user_uid_gid(); + if let Some((uid, gid)) = uid_gid { + debug!("Running under sudo, dropping privileges to uid={uid}, gid={gid}"); + } + + let mut session = tracker + .spawn(&cmd, uid_gid) + .map_err(|e| anyhow!("Failed to spawn child process: {e}"))?; + let root_pid = session.pid(); + let event_rx = session.take_events()?; + debug!("Spawned child with pid {root_pid}"); + + // Generate output file name and create file for streaming events + let file_name = MemtrackArtifact::file_name(Some(root_pid)); + let out_file = std::fs::File::create(out_dir.join(file_name))?; + + // Leave headroom for the ring buffer poll thread and the tracked + // command: encode workers on every core starve the poller during + // allocation bursts, which overflows the kernel ring buffer. + let n_workers = thread::available_parallelism() + .map(|n| n.get().saturating_sub(2).max(1)) + .unwrap_or(4); + + let pipeline_thread = thread::spawn(move || encode_events(event_rx, out_file, n_workers)); + + // Wait for the command to complete + let status = session.wait().context("Failed to wait for command")?; + debug!("Command exited with status: {status}"); + + // Stop allocator-event production before draining: the child has exited, + // so anything still arriving is already in the ring buffer. + if let Err(e) = tracker.disable_tracking() { + warn!("Failed to disable tracking: {e:#}"); + } + + // Dropping the session drops the event poller, which does a final drain of + // the ring buffer and then closes the event channel. Without this the + // encode pipeline join below would block forever. + debug!("Stopping the ring buffer poller"); + drop(session); + + debug!("Waiting for the encode pipeline to finish"); + let total = pipeline_thread + .join() + .map_err(|_| anyhow::anyhow!("Failed to join memtrack encode pipeline"))??; + + info!("Wrote {total} memtrack events to disk"); + + // Stop the attach worker and surface any fatal error it recorded (missed + // exec mappings mean incomplete allocator coverage). + tracker.finish()?; + + // Detach probes explicitly: the IPC thread still holds an Arc clone, so the + // tracker would otherwise never be dropped before process::exit and the + // kernel would close every link fd serially during exit. + tracker.detach(); + + // Read the eBPF dropped-event counter after the run is complete. + // A non-zero value means the ring buffer overflowed and the trace is + // incomplete. + let dropped_events = tracker + .dropped_events_count() + .context("Failed to read memtrack dropped-event counter")?; + if dropped_events > 0 { + bail!( + "Memtrack ring buffer overflowed: {dropped_events} events lost, aborting since the trace is incomplete.\n\ + Try reducing the benchmark's allocation rate (fewer iterations or smaller inputs), \ + or report it at https://github.com/CodSpeedHQ/codspeed/issues." + ); + } + + // IPC thread will exit when channel closes + drop(ipc_handle); + + Ok(status) +} diff --git a/crates/memtrack/src/ebpf/memtrack/mod.rs b/crates/memtrack/src/ebpf/memtrack/mod.rs index 2ec1f04f5..22afdec11 100644 --- a/crates/memtrack/src/ebpf/memtrack/mod.rs +++ b/crates/memtrack/src/ebpf/memtrack/mod.rs @@ -282,19 +282,62 @@ impl Drop for MemtrackBpf { #[cfg(test)] mod tests { use super::*; + use std::process::{Child, Command, Stdio}; + use std::time::{Duration, Instant}; + + /// A spawned child, killed and reaped when it goes out of scope, including + /// on the panic path of a failed assertion. + struct Reaped(Child); + + impl Drop for Reaped { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } + } + + /// The `libc.so.6` that a freshly spawned child has mapped. + /// + /// Read from a child rather than from `/proc/self/maps`: a static musl + /// build of this binary maps no `libc.so.6` at all, and the production path + /// resolves symbols in a traced process anyway, never in memtrack's own. + fn libc_mapped_by_a_child() -> String { + let child = Reaped( + Command::new("sleep") + .arg("30") + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("could not spawn `sleep`"), + ); + let pid = child.0.id(); + + // The mapping is made by the child's dynamic linker, which has not + // necessarily run by the time `spawn` returns. + let deadline = Instant::now() + Duration::from_secs(5); + loop { + if let Ok(maps) = std::fs::read_to_string(format!("/proc/{pid}/maps")) { + if let Some(path) = maps.lines().find_map(|line| { + let path = line.split_whitespace().last()?; + path.contains("libc.so.6").then(|| path.to_owned()) + }) { + return path; + } + } + assert!( + Instant::now() < deadline, + "pid {pid} mapped no libc.so.6 within 5s — is `sleep` statically \ + linked or built against a non-glibc libc on this host?" + ); + std::thread::sleep(Duration::from_millis(10)); + } + } /// Allocator entry points must resolve to file offsets; a symbol that /// silently fails to resolve attaches nothing and loses all events. #[test] fn libc_allocator_symbols_resolve_to_offsets() { - let maps = std::fs::read_to_string("/proc/self/maps").unwrap(); - let libc_path = maps - .lines() - .find_map(|line| { - let path = line.split_whitespace().last()?; - path.contains("libc.so.6").then(|| path.to_owned()) - }) - .expect("test process has no mapped libc.so.6"); + let libc_path = libc_mapped_by_a_child(); let symbols = resolve_symbol_offsets(Path::new(&libc_path)).unwrap(); for symbol in ["malloc", "calloc", "realloc", "free"] { diff --git a/crates/memtrack/src/lib.rs b/crates/memtrack/src/lib.rs index ccd93399f..3cc133be4 100644 --- a/crates/memtrack/src/lib.rs +++ b/crates/memtrack/src/lib.rs @@ -1,5 +1,10 @@ mod allocators; mod bpf_token; +/// The command line, shared by the standalone binary and the bundled subcommand. +/// Gated on `ebpf` for the same reason the `[[bin]]` is: without it there is no +/// `Tracker` to drive. +#[cfg(feature = "ebpf")] +pub mod cli; #[cfg(feature = "ebpf")] mod ebpf; mod ipc; diff --git a/crates/memtrack/src/main.rs b/crates/memtrack/src/main.rs index 283cff194..231c27c24 100644 --- a/crates/memtrack/src/main.rs +++ b/crates/memtrack/src/main.rs @@ -1,45 +1,12 @@ -use clap::Parser; -use ipc_channel::ipc; +//! The standalone `codspeed-memtrack` binary. +//! +//! Everything it does lives in [`memtrack::cli::run_cli`], which the `codspeed` +//! CLI also calls when memtrack runs as a bundled subcommand. All that is left +//! here is what only makes sense when memtrack owns the whole process: the +//! global logger, and turning the tracked command's exit code into our own. + +use memtrack::cli::run_cli; use memtrack::prelude::*; -use memtrack::{MemtrackIpcMessage, Tracker, handle_ipc_message}; -use runner_shared::artifacts::{ArtifactExt, MemtrackArtifact, encode_events}; -use std::path::{Path, PathBuf}; -use std::process::Command; -use std::sync::Arc; -use std::thread; - -#[derive(Parser)] -#[command(name = "memtrack")] -#[command(version, about = "Track memory allocations using eBPF", long_about = None)] -struct Cli { - #[command(subcommand)] - command: Commands, -} - -#[derive(Parser)] -enum Commands { - /// Track memory allocations for a command - Track { - /// Command to execute and track - command: String, - - /// Output folder for the allocations data - #[arg(short, long, default_value = ".")] - output: PathBuf, - - /// Optional IPC server name for receiving control commands - #[arg(long)] - ipc_server: Option, - }, -} - -/// Get the original user's UID and GID when running under sudo. -/// Returns None if not running under sudo or if the environment variables are not set. -fn get_user_uid_gid() -> Option<(u32, u32)> { - let uid = std::env::var("SUDO_UID").ok()?.parse().ok()?; - let gid = std::env::var("SUDO_GID").ok()?.parse().ok()?; - Some((uid, gid)) -} fn main() -> Result<()> { env_logger::builder() @@ -47,139 +14,6 @@ fn main() -> Result<()> { .format_timestamp(None) .init(); - let cli = Cli::parse(); - - match cli.command { - Commands::Track { - command, - output: out_dir, - ipc_server, - } => { - debug!("Starting memtrack for command: {command}"); - - let status = - track_command(&command, ipc_server, &out_dir).context("Failed to track command")?; - - std::process::exit(status.code().unwrap_or(1)); - } - } -} - -fn track_command( - cmd_string: &str, - ipc_server_name: Option, - out_dir: &Path, -) -> anyhow::Result { - // First, establish IPC connection if needed to avoid timeouts on the runner because - // creating the Tracker instance takes some time. - let ipc_channel = if let Some(server_name) = ipc_server_name { - debug!("Connecting to IPC server: {server_name}"); - - let (tx, rx) = ipc::channel::()?; - let sender = ipc::IpcSender::connect(server_name)?; - sender.send(tx)?; - - Some(rx) - } else { - None - }; - - let tracker = Arc::new(Tracker::new()?); - - // Spawn IPC handler thread with the now-available tracker - let ipc_handle = if let Some(rx) = ipc_channel { - let tracker = tracker.clone(); - Some(thread::spawn(move || { - while let Ok(msg) = rx.recv() { - handle_ipc_message(msg, &tracker); - } - })) - } else { - // Without IPC, nothing toggles the tracking_enabled map, so allocator - // events would be dropped by the eBPF is_enabled() check. Enable it up - // front. - tracker.enable_tracking()?; - None - }; - - // Run the target command through bash to handle shell syntax. Drop - // privileges if running under sudo to avoid permission issues when the - // target accesses files owned by the original user. - let mut cmd = Command::new("bash"); - cmd.arg("-c").arg(cmd_string); - let uid_gid = get_user_uid_gid(); - if let Some((uid, gid)) = uid_gid { - debug!("Running under sudo, dropping privileges to uid={uid}, gid={gid}"); - } - - let mut session = tracker - .spawn(&cmd, uid_gid) - .map_err(|e| anyhow!("Failed to spawn child process: {e}"))?; - let root_pid = session.pid(); - let event_rx = session.take_events()?; - debug!("Spawned child with pid {root_pid}"); - - // Generate output file name and create file for streaming events - let file_name = MemtrackArtifact::file_name(Some(root_pid)); - let out_file = std::fs::File::create(out_dir.join(file_name))?; - - // Leave headroom for the ring buffer poll thread and the tracked - // command: encode workers on every core starve the poller during - // allocation bursts, which overflows the kernel ring buffer. - let n_workers = thread::available_parallelism() - .map(|n| n.get().saturating_sub(2).max(1)) - .unwrap_or(4); - - let pipeline_thread = thread::spawn(move || encode_events(event_rx, out_file, n_workers)); - - // Wait for the command to complete - let status = session.wait().context("Failed to wait for command")?; - debug!("Command exited with status: {status}"); - - // Stop allocator-event production before draining: the child has exited, - // so anything still arriving is already in the ring buffer. - if let Err(e) = tracker.disable_tracking() { - warn!("Failed to disable tracking: {e:#}"); - } - - // Dropping the session drops the event poller, which does a final drain of - // the ring buffer and then closes the event channel. Without this the - // encode pipeline join below would block forever. - debug!("Stopping the ring buffer poller"); - drop(session); - - debug!("Waiting for the encode pipeline to finish"); - let total = pipeline_thread - .join() - .map_err(|_| anyhow::anyhow!("Failed to join memtrack encode pipeline"))??; - - info!("Wrote {total} memtrack events to disk"); - - // Stop the attach worker and surface any fatal error it recorded (missed - // exec mappings mean incomplete allocator coverage). - tracker.finish()?; - - // Detach probes explicitly: the IPC thread still holds an Arc clone, so the - // tracker would otherwise never be dropped before process::exit and the - // kernel would close every link fd serially during exit. - tracker.detach(); - - // Read the eBPF dropped-event counter after the run is complete. - // A non-zero value means the ring buffer overflowed and the trace is - // incomplete. - let dropped_events = tracker - .dropped_events_count() - .context("Failed to read memtrack dropped-event counter")?; - if dropped_events > 0 { - bail!( - "Memtrack ring buffer overflowed: {dropped_events} events lost, aborting since the trace is incomplete.\n\ - Try reducing the benchmark's allocation rate (fewer iterations or smaller inputs), \ - or report it at https://github.com/CodSpeedHQ/codspeed/issues." - ); - } - - // IPC thread will exit when channel closes - drop(ipc_handle); - - Ok(status) + let code = run_cli(std::env::args_os())?; + std::process::exit(code); } diff --git a/src/binary_installer/mod.rs b/src/binary_installer/mod.rs deleted file mode 100644 index d8bdb75bf..000000000 --- a/src/binary_installer/mod.rs +++ /dev/null @@ -1,97 +0,0 @@ -use crate::binary_pins::PinnedBinary; -use crate::cli::run::helpers::download_pinned_file; -use crate::prelude::*; -use semver::Version; -use std::process::Command; -use tempfile::NamedTempFile; - -mod versions; - -/// Ensure a binary is installed, or install it from a `PinnedBinary` installer script. -/// -/// This function checks if the binary is already installed with the correct version. -/// If not, it downloads and executes the pinned installer script. -/// -/// # Arguments -/// * `binary_name` - The binary command name (e.g., "codspeed-memtrack", "codspeed-exec-harness") -/// * `version` - The version to install (e.g., "4.4.2-alpha.2") -/// * `installer` - The `PinnedBinary` installer to download. -pub async fn ensure_binary_installed( - binary_name: &str, - version: &str, - installer: PinnedBinary, -) -> Result<()> { - if is_command_installed( - binary_name, - Version::parse(version).context("Invalid version format")?, - ) { - debug!("{binary_name} version {version} is already installed"); - return Ok(()); - } - - debug!("Downloading installer for {binary_name}"); - - // Download the installer script to a temporary file (with sha256 verification) - let temp_file = NamedTempFile::new().context("Failed to create temporary file")?; - download_pinned_file(installer, temp_file.path()).await?; - - // Execute the installer script - let output = Command::new("sh") - .arg(temp_file.path()) - .output() - .context("Failed to execute installer command")?; - - if !output.status.success() { - bail!( - "Failed to install {binary_name} version {version}. Installer exited with output: {output:?}", - ); - } - - if !is_command_installed( - binary_name, - Version::parse(version).context("Invalid version format")?, - ) { - bail!( - "Could not veryfy installation of {binary_name} version {version} after running installer" - ); - } - - info!("Successfully installed {binary_name} version {version}"); - Ok(()) -} - -/// Check if the given command is installed and its version matches the expected version. -/// -/// Expects the command to support the `--version` flag and return a version string. -fn is_command_installed(command: &str, expected_version: Version) -> bool { - let is_command_installed = Command::new("which") - .arg(command) - .output() - .is_ok_and(|output| output.status.success()); - - if !is_command_installed { - debug!("{command} is not installed"); - return false; - } - - let Ok(version_output) = Command::new(command).arg("--version").output() else { - return false; - }; - - if !version_output.status.success() { - debug!( - "Failed to get command version. stderr: {}", - String::from_utf8_lossy(&version_output.stderr) - ); - return false; - } - - let version_string = String::from_utf8_lossy(&version_output.stdout); - let Ok(version) = versions::parse_from_output(&version_string) else { - return false; - }; - - debug!("Found {command} version: {version}"); - - versions::is_compatible(command, &version, &expected_version) -} diff --git a/src/binary_installer/versions.rs b/src/binary_installer/versions.rs deleted file mode 100644 index 4d12e7de7..000000000 --- a/src/binary_installer/versions.rs +++ /dev/null @@ -1,134 +0,0 @@ -use crate::prelude::*; -use semver::Version; - -/// Parse a version string from command output. -/// -/// Expects the output format to be: "command_name version_string" -/// Example: "codspeed-memtrack 4.4.2" -pub(super) fn parse_from_output(output: &str) -> Result { - let version_str = output - .split_once(" ") - .context("Unexpected version output format: missing space separator")? - .1 - .trim(); - - Version::parse(version_str) - .with_context(|| format!("Failed to parse version from: {version_str}")) -} - -/// Check if an installed version is compatible with the expected version. -/// -/// Returns true if the installed version is greater than or equal to the expected version. -/// Logs warnings for outdated or experimental versions. -pub(super) fn is_compatible(command: &str, installed: &Version, expected: &Version) -> bool { - match installed.cmp(expected) { - std::cmp::Ordering::Less => { - warn!( - "{command} is installed but the version is too old. expecting {expected} or higher but found installed: {installed}", - ); - false - } - std::cmp::Ordering::Greater => { - warn!( - "Using experimental {command} version {installed}. The recommended version is {expected}", - ); - true - } - std::cmp::Ordering::Equal => true, - } -} -#[cfg(test)] -mod tests { - use super::*; - - mod parse_version_from_output { - use super::*; - - #[test] - fn parses_valid_version() { - let output = "codspeed-memtrack 4.4.2"; - let version = parse_from_output(output).unwrap(); - assert_eq!(version, Version::new(4, 4, 2)); - } - - #[test] - fn parses_version_with_prerelease() { - let output = "codspeed-exec-harness 4.4.2-alpha.2"; - let version = parse_from_output(output).unwrap(); - assert_eq!(version.major, 4); - assert_eq!(version.minor, 4); - assert_eq!(version.patch, 2); - assert_eq!(version.pre.as_str(), "alpha.2"); - } - } - - mod is_version_compatible { - use super::*; - - #[test] - fn returns_true_for_equal_versions() { - let installed = Version::new(4, 4, 2); - let expected = Version::new(4, 4, 2); - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - #[test] - fn returns_true_for_newer_version() { - let installed = Version::new(4, 5, 0); - let expected = Version::new(4, 4, 2); - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - #[test] - fn returns_false_for_older_version() { - let installed = Version::new(4, 3, 0); - let expected = Version::new(4, 4, 2); - assert!(!is_compatible("test-cmd", &installed, &expected)); - } - - #[test] - fn handles_prerelease_versions() { - let installed = Version::parse("4.4.2-alpha.2").unwrap(); - let expected = Version::new(4, 4, 1); - // 4.4.2-alpha.2 > 4.4.1 because 4.4.2 > 4.4.1 - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - #[test] - fn prerelease_different_stage() { - { - let installed = Version::parse("4.4.2-alpha.2").unwrap(); - let expected = Version::new(4, 4, 2); - // 4.4.2-alpha.2 < 4.4.2 - assert!(!is_compatible("test-cmd", &installed, &expected)); - } - - { - let installed = Version::parse("4.4.2-beta.1").unwrap(); - let expected = Version::parse("4.4.2-alpha.1").unwrap(); - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - { - let installed = Version::new(4, 4, 2); - let expected = Version::parse("4.4.2-alpha.2").unwrap(); - // 4.4.2 > 4.4.2-alpha.2 - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - { - let installed = Version::parse("4.4.2-alpha.1").unwrap(); - let expected = Version::parse("4.4.2-beta.1").unwrap(); - assert!(!is_compatible("test-cmd", &installed, &expected)); - } - } - - #[test] - fn prerelease_same_stage() { - let installed = Version::parse("4.4.2-alpha.1").unwrap(); - let expected = Version::parse("4.4.2-alpha.2").unwrap(); - - assert!(!is_compatible("test-cmd", &installed, &expected)); - } - } -} diff --git a/src/binary_pins.rs b/src/binary_pins.rs index 2b2734a82..355d725e8 100644 --- a/src/binary_pins.rs +++ b/src/binary_pins.rs @@ -108,21 +108,6 @@ impl ValgrindTarget { } } -const MEMTRACK_INSTALLER: BinaryPin = BinaryPin { - version: "1.5.1", - url_template: "https://github.com/CodSpeedHQ/codspeed/releases/download/memtrack-v{version}/memtrack-installer.sh", - sha256: "47d529728d9e2a02fc0773c8ca0ece214f67cbc965d7ae327fe8c213ae2735a7", -}; -#[cfg(target_os = "linux")] -pub const MEMTRACK_VERSION: &str = MEMTRACK_INSTALLER.version; - -const EXEC_HARNESS_INSTALLER: BinaryPin = BinaryPin { - version: "1.3.0", - url_template: "https://github.com/CodSpeedHQ/codspeed/releases/download/exec-harness-v{version}/exec-harness-installer.sh", - sha256: "75cbff4fdaefe98927d24fff43fd600c621eb1263b0c40b0fd32c68fa6d88ebd", -}; -pub const EXEC_HARNESS_VERSION: &str = EXEC_HARNESS_INSTALLER.version; - const MONGO_TRACER_INSTALLER: BinaryPin = BinaryPin { version: "cs-mongo-tracer-v0.2.0", url_template: "https://codspeed-public-assets.s3.eu-west-1.amazonaws.com/mongo-tracer/{version}/cs-mongo-tracer-installer.sh", @@ -135,10 +120,6 @@ const MONGO_TRACER_INSTALLER: BinaryPin = BinaryPin { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum PinnedBinary { ValgrindDeb(ValgrindTarget), - // Only installed by the Linux-only memory executor. - #[cfg_attr(not(target_os = "linux"), allow(dead_code))] - MemtrackInstaller, - ExecHarnessInstaller, MongoTracerInstaller, } @@ -146,8 +127,6 @@ impl PinnedBinary { pub fn url(&self) -> String { match self { PinnedBinary::ValgrindDeb(target) => target.url(), - PinnedBinary::MemtrackInstaller => MEMTRACK_INSTALLER.url(), - PinnedBinary::ExecHarnessInstaller => EXEC_HARNESS_INSTALLER.url(), PinnedBinary::MongoTracerInstaller => MONGO_TRACER_INSTALLER.url(), } } @@ -155,8 +134,6 @@ impl PinnedBinary { pub fn sha256(&self) -> &'static str { match self { PinnedBinary::ValgrindDeb(target) => target.sha256(), - PinnedBinary::MemtrackInstaller => MEMTRACK_INSTALLER.sha256, - PinnedBinary::ExecHarnessInstaller => EXEC_HARNESS_INSTALLER.sha256, PinnedBinary::MongoTracerInstaller => MONGO_TRACER_INSTALLER.sha256, } } @@ -168,11 +145,7 @@ mod tests { use crate::cli::run::helpers::download_pinned_file; use tempfile::NamedTempFile; - const INSTALLER_BINARIES: &[PinnedBinary] = &[ - PinnedBinary::MemtrackInstaller, - PinnedBinary::ExecHarnessInstaller, - PinnedBinary::MongoTracerInstaller, - ]; + const INSTALLER_BINARIES: &[PinnedBinary] = &[PinnedBinary::MongoTracerInstaller]; const ALL_VALGRIND_TARGETS: &[ValgrindTarget] = &[ ValgrindTarget { @@ -196,9 +169,7 @@ mod tests { fn assert_installer_variant_is_listed(binary: PinnedBinary) { match binary { PinnedBinary::ValgrindDeb(_) => {} - PinnedBinary::MemtrackInstaller - | PinnedBinary::ExecHarnessInstaller - | PinnedBinary::MongoTracerInstaller => { + PinnedBinary::MongoTracerInstaller => { assert!(INSTALLER_BINARIES.contains(&binary)); } } @@ -214,8 +185,6 @@ mod tests { #[test] fn installer_variant_list_is_exhaustive() { - assert_installer_variant_is_listed(PinnedBinary::MemtrackInstaller); - assert_installer_variant_is_listed(PinnedBinary::ExecHarnessInstaller); assert_installer_variant_is_listed(PinnedBinary::MongoTracerInstaller); } diff --git a/src/cli/exec/multi_targets.rs b/src/cli/exec/multi_targets.rs index d24c16b93..0d511a6cf 100644 --- a/src/cli/exec/multi_targets.rs +++ b/src/cli/exec/multi_targets.rs @@ -1,5 +1,4 @@ use crate::executor::config::BenchmarkTarget; -use crate::executor::orchestrator::EXEC_HARNESS_COMMAND; use crate::prelude::*; use crate::project_config::{Target, TargetCommand, WalltimeOptions}; use exec_harness::BenchmarkCommand; @@ -69,8 +68,11 @@ pub fn build_benchmark_targets( .collect() } -/// Build a shell command string that pipes BenchmarkTarget::Exec variants to exec-harness via stdin +/// Build a shell command string that pipes BenchmarkTarget::Exec variants to exec-harness via stdin. +/// +/// `exec_harness` is the already shell-quoted invocation of exec-harness. pub fn build_exec_targets_pipe_command( + exec_harness: &str, targets: &[&crate::executor::config::BenchmarkTarget], ) -> Result { let inputs: Vec = targets @@ -92,9 +94,43 @@ pub fn build_exec_targets_pipe_command( .collect::>>()?; let json = serde_json::to_string(&inputs).context("Failed to serialize targets to JSON")?; - Ok(build_pipe_command_from_json(&json)) + Ok(build_pipe_command_from_json(exec_harness, &json)) +} + +fn build_pipe_command_from_json(exec_harness: &str, json: &str) -> String { + format!("{exec_harness} - <<'CODSPEED_EOF'\n{json}\nCODSPEED_EOF") } -fn build_pipe_command_from_json(json: &str) -> String { - format!("{EXEC_HARNESS_COMMAND} - <<'CODSPEED_EOF'\n{json}\nCODSPEED_EOF") +#[cfg(test)] +mod tests { + use super::*; + use crate::cli::exec_harness::ExecHarnessArgs; + use crate::cli::{InternalCommands, SELF_EXE_ENV_VAR}; + + /// The invocation is spliced into a string that `bash -c` runs, so a + /// self-exe path containing a space has to come back out as one word. + #[test] + fn exec_harness_invocation_survives_a_self_exe_path_with_spaces() { + temp_env::with_var(SELF_EXE_ENV_VAR, Some("/opt/my tools/codspeed"), || { + let invocation = InternalCommands::ExecHarness(ExecHarnessArgs { args: vec![] }) + .get_shell_command() + .unwrap(); + + assert_eq!( + shell_words::split(&invocation).unwrap(), + vec!["/opt/my tools/codspeed", "exec-harness"] + ); + }); + } + + /// The delimiter is quoted, so the shell expands nothing inside the body. + #[test] + fn pipe_command_wraps_the_payload_in_an_unexpanded_heredoc() { + let cmd = build_pipe_command_from_json("/bin/codspeed exec-harness", r#"{"a":"$HOME"}"#); + + assert_eq!( + cmd, + "/bin/codspeed exec-harness - <<'CODSPEED_EOF'\n{\"a\":\"$HOME\"}\nCODSPEED_EOF" + ); + } } diff --git a/src/cli/exec_harness.rs b/src/cli/exec_harness.rs new file mode 100644 index 000000000..f8a4b4d2b --- /dev/null +++ b/src/cli/exec_harness.rs @@ -0,0 +1,16 @@ +use crate::prelude::*; + +/// Run the bundled exec-harness. Arguments after `exec-harness` are forwarded +/// verbatim to exec-harness's own CLI parser. +#[derive(Debug, clap::Args)] +pub struct ExecHarnessArgs { + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + pub args: Vec, +} + +pub fn run(args: ExecHarnessArgs) -> Result<()> { + // exec-harness's own clap parser expects its name as `argv[0]`, not ours. + let argv = std::iter::once(std::ffi::OsString::from("exec-harness")).chain(args.args); + + ::exec_harness::cli::run_cli(argv) +} diff --git a/src/cli/memtrack.rs b/src/cli/memtrack.rs new file mode 100644 index 000000000..686d7692e --- /dev/null +++ b/src/cli/memtrack.rs @@ -0,0 +1,19 @@ +use crate::prelude::*; + +/// Run the bundled memtrack. Arguments after `memtrack` are forwarded verbatim +/// to memtrack's own CLI parser. +#[derive(Debug, clap::Args)] +pub struct MemtrackArgs { + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + pub args: Vec, +} + +pub fn run(args: MemtrackArgs) -> Result<()> { + // memtrack's own clap parser expects its name as `argv[0]`, not ours. + let argv = std::iter::once(std::ffi::OsString::from("memtrack")).chain(args.args); + + // memtrack's exit code is the tracked command's own, and the runner reads + // it to decide whether the benchmark failed, so it has to become ours. + let code = ::memtrack::cli::run_cli(argv)?; + std::process::exit(code); +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 2a9218ddc..3ebaccd0e 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -1,6 +1,9 @@ mod auth; pub(crate) mod exec; +pub(crate) mod exec_harness; pub(crate) mod experimental; +#[cfg(target_os = "linux")] +pub(crate) mod memtrack; mod profile; pub(crate) mod run; pub(crate) mod samply; @@ -110,37 +113,96 @@ pub(crate) enum InternalCommands { /// Run the bundled samply profiler. Args are forwarded to samply. #[command(disable_help_flag = true, disable_help_subcommand = true)] Samply(samply::SamplyArgs), + /// Run the bundled exec-harness. Args are forwarded to exec-harness. + #[command(disable_help_flag = true, disable_help_subcommand = true)] + ExecHarness(exec_harness::ExecHarnessArgs), + /// Run the bundled memtrack. Args are forwarded to memtrack. + /// + /// Linux-only, like the memory executor that drives it: memtrack is an eBPF + /// tracker and is not built at all on other platforms. + #[cfg(target_os = "linux")] + #[command(disable_help_flag = true, disable_help_subcommand = true)] + Memtrack(memtrack::MemtrackArgs), } -/// Overrides the executable used to re-invoke internal subcommands. +/// Test-only override for the executable that internal subcommands are +/// re-invoked through. Under `cargo test` [`std::env::current_exe`] is the test +/// harness, which rejects their arguments. /// -/// [`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. +/// Deliberately `cfg(test)`: this path is handed to `sudo setcap +ep`, so +/// honouring it in a release build would let anyone who can set one environment +/// variable pick which file receives `CAP_SYS_ADMIN`. +#[cfg(test)] pub(crate) const SELF_EXE_ENV_VAR: &str = "CODSPEED_SELF_EXE"; +/// The executable that internal subcommands are re-invoked through. +/// +/// Exposed separately from [`InternalCommands::get_command_builder`] because the +/// memory executor grants file capabilities to this exact path before running +/// it, and `setcap` on a path that is not the one later exec'd succeeds while +/// changing nothing. +pub(crate) fn self_exe() -> Result { + #[cfg(test)] + if let Some(path) = std::env::var_os(SELF_EXE_ENV_VAR) { + return Ok(PathBuf::from(path)); + } + + std::env::current_exe().context("failed to resolve current executable for internal subcommand") +} + 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 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); + let mut builder = CommandBuilder::new(self_exe()?); match self { InternalCommands::Samply(args) => { builder.arg("samply"); builder.args(args.args.iter().cloned()); } + InternalCommands::ExecHarness(args) => { + builder.arg("exec-harness"); + builder.args(args.args.iter().cloned()); + } + #[cfg(target_os = "linux")] + InternalCommands::Memtrack(args) => { + builder.arg("memtrack"); + builder.args(args.args.iter().cloned()); + } } Ok(builder) } + + /// The same re-exec as a single POSIX-shell command string, for the call + /// sites that splice it into a script rather than spawning it: exec-harness + /// is handed its targets through a heredoc. + pub fn get_shell_command(&self) -> Result { + Ok(self.get_command_builder()?.as_command_line()) + } +} + +/// Dispatch a bundled subcommand. +/// +/// These are a re-exec of this binary and share nothing with the runner: no +/// profile, no project config, no API client, no logger. They also run in the +/// benchmark's working directory, so anything the runner discovers from the +/// filesystem could abort a measurement for a reason unrelated to it. +fn run_internal(command: InternalCommands) -> Result<()> { + match command { + InternalCommands::Samply(args) => samply::run(args), + InternalCommands::ExecHarness(args) => exec_harness::run(args), + #[cfg(target_os = "linux")] + InternalCommands::Memtrack(args) => memtrack::run(args), + } } pub async fn run() -> Result<()> { let cli = Cli::parse(); + + if let Commands::Internal(command) = cli.command { + return run_internal(command); + } + let codspeed_config = load_config(&cli)?; let mut api_client = build_api_client(&cli, &codspeed_config); @@ -158,7 +220,8 @@ pub async fn run() -> Result<()> { let setup_cache_dir = setup_cache_dir.as_deref(); match cli.command { - Commands::Run(_) | Commands::Exec(_) | Commands::Internal(InternalCommands::Samply(_)) => {} // these are responsible for their own logger initialization + // These initialize their own logging. + Commands::Run(_) | Commands::Exec(_) => {} _ => { init_local_logger()?; } @@ -210,7 +273,9 @@ pub async fn run() -> Result<()> { Commands::Use(args) => use_mode::run(args)?, Commands::Show => show::run()?, Commands::Update => update::run().await?, - Commands::Internal(InternalCommands::Samply(args)) => samply::run(args)?, + Commands::Internal(_) => { + unreachable!("internal subcommands are dispatched before runner setup") + } } Ok(()) } diff --git a/src/executor/config.rs b/src/executor/config.rs index 39f958510..bc2584de2 100644 --- a/src/executor/config.rs +++ b/src/executor/config.rs @@ -193,12 +193,23 @@ impl OrchestratorConfig { /// Produce a per-execution [`ExecutorConfig`] for the given command and mode. /// - /// `enable_introspection` controls whether language-level wrappers (Node.js, Go) - /// are injected into `PATH`. This should be `false` for exec-harness targets. + /// `uses_exec_harness` says whether this run is driven by exec-harness + /// rather than being a plain entrypoint command. Two things follow from it: + /// + /// - Language-level wrappers (Node.js, Go) are injected into `PATH` only for + /// entrypoint runs. + /// - Subprocess tracking is forced on for exec-harness runs, because + /// exec-harness toggles instrumentation in its own process and then forks + /// the benchmark. The child is measured only if valgrind propagates that + /// state across `fork`/`exec`, which `--instr-atstart=inherit` enables; + /// with `no` it dumps a single zero-cost part and reports nothing. + /// + /// Entrypoint runs must keep the opposite default, or a benchmark that forks + /// would silently start counting its children. pub fn executor_config_for_command( &self, command: String, - enable_introspection: bool, + uses_exec_harness: bool, ) -> ExecutorConfig { ExecutorConfig { working_directory: self.working_directory.clone(), @@ -212,11 +223,11 @@ impl OrchestratorConfig { allow_empty: self.allow_empty, go_runner_version: self.go_runner_version.clone(), extra_env: self.extra_env.clone(), - enable_introspection, + enable_introspection: !uses_exec_harness, fair_sched: self.fair_sched, cycle_estimation: self.cycle_estimation, exclude_allocations: self.exclude_allocations, - simulation_track_subprocess: self.simulation_track_subprocess, + simulation_track_subprocess: self.simulation_track_subprocess || uses_exec_harness, memory_track_physical: self.memory_track_physical, } } @@ -262,7 +273,7 @@ impl OrchestratorConfig { impl ExecutorConfig { /// Constructs a new `ExecutorConfig` with default values for testing purposes pub fn test() -> Self { - OrchestratorConfig::test().executor_config_for_command("".into(), true) + OrchestratorConfig::test().executor_config_for_command("".into(), false) } } diff --git a/src/executor/helpers/command.rs b/src/executor/helpers/command.rs index 345f96975..72f9b4d15 100644 --- a/src/executor/helpers/command.rs +++ b/src/executor/helpers/command.rs @@ -128,7 +128,12 @@ impl CommandBuilder { self } - /// Returns the command line as a string for debugging/testing purposes + /// The command line as a single shell-quoted string. + /// + /// Used for logging and assertions, and by + /// [`InternalCommands::get_shell_command`](crate::cli::InternalCommands::get_shell_command) + /// to splice a re-exec into a command that `bash -c` runs — so the quoting + /// has to stay correct, not merely readable. pub fn as_command_line(&self) -> String { let mut parts: Vec = vec![self.program.to_string_lossy().into_owned()]; parts.extend( diff --git a/src/executor/memory/executor.rs b/src/executor/memory/executor.rs index 6cbb0f97f..f9b71e590 100644 --- a/src/executor/memory/executor.rs +++ b/src/executor/memory/executor.rs @@ -1,3 +1,5 @@ +use crate::cli::InternalCommands; +use crate::cli::memtrack::MemtrackArgs; use crate::executor::ExecutorName; use crate::executor::ExecutorSupport; use crate::executor::PrivilegeStatus; @@ -61,8 +63,9 @@ impl MemoryExecutor { let bench_command = get_bench_command(&execution_context.config)?; let (bench_command, env_file) = prefix_command_with_env(&bench_command, &extra_env)?; - // Build the memtrack command - let mut cmd_builder = CommandBuilder::new(MEMTRACK_COMMAND); + // memtrack is a hidden subcommand of this binary: re-exec ourselves. + let mut cmd_builder = + InternalCommands::Memtrack(MemtrackArgs { args: vec![] }).get_command_builder()?; if execution_context.config.memory_track_physical { cmd_builder.env("CODSPEED_MEMTRACK_TRACK_PHYSICAL", "1"); } diff --git a/src/executor/memory/setup.rs b/src/executor/memory/setup.rs index e393e8b1b..e3d67e412 100644 --- a/src/executor/memory/setup.rs +++ b/src/executor/memory/setup.rs @@ -1,15 +1,13 @@ -use crate::binary_installer::ensure_binary_installed; -use crate::binary_pins::{self, PinnedBinary}; +use crate::cli::self_exe; use crate::executor::helpers::capabilities::binary_has_capabilities; use crate::executor::helpers::run_with_sudo::{is_root_user, run_with_sudo}; use crate::executor::{ToolInstallStatus, ToolStatus}; use crate::prelude::*; use caps::Capability; use std::path::PathBuf; -use std::process::Command; -pub const MEMTRACK_COMMAND: &str = "codspeed-memtrack"; -pub const MEMTRACK_CODSPEED_VERSION: &str = binary_pins::MEMTRACK_VERSION; +/// How memtrack is named in user-facing messages. +pub const MEMTRACK_COMMAND: &str = "memtrack"; const MEMTRACK_REQUIRED_CAPS: &[Capability] = &[ Capability::CAP_DAC_READ_SEARCH, @@ -28,7 +26,7 @@ fn memtrack_required_caps_mask() -> u64 { /// `setcap` grammar form of [`MEMTRACK_REQUIRED_CAPS`]: the lowercase cap names /// (libcap renders them lowercase) joined with commas and the `+ep` /// effective+permitted flag. Derived from the enum so the two never drift. -fn memtrack_setcap_spec() -> String { +pub(crate) fn memtrack_setcap_spec() -> String { let caps = MEMTRACK_REQUIRED_CAPS .iter() .map(|c| c.to_string().to_lowercase()) @@ -37,8 +35,14 @@ fn memtrack_setcap_spec() -> String { format!("{caps}+ep") } +/// The binary that must carry the eBPF capabilities: memtrack runs as a +/// subcommand of this executable, so it is this one. +/// +/// Every invocation of the CLI therefore carries [`MEMTRACK_REQUIRED_CAPS`], +/// `CAP_SYS_ADMIN` included. They are granted `+ep` and not inheritable, so a +/// spawned benchmark does not receive them: the elevation stops here. fn memtrack_path() -> Option { - which::which(MEMTRACK_COMMAND).ok() + self_exe().ok() } /// Whether the installed memtrack binary already carries the required capabilities. @@ -94,73 +98,19 @@ pub fn ensure_memtrack_capabilities() -> Result<()> { } pub fn get_memtrack_status() -> ToolStatus { - let tool_name = MEMTRACK_COMMAND.to_string(); - - let is_available = Command::new("which") - .arg(MEMTRACK_COMMAND) - .output() - .is_ok_and(|output| output.status.success()); - if !is_available { - return ToolStatus { - tool_name, - status: ToolInstallStatus::NotInstalled, - }; - } - - let Ok(version_output) = Command::new(MEMTRACK_COMMAND).arg("--version").output() else { - return ToolStatus { - tool_name, - status: ToolInstallStatus::NotInstalled, - }; - }; - - if !version_output.status.success() { - return ToolStatus { - tool_name, - status: ToolInstallStatus::NotInstalled, - }; - } - - let version = String::from_utf8_lossy(&version_output.stdout) - .trim() - .to_string(); - - // Parse the version number from output like "memtrack 1.2.2" - let expected = semver::Version::parse(MEMTRACK_CODSPEED_VERSION).unwrap(); - if let Some(version_str) = version.split_once(' ').map(|(_, v)| v.trim()) { - if let Ok(installed) = semver::Version::parse(version_str) { - if installed < expected { - return ToolStatus { - tool_name, - status: ToolInstallStatus::IncorrectVersion { - version, - message: format!( - "version too old, expecting {MEMTRACK_CODSPEED_VERSION} or higher", - ), - }, - }; - } - return ToolStatus { - tool_name, - status: ToolInstallStatus::Installed { version }, - }; - } - } - + // memtrack ships inside this binary, so it is installed by construction + // and carries this crate's version. ToolStatus { - tool_name, - status: ToolInstallStatus::IncorrectVersion { - version, - message: "could not parse version".to_string(), + tool_name: MEMTRACK_COMMAND.to_string(), + status: ToolInstallStatus::Installed { + version: env!("CARGO_PKG_VERSION").to_string(), }, } } +/// No-op: memtrack is part of this binary. Kept so the setup flow can treat it +/// like the tools that do need installing. pub async fn install_memtrack() -> Result<()> { - ensure_binary_installed( - MEMTRACK_COMMAND, - MEMTRACK_CODSPEED_VERSION, - PinnedBinary::MemtrackInstaller, - ) - .await + debug!("{MEMTRACK_COMMAND} is bundled into this binary, nothing to install"); + Ok(()) } diff --git a/src/executor/orchestrator.rs b/src/executor/orchestrator.rs index ca2dbdf4f..cdf082120 100644 --- a/src/executor/orchestrator.rs +++ b/src/executor/orchestrator.rs @@ -1,8 +1,8 @@ use super::{ExecutionContext, ExecutorName, get_executor_from_mode, run_executor}; use crate::api_client::CodSpeedAPIClient; -use crate::binary_installer::ensure_binary_installed; -use crate::binary_pins::{self, PinnedBinary}; +use crate::cli::InternalCommands; use crate::cli::exec::multi_targets; +use crate::cli::exec_harness::ExecHarnessArgs; use crate::cli::run::logger::Logger; use crate::executor::config::BenchmarkTarget; use crate::executor::config::OrchestratorConfig; @@ -17,9 +17,6 @@ use serde_json::Value; use std::collections::BTreeMap; use std::path::{Path, PathBuf}; -pub const EXEC_HARNESS_COMMAND: &str = "exec-harness"; -pub const EXEC_HARNESS_VERSION: &str = binary_pins::EXEC_HARNESS_VERSION; - /// Shared orchestration state created once per CLI invocation. /// /// Holds the run-level configuration, environment provider, system info, and logger. @@ -82,14 +79,12 @@ impl Orchestrator { .collect(); if !exec_targets.is_empty() { - ensure_binary_installed( - EXEC_HARNESS_COMMAND, - EXEC_HARNESS_VERSION, - PinnedBinary::ExecHarnessInstaller, - ) - .await?; + // exec-harness is a hidden subcommand of this binary: re-exec ourselves. + let exec_harness = InternalCommands::ExecHarness(ExecHarnessArgs { args: vec![] }) + .get_shell_command()?; - let pipe_cmd = multi_targets::build_exec_targets_pipe_command(&exec_targets)?; + let pipe_cmd = + multi_targets::build_exec_targets_pipe_command(&exec_harness, &exec_targets)?; let label = match exec_targets.as_slice() { [BenchmarkTarget::Exec { command, .. }] => { format!("Running `{}` with exec-harness", command.join(" ")) @@ -143,7 +138,7 @@ impl Orchestrator { for (run_part_index, part) in run_parts.into_iter().enumerate() { let config = self .config - .executor_config_for_command(part.command, !part.uses_exec_harness); + .executor_config_for_command(part.command, part.uses_exec_harness); let mut executor = get_executor_from_mode(part.mode, self.config.walltime_profiler); let profile_folder = self.resolve_profile_folder(&executor.name(), run_part_index, total_parts)?; diff --git a/src/executor/tests.rs b/src/executor/tests.rs index 65507fcac..d18bd520c 100644 --- a/src/executor/tests.rs +++ b/src/executor/tests.rs @@ -175,6 +175,29 @@ fi (execution_context, temp_dir) } + /// Serializes every executor test that drives a benchmark through the runner + /// FIFOs: the walltime and memory tests. + /// + /// `RUNNER_CTL_FIFO` and `RUNNER_ACK_FIFO` are fixed absolute paths shared + /// with the integrations, and `RunnerFifo::new` unlinks and recreates both. + /// Two overlapping executions therefore pull the FIFO out from under each + /// other: the first keeps its fds on a now-unlinked inode, the second's + /// child opens the replacement by path, and they never meet again. Nothing + /// there times out, so the test hangs forever instead of failing. + /// + /// The race exists outside the tests too, where this lock cannot reach it: + /// COD-3560. + pub static RUNNER_FIFO_LOCK: OnceCell = OnceCell::const_new(); + + pub async fn acquire_runner_fifo_lock() -> SemaphorePermit<'static> { + RUNNER_FIFO_LOCK + .get_or_init(|| async { Semaphore::new(1) }) + .await + .acquire() + .await + .unwrap() + } + // Uprobes set by memtrack, lead to crashes in valgrind because they work by setting breakpoints on the first // instruction. Valgrind doesn't rethrow those breakpoint exceptions, which makes the test crash. // @@ -256,14 +279,10 @@ mod walltime { use crate::executor::wall_time::executor::WallTimeExecutor; async fn get_walltime_executor() -> (SemaphorePermit<'static>, WallTimeExecutor) { - static WALLTIME_SEMAPHORE: OnceCell = OnceCell::const_new(); - // We can't execute multiple walltime executors in parallel because perf isn't thread-safe (yet). We have to - // use a semaphore to limit concurrent access. - let semaphore = WALLTIME_SEMAPHORE - .get_or_init(|| async { Semaphore::new(1) }) - .await; - let permit = semaphore.acquire().await.unwrap(); + // use a semaphore to limit concurrent access. The same permit also + // excludes the memory tests, which share the global runner FIFOs. + let permit = acquire_runner_fifo_lock().await; let executor = WallTimeExecutor::new(None); let system_info = SystemInfo::new().unwrap(); @@ -445,7 +464,9 @@ fi #[cfg(target_os = "linux")] mod memory { use super::helpers::*; + use crate::executor::helpers::run_with_sudo::{can_elevate_without_prompt, is_root_user}; use crate::executor::memory::executor::MemoryExecutor; + use crate::executor::memory::setup::{has_memtrack_capabilities, memtrack_setcap_spec}; async fn get_memory_executor() -> ( SemaphorePermit<'static>, @@ -453,23 +474,47 @@ mod memory { MemoryExecutor, ) { static MEMORY_INIT: OnceCell<()> = OnceCell::const_new(); - static MEMORY_SEMAPHORE: OnceCell = OnceCell::const_new(); MEMORY_INIT .get_or_init(|| async { - let executor = MemoryExecutor; - let system_info = SystemInfo::new().unwrap(); - executor.setup(&system_info, None).await.unwrap(); - executor.grant_privileges().unwrap(); + // `grant_privileges` setcaps the binary memtrack is run from, + // i.e. `current_exe`. Without the override that is the test + // harness, so the capabilities would land on a throwaway binary + // and the run would still lack them. + let self_exe = codspeed_binary_path().await; + temp_env::async_with_vars(&[(SELF_EXE_ENV_VAR, Some(self_exe))], async { + // `setcap` goes through sudo, and a sudo password prompt + // under `cargo test` is buried in captured output while the + // read blocks forever: the suite hangs with nothing to say + // what it is waiting for. Check first, fail with the command + // to run. This bites on every rebuild, not once, because + // file capabilities are an xattr and cargo writes a new + // binary on each relink. + let needs_grant = !is_root_user() && !has_memtrack_capabilities(); + assert!( + !needs_grant || can_elevate_without_prompt(), + "The memory tests have to `setcap` {self_exe}, and sudo would prompt for a \ + password here -- a prompt `cargo test` hides and then blocks on forever.\n\ + Cache the credentials first (`sudo -v && cargo test ...`), or grant them \ + by hand:\n sudo setcap {} {self_exe}", + memtrack_setcap_spec(), + ); + + let executor = MemoryExecutor; + let system_info = SystemInfo::new().unwrap(); + executor.setup(&system_info, None).await.unwrap(); + executor.grant_privileges().unwrap(); + }) + .await; }) .await; - let semaphore = MEMORY_SEMAPHORE - .get_or_init(|| async { Semaphore::new(1) }) - .await; - let permit = semaphore.acquire().await.unwrap(); + let permit = acquire_runner_fifo_lock().await; // Memory executor uses heaptrack which uses BPF-based instrumentation, which conflicts with valgrind. + // + // Lock order: always after the FIFO permit. No other test takes both, + // so the two can never be wanted in opposite orders. let _lock = acquire_bpf_instrumentation_lock().await; (permit, _lock, MemoryExecutor) @@ -487,12 +532,19 @@ mod memory { async fn test_memory_executor(#[case] cmd: &str) { let (_permit, _lock, mut executor) = get_memory_executor().await; + // The executor re-execs `current_exe` to reach the memtrack subcommand, + // and under `cargo test` that is the test harness. Point it at the + // real binary. + 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 = memory_config(cmd); - 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 = memory_config(cmd); + let (execution_context, _temp_dir) = create_test_setup(config).await; + executor.run(&execution_context, &None).await.unwrap(); + }, + ) .await; } @@ -502,8 +554,13 @@ mod memory { let (_permit, _lock, mut executor) = get_memory_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 = memory_config(&cmd); @@ -533,9 +590,16 @@ fi let (execution_context, _temp_dir) = create_test_setup(config).await; let (_permit, _lock, mut executor) = get_memory_executor().await; - temp_env::async_with_vars(&[("PATH", Some(&modified_path))], async { - executor.run(&execution_context, &None).await.unwrap(); - }) + let self_exe = codspeed_binary_path().await; + temp_env::async_with_vars( + &[ + ("PATH", Some(modified_path.as_str())), + (SELF_EXE_ENV_VAR, Some(self_exe)), + ], + async { + executor.run(&execution_context, &None).await.unwrap(); + }, + ) .await; } @@ -564,9 +628,16 @@ fi let (execution_context, _temp_dir) = create_test_setup(config).await; let (_permit, _lock, mut executor) = get_memory_executor().await; - temp_env::async_with_vars(&[("LD_LIBRARY_PATH", Some(&modified))], async { - executor.run(&execution_context, &None).await.unwrap(); - }) + let self_exe = codspeed_binary_path().await; + temp_env::async_with_vars( + &[ + ("LD_LIBRARY_PATH", Some(modified.as_str())), + (SELF_EXE_ENV_VAR, Some(self_exe)), + ], + async { + executor.run(&execution_context, &None).await.unwrap(); + }, + ) .await; } } diff --git a/src/lib.rs b/src/lib.rs index fe86e3e32..c2926fae1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,7 +1,6 @@ //! CodSpeed Runner library mod api_client; -mod binary_installer; mod binary_pins; pub mod cli; mod config; diff --git a/src/run_environment/local/provider.rs b/src/run_environment/local/provider.rs index ab67f69b5..7370e8e78 100644 --- a/src/run_environment/local/provider.rs +++ b/src/run_environment/local/provider.rs @@ -425,32 +425,35 @@ mod tests { assert_eq!(FAKE_COMMIT_REF.len(), 40); } - fn create_git_repo_with_remote(dir: &std::path::Path, remote_url: &str) -> String { - std::process::Command::new("git") - .args(["init", "-b", "main"]) - .current_dir(dir) - .output() - .unwrap(); - std::process::Command::new("git") - .args(["config", "user.email", "test@test.com"]) - .current_dir(dir) - .output() - .unwrap(); - std::process::Command::new("git") - .args(["config", "user.name", "Test"]) - .current_dir(dir) - .output() - .unwrap(); - std::process::Command::new("git") - .args(["remote", "add", "origin", remote_url]) - .current_dir(dir) - .output() - .unwrap(); - std::process::Command::new("git") - .args(["commit", "--allow-empty", "-m", "init"]) + /// Run one `git` command in `dir`, failing loudly if it does not succeed. + /// + /// `.output().unwrap()` would only unwrap the *spawn*, letting a git that + /// runs and exits non-zero pass silently — a failed setup then surfaces + /// several frames later as an `UnbornBranch` error on `refs/heads/main`, + /// which says nothing about the cause. + fn git(dir: &std::path::Path, args: &[&str]) { + let output = std::process::Command::new("git") + .args(args) .current_dir(dir) .output() - .unwrap(); + .unwrap_or_else(|e| panic!("failed to spawn `git {}`: {e}", args.join(" "))); + + assert!( + output.status.success(), + "`git {}` failed with {}\nstdout: {}\nstderr: {}", + args.join(" "), + output.status, + String::from_utf8_lossy(&output.stdout).trim(), + String::from_utf8_lossy(&output.stderr).trim(), + ); + } + + fn create_git_repo_with_remote(dir: &std::path::Path, remote_url: &str) -> String { + git(dir, &["init", "-b", "main"]); + git(dir, &["config", "user.email", "test@test.com"]); + git(dir, &["config", "user.name", "Test"]); + git(dir, &["remote", "add", "origin", remote_url]); + git(dir, &["commit", "--allow-empty", "-m", "init"]); format!("{}/", dir.to_string_lossy()) }