From d9612e39afc89b405b31d2849bb81393acb43b62 Mon Sep 17 00:00:00 2001 From: not-matthias Date: Thu, 22 Oct 2026 16:33:06 +0200 Subject: [PATCH] fix(valgrind): stop the libpython symlink warning from firing falsely The venv compat script treated any python it found as a venv. With a uv shim in ~/.local/bin the link target directory did not exist, and for a non-venv python it tried to link a file onto itself, so `ln` failed and the runner warned although benchmarks ran fine. The script now skips pythons that are not in a virtual environment, creates the lib directory, and uses `ln -sfn` so a dangling link left by a moved or upgraded uv python is replaced instead of failing with "File exists". The warning keeps its level but now includes the script error and says that flamegraphs may contain artifacts. Fixes COD-3977 --- src/executor/valgrind/executor.rs | 5 +- src/executor/valgrind/helpers/venv_compat.rs | 95 ++++++++++++++++++++ src/executor/valgrind/helpers/venv_compat.sh | 24 +++-- 3 files changed, 114 insertions(+), 10 deletions(-) diff --git a/src/executor/valgrind/executor.rs b/src/executor/valgrind/executor.rs index aa28c8c8b..67a726c9a 100644 --- a/src/executor/valgrind/executor.rs +++ b/src/executor/valgrind/executor.rs @@ -57,8 +57,9 @@ impl Executor for ValgrindExecutor { } if let Err(error) = venv_compat::symlink_libpython(None) { - warn!("Failed to symlink libpython"); - debug!("Script error: {error}"); + warn!( + "Failed to symlink libpython into the virtual environment, flamegraphs may contain artifacts: {error}" + ); } Ok(()) diff --git a/src/executor/valgrind/helpers/venv_compat.rs b/src/executor/valgrind/helpers/venv_compat.rs index d1d6b660b..1aae87769 100644 --- a/src/executor/valgrind/helpers/venv_compat.rs +++ b/src/executor/valgrind/helpers/venv_compat.rs @@ -50,6 +50,101 @@ pub fn symlink_libpython(cwd: Option<&String>) -> anyhow::Result<()> { #[cfg(test)] mod tests { use super::*; + use std::{fs, os::unix::fs::symlink, path::Path}; + + const LIBPYTHON: &str = "libpython3.13.so.1.0"; + + fn write_executable(path: &Path, content: &str) { + fs::write(path, content).unwrap(); + fs::set_permissions(path, fs::Permissions::from_mode(0o755)).unwrap(); + } + + /// Runs the script with a fake `uv` and `ldd` first in `PATH`. `uv python find` reports + /// `uv_python`, and `ldd` always reports `LIBPYTHON` as missing. + fn run_script(root: &Path, uv_python: &Path) -> std::process::Output { + let fake_bin = root.join("fake-bin"); + fs::create_dir_all(&fake_bin).unwrap(); + write_executable( + &fake_bin.join("uv"), + &format!("#!/usr/bin/env bash\necho {}\n", uv_python.display()), + ); + write_executable( + &fake_bin.join("ldd"), + &format!("#!/usr/bin/env bash\necho '{LIBPYTHON} => not found'\n"), + ); + // Without a `python3` in the fake bin, the script would inspect the host's python. + symlink(uv_python, fake_bin.join("python3")).unwrap(); + + let path = format!("{}:{}", fake_bin.display(), std::env::var("PATH").unwrap()); + Command::new("bash") + .arg("-c") + .arg(VENV_COMPAT_SCRIPT) + .env("PATH", path) + .output() + .unwrap() + } + + /// Creates `real/bin/python3` and a `venv/bin/python3` pointing at it. + fn create_venv(root: &Path) -> std::path::PathBuf { + fs::create_dir_all(root.join("real/bin")).unwrap(); + fs::create_dir_all(root.join("venv/bin")).unwrap(); + fs::write(root.join("real/bin/python3"), "").unwrap(); + fs::write(root.join("venv/pyvenv.cfg"), "").unwrap(); + symlink(root.join("real/bin/python3"), root.join("venv/bin/python3")).unwrap(); + root.join("venv/bin/python3") + } + + #[test] + fn test_creates_missing_lib_dir_and_link() { + let dir = tempfile::tempdir().unwrap(); + let venv_python = create_venv(dir.path()); + + let output = run_script(dir.path(), &venv_python); + + assert!(output.status.success(), "{output:?}"); + let link = dir.path().join("venv/lib").join(LIBPYTHON); + assert_eq!( + fs::read_link(link).unwrap(), + dir.path() + .canonicalize() + .unwrap() + .join("real/lib") + .join(LIBPYTHON) + ); + } + + #[test] + fn test_replaces_dangling_link() { + let dir = tempfile::tempdir().unwrap(); + let venv_python = create_venv(dir.path()); + fs::create_dir_all(dir.path().join("venv/lib")).unwrap(); + let link = dir.path().join("venv/lib").join(LIBPYTHON); + symlink(dir.path().join("moved-python/lib").join(LIBPYTHON), &link).unwrap(); + + let output = run_script(dir.path(), &venv_python); + + assert!(output.status.success(), "{output:?}"); + assert_eq!( + fs::read_link(link).unwrap(), + dir.path() + .canonicalize() + .unwrap() + .join("real/lib") + .join(LIBPYTHON) + ); + } + + #[test] + fn test_skips_python_outside_a_venv() { + let dir = tempfile::tempdir().unwrap(); + let venv_python = create_venv(dir.path()); + fs::remove_file(dir.path().join("venv/pyvenv.cfg")).unwrap(); + + let output = run_script(dir.path(), &venv_python); + + assert!(output.status.success(), "{output:?}"); + assert!(!dir.path().join("venv/lib").exists()); + } // Only run in Github Actions, to ensure python is dynamically linked. #[test_with::env(GITHUB_ACTIONS)] diff --git a/src/executor/valgrind/helpers/venv_compat.sh b/src/executor/valgrind/helpers/venv_compat.sh index fb5ac21eb..e4cfbb0b3 100755 --- a/src/executor/valgrind/helpers/venv_compat.sh +++ b/src/executor/valgrind/helpers/venv_compat.sh @@ -3,6 +3,14 @@ function add_symlink() { local venv_python="$1" + venv_path="$(dirname "$(dirname "$venv_python")")" + # Only virtual environments resolve libpython relative to themselves; any other Python + # (e.g. a uv shim in ~/.local/bin) has nowhere to put the link. + if [ ! -f "$venv_path/pyvenv.cfg" ]; then + echo "Not a virtual environment, skipping: $venv_python" + return 0 + fi + system_python="$(readlink -f "$venv_python")" if [ -z "$system_python" ]; then echo "Error: Failed to resolve real path for $venv_python" >&2 @@ -10,7 +18,6 @@ function add_symlink() { fi system_path="$(dirname $(dirname "$system_python"))" - venv_path="$(dirname $(dirname "$venv_python"))" echo "Python installation (system): $system_path" echo "Python installation (venv): $venv_path" @@ -26,13 +33,14 @@ function add_symlink() { system_link="$system_path/lib/$libpython_name" if [ -e "$venv_link" ]; then echo "Symlink already exists: $venv_link" - else - echo "Creating symlink: $system_link -> $venv_link" - ln -s "$system_link" "$venv_link" - if [ $? -ne 0 ]; then - echo "Error: Failed to create symlink $venv_link" >&2 - return 1 - fi + return 0 + fi + + # `-f` replaces a dangling link left behind when the venv's Python was moved or upgraded. + echo "Creating symlink: $system_link -> $venv_link" + if ! mkdir -p "$venv_path/lib" || ! ln -sfn "$system_link" "$venv_link"; then + echo "Error: Failed to create symlink $venv_link" >&2 + return 1 fi }