Skip to content

Commit 5c0854f

Browse files
not-matthiasclaude
andcommitted
fix(valgrind): compare codspeed iteration of installed valgrind
Previously only the upstream semver was compared, so a cached 3.26.0.codspeed2 build still satisfied the 3.26.0-0codspeed3 pin and stale valgrind builds were restored from the CI cache instead of being reinstalled. Compare the (semver, iteration) pair against the pinned values, treating legacy `.codspeed` builds without an iteration as iteration 0. The pinned iteration is a single constant that also forms the .deb revision, so future bumps stay in one place. Fixes COD-2782 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 7c5e040 commit 5c0854f

2 files changed

Lines changed: 91 additions & 33 deletions

File tree

src/binary_pins.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,18 @@ use std::sync::LazyLock;
99
/// download (combined with `VALGRIND_DEB_REV`) and for detecting an already
1010
/// installed copy.
1111
pub const VALGRIND_CODSPEED_VERSION: Version = Version::new(3, 26, 0);
12+
/// CodSpeed repackaging iteration of `VALGRIND_CODSPEED_VERSION`. Bumps when
13+
/// the .deb is repackaged without a new upstream valgrind release. Appears in
14+
/// the .deb package version (`3.26.0-0codspeed3`) and in `valgrind --version`
15+
/// output (`valgrind-3.26.0.codspeed3`).
16+
pub const VALGRIND_CODSPEED_ITERATION: u32 = 3;
1217
/// Suffix appended to `VALGRIND_CODSPEED_VERSION` to form the .deb package version.
13-
/// Bumps when the .deb is repackaged without a new upstream valgrind release.
14-
const VALGRIND_DEB_REV: &str = "0codspeed3";
15-
/// String form of `VALGRIND_CODSPEED_VERSION` as it appears in `valgrind --version`
18+
static VALGRIND_DEB_REV: LazyLock<String> =
19+
LazyLock::new(|| format!("0codspeed{VALGRIND_CODSPEED_ITERATION}"));
20+
/// String form of the pinned version as it appears in `valgrind --version`
1621
/// output, used to identify a CodSpeed build at runtime.
1722
pub static VALGRIND_CODSPEED_VERSION_STRING: LazyLock<String> =
18-
LazyLock::new(|| format!("{VALGRIND_CODSPEED_VERSION}.codspeed"));
23+
LazyLock::new(|| format!("{VALGRIND_CODSPEED_VERSION}.codspeed{VALGRIND_CODSPEED_ITERATION}"));
1924

