fix(valgrind): cache the source build in the setup cache

[trim21]

The runner-side half of CodSpeedHQ/action#248. Once the cache directory exists, the action-side symptom (the instruments cache step reporting that it has nothing to save) goes away with it, but the rest of that issue is separate from this change.

Today

install_valgrind_from_package is the only caller that hands setup_cache_dir to apt::install_cached, so the instruments cache only ever holds the files of apt-installed packages. On a system we publish no valgrind .deb for — ubuntu-26.04 right now, any rolling release or non-apt distribution, and the next new Ubuntu release — ValgrindExecutor::setup takes try_install_from_source, which never receives setup_cache_dir:

##[warning]CodSpeed does not publish a valgrind package for ubuntu 26.04
Building valgrind-codspeed from source, this can take a few minutes
...
valgrind-codspeed has been built and installed from source

Every run repeats that build (about 150 s on a GitHub-hosted runner), and since nothing is ever written to ~/.cache/codspeed-action, the cache directory never exists and the action's cache step can only report that there is nothing to save.

Change

• src/executor/helpers/apt.rs: the copy-to-root step of restore_from_cache is extracted as restore_files_to_root. The existing entry point keeps its apt guard and its metadata logging, so the package path is unchanged; the source-build path needs the same copy without that guard, because the distributions that take it are exactly the ones the guard skips.
• src/executor/valgrind/build_from_source.rs: install_build stages into the cache with make install DESTDIR=<cache_dir> (no root needed) and then installs from there through the shared copy, which is what puts the staged files in the root-relative layout a later restore expects.
• src/executor/valgrind/setup.rs: try_install_from_source restores the cache before the version check, so a warm cache returns the same way an already-installed valgrind does, and the "we publish no package for this system" warning is only logged when a build or a manual install really is next.
• src/executor/valgrind/executor.rs: passes setup_cache_dir through.

Verification

• cargo fmt --all -- --check and cargo clippy --all-targets -- -D warnings are clean.
• cargo test --all --exclude memtrack --exclude exec-harness fails the same 23 tests as the base commit, with an identical failure list (all of them read testdata/perf_map/*, git-lfs pointers in my checkout): no new failures. A 24th failure, test_run_wrapper_script, showed up in one run and was gone in the next — a "Text file busy" flake, unrelated to this change.
• The staging step, end to end on the affected host (ubuntu 26.04): cloned valgrind-codspeed, then ./autogen.sh && ./configure && make -j16 && make install DESTDIR=/tmp/vgcache — all green, unprivileged, and the result is the usual usr/local/{bin,libexec/valgrind,lib,include} layout with the executable bits preserved. Its runtime file set matches the published 24.04 .deb, the only difference being the Debian rename of the launcher to valgrind.bin.
• Not covered here: restoring onto / and a second, warm run. Both need root or a container, so they want CODSPEED_TEST_BASE_IMAGE=ubuntu:26.04 just test-integ, or a run in CI.

Open questions

• The staged tree is ~202 MB (134 MB libexec/, 66 MB of static libraries under lib/, 2.6 MB of headers). Only bin/ and libexec/valgrind/ are needed at runtime; should the staging drop the rest?
• A rebuild leaves the previous build's files behind in the cache. In the action flow a cache key never spans two valgrind versions (the key carries the runner version), and for a self-managed cache directory the version check still decides whether a restored build is usable — so this looked acceptable, but it is easy to clear what is being replaced if you prefer.
• libc6-dbg is still only installed on the package path, so the "debug info for the system libc" warning remains on this path. That looked out of scope, but it can be added here.

[greptile-apps]

Confidence Score: 2/5

[Medium impact] Caches valgrind builds across executor runs.

Fix cache path handling and the installed-version check order before merging.

Findings

1.  Relative cache paths fail ▶
2.  Spaces break cache restore ▶
3.  Working installs require root ▶

Fix with agent prompt

### Issue 1
src/executor/valgrind/build_from_source.rs:146-149
With `--setup-cache-dir .cache/codspeed`, `make install` resolves `DESTDIR` from the temporary source directory. But `restore_files_to_root` reads the same path from the runner's working directory. The build writes somewhere the restore never reads, so setup fails and the temporary files are deleted. Resolve the cache path to an absolute path before staging.

### Issue 2
src/executor/valgrind/setup.rs:271
The new source-cache restore inherits the unquoted shell copy. A populated cache at `/home/user/CodSpeed Cache` passes the directory checks, but Bash splits the path into separate arguments. Restore then fails and stops setup. This previously did not affect the source-install route. Pass the path as a separate argument or quote it safely.

### Issue 3
src/executor/valgrind/setup.rs:270-274
If a non-root user has no `sudo` and configures a non-empty cache, restore now fails before checking their installed `valgrind`. Setup stops even when the manual installation meets the version requirement. Check the installed version first, then restore and check again, as `install_cached` already does.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR caches source-built Valgrind and restores it on hosts without a published package.

• Source-built Valgrind now survives in the setup cache.

trim21 acknowledged cache size and leftover files after rebuilding as open questions, and missing libc6-dbg as out of scope.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Source-install setup] --> B[Restore configured cache]
  B --> C{Installed version usable?}
  C -- Yes --> D[Setup complete]
  C -- No --> E[Build Valgrind]
  E --> F{Cache configured?}
  F -- Yes --> G[Stage with DESTDIR]
  G --> H[Copy staged files to root]
  F -- No --> I[Install directly with sudo]
  H --> J[Check installed version]
  I --> J

Loading

_{Reviews (1) · Last reviewed commit: "fix(valgrind): cache the source build in..." · Reviewed by Greptile}

[trim21]

All three are real, thanks. Fixed in f23530a.

1. Relative cache paths (build_from_source.rs:146-149) — valid, and introduced by this PR. --setup-cache-dir was passed through verbatim (only ~ expanded), so a relative path was resolved against two different working directories: make install DESTDIR= runs in the temporary build directory, restore_files_to_root in the runner's. The staged tree went into the build directory, the restore found nothing to copy, and the install failed with "the freshly built valgrind is not usable". The directory is now resolved against the current directory where the CLI parses it (std::path::absolute, cli/mod.rs:154), with an empty value read as "no cache" — now that the path is made absolute, "" would otherwise become the working directory, i.e. cp -r <cwd>/* /.

2. Unquoted path in the restore command (setup.rs:271) — valid, but pre-existing: cp -r <dir>/* / is unchanged from restore_from_cache, and this PR is what routes the source build through that function. The directory is now single-quoted (embedded quotes escaped), so a path containing a space reaches cp as one argument.

3. Restore before the installed check (setup.rs:270-274) — valid, and the one real regression. restore_files_to_root needs root (run_with_sudo bails when neither root nor sudo), so on a system without a published package a non-root user without sudo, with an adequate valgrind already installed and an existing cache directory, went from "setup succeeds" to "setup fails". It also did a privileged full copy over / for an installation that was already fine. The installed check now comes first, then the restore, then the check again — the same shape as install_cached, and the restore still happens before the build prompt, so a warm cache keeps a run from building.

Verification on 1.88.0: cargo fmt --all -- --check and cargo clippy --all-targets -- -D warnings clean; cargo test --no-fail-fast has the same 23 failures as before the change, all in tests that read testdata/perf_map/* — those are unfetched git-lfs pointers in this checkout ("Unknown file magic"), unrelated to the diff.