2025
#[derive(Debug, Clone, Copy)]
2126
struct BinaryPin {
@@ -74,7 +79,7 @@ pub struct ValgrindTarget {
7479
}
7580

7681
static VALGRIND_DEB_VERSION: LazyLock<String> =
77-
LazyLock::new(|| format!("{VALGRIND_CODSPEED_VERSION}-{VALGRIND_DEB_REV}"));
82+
LazyLock::new(|| format!("{VALGRIND_CODSPEED_VERSION}-{}", VALGRIND_DEB_REV.as_str()));
7883
const VALGRIND_DEB_URL_TEMPLATE: &str = "https://github.com/CodSpeedHQ/valgrind-codspeed/releases/download/{version}/valgrind_{version}_ubuntu-{distro_version}_{arch}.deb";
7984

8085
impl ValgrindTarget {

src/executor/valgrind/setup.rs

Lines changed: 81 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::binary_pins::{
2-
Arch, DistroVersion, PinnedBinary, VALGRIND_CODSPEED_VERSION, VALGRIND_CODSPEED_VERSION_STRING,
3-
ValgrindTarget,
2+
Arch, DistroVersion, PinnedBinary, VALGRIND_CODSPEED_ITERATION, VALGRIND_CODSPEED_VERSION,
3+
VALGRIND_CODSPEED_VERSION_STRING, ValgrindTarget,
44
};
55
use crate::cli::run::helpers::download_pinned_file;
66
use crate::executor::helpers::apt;
@@ -126,45 +126,47 @@ pub fn get_valgrind_status() -> ToolStatus {
126126
.trim()
127127
.to_string();
128128

129+
ToolStatus {
130+
tool_name,
131+
status: classify_valgrind_version(version),
132+
}
133+
}
134+
135+
/// Classify a trimmed `valgrind --version` output against the pinned CodSpeed
136+
/// valgrind version.
137+
fn classify_valgrind_version(version: String) -> ToolInstallStatus {
129138
// Check if it's a codspeed version
130139
if !version.contains(".codspeed") {
131-
return ToolStatus {
132-
tool_name,
133-
status: ToolInstallStatus::IncorrectVersion {
134-
version,
135-
message: "not a CodSpeed build".to_string(),
136-
},
140+
return ToolInstallStatus::IncorrectVersion {
141+
version,
142+
message: "not a CodSpeed build".to_string(),
137143
};
138144
}
139145

140146
// Parse the installed version
141147
let Some(installed_version) = parse_valgrind_codspeed_version(&version) else {
142-
return ToolStatus {
143-
tool_name,
144-
status: ToolInstallStatus::IncorrectVersion {
145-
version,
146-
message: "could not parse version".to_string(),
147-
},
148+
return ToolInstallStatus::IncorrectVersion {
149+
version,
150+
message: "could not parse version".to_string(),
148151
};
149152
};
150153

151-
if installed_version.semver < VALGRIND_CODSPEED_VERSION {
152-
return ToolStatus {
153-
tool_name,
154-
status: ToolInstallStatus::IncorrectVersion {
155-
version,
156-
message: format!(
157-
"version too old, expecting {} or higher",
158-
VALGRIND_CODSPEED_VERSION_STRING.as_str()
159-
),
160-
},
154+
// Legacy `.codspeed` builds (no iteration suffix) predate iteration
155+
// tracking, so they count as iteration 0.
156+
let installed_iteration = installed_version.codspeed_iteration.unwrap_or(0);
157+
let is_version_outdated = (installed_version.semver, installed_iteration)
158+
< (VALGRIND_CODSPEED_VERSION, VALGRIND_CODSPEED_ITERATION);
159+
if is_version_outdated {
160+
return ToolInstallStatus::IncorrectVersion {
161+
version,
162+
message: format!(
163+
"version too old, expecting {} or higher",
164+
VALGRIND_CODSPEED_VERSION_STRING.as_str()
165+
),
161166
};
162167
}
163168

164-
ToolStatus {
165-
tool_name,
166-
status: ToolInstallStatus::Installed { version },
167-
}
169+
ToolInstallStatus::Installed { version }
168170
}
169171

170172
fn is_valgrind_installed() -> bool {
@@ -330,6 +332,57 @@ mod tests {
330332
assert_ne!(v1, v2);
331333
}
332334

335+
#[test]
336+
fn test_classify_same_version_older_iteration_is_rejected() {
337+
// Pinned is 3.26.0-0codspeed3: a cached 3.26.0.codspeed2 build is the
338+
// same upstream valgrind but an older repackaging, so it must be
339+
// rejected and reinstalled.
340+
let status = classify_valgrind_version("valgrind-3.26.0.codspeed2".to_string());
341+
assert!(
342+
matches!(status, ToolInstallStatus::IncorrectVersion { .. }),
343+
"stale codspeed iteration must be rejected, got: Installed"
344+
);
345+
}
346+
347+
#[test]
348+
fn test_classify_legacy_suffix_with_pinned_semver_is_rejected() {
349+
// Old builds report just `.codspeed` (no iteration); for the pinned
350+
// upstream version they predate the pinned iteration.
351+
let version = format!("valgrind-{VALGRIND_CODSPEED_VERSION}.codspeed");
352+
let status = classify_valgrind_version(version);
353+
assert!(matches!(status, ToolInstallStatus::IncorrectVersion { .. }));
354+
}
355+
356+
#[test]
357+
fn test_classify_pinned_iteration_is_installed() {
358+
let version =
359+
format!("valgrind-{VALGRIND_CODSPEED_VERSION}.codspeed{VALGRIND_CODSPEED_ITERATION}");
360+
let status = classify_valgrind_version(version);
361+
assert!(matches!(status, ToolInstallStatus::Installed { .. }));
362+
}
363+
364+
#[test]
365+
fn test_classify_newer_iteration_is_installed() {
366+
let version = format!(
367+
"valgrind-{VALGRIND_CODSPEED_VERSION}.codspeed{}",
368+
VALGRIND_CODSPEED_ITERATION + 1
369+
);
370+
let status = classify_valgrind_version(version);
371+
assert!(matches!(status, ToolInstallStatus::Installed { .. }));
372+
}
373+
374+
#[test]
375+
fn test_classify_newer_semver_with_legacy_suffix_is_installed() {
376+
let status = classify_valgrind_version("valgrind-3.99.0.codspeed".to_string());
377+
assert!(matches!(status, ToolInstallStatus::Installed { .. }));
378+
}
379+
380+
#[test]
381+
fn test_classify_non_codspeed_build_is_rejected() {
382+
let status = classify_valgrind_version("valgrind-3.26.0".to_string());
383+
assert!(matches!(status, ToolInstallStatus::IncorrectVersion { .. }));
384+
}
385+
333386
#[test]
334387
fn test_parse_valgrind_codspeed_version_invalid_format() {
335388
assert_eq!(

0 commit comments

Comments
 (0)