fix: valgrind-codspeed version comparison

src/binary_pins.rs

@@ -9,13 +9,18 @@ use std::sync::LazyLock;
 /// download (combined with `VALGRIND_DEB_REV`) and for detecting an already
 /// installed copy.
 pub const VALGRIND_CODSPEED_VERSION: Version = Version::new(3, 26, 0);
+/// CodSpeed repackaging iteration of `VALGRIND_CODSPEED_VERSION`. Bumps when
+/// the .deb is repackaged without a new upstream valgrind release. Appears in
+/// the .deb package version (`3.26.0-0codspeed3`) and in `valgrind --version`
+/// output (`valgrind-3.26.0.codspeed3`).
+pub const VALGRIND_CODSPEED_ITERATION: u32 = 3;
 /// Suffix appended to `VALGRIND_CODSPEED_VERSION` to form the .deb package version.
-/// Bumps when the .deb is repackaged without a new upstream valgrind release.
-const VALGRIND_DEB_REV: &str = "0codspeed3";
-/// String form of `VALGRIND_CODSPEED_VERSION` as it appears in `valgrind --version`
+static VALGRIND_DEB_REV: LazyLock<String> =
+    LazyLock::new(|| format!("0codspeed{VALGRIND_CODSPEED_ITERATION}"));
+/// String form of the pinned version as it appears in `valgrind --version`
 /// output, used to identify a CodSpeed build at runtime.
 pub static VALGRIND_CODSPEED_VERSION_STRING: LazyLock<String> =
-    LazyLock::new(|| format!("{VALGRIND_CODSPEED_VERSION}.codspeed"));
+    LazyLock::new(|| format!("{VALGRIND_CODSPEED_VERSION}.codspeed{VALGRIND_CODSPEED_ITERATION}"));
 
 #[derive(Debug, Clone, Copy)]
 struct BinaryPin {
@@ -74,7 +79,7 @@ pub struct ValgrindTarget {
 }
 
 static VALGRIND_DEB_VERSION: LazyLock<String> =
-    LazyLock::new(|| format!("{VALGRIND_CODSPEED_VERSION}-{VALGRIND_DEB_REV}"));
+    LazyLock::new(|| format!("{VALGRIND_CODSPEED_VERSION}-{}", VALGRIND_DEB_REV.as_str()));
 const VALGRIND_DEB_URL_TEMPLATE: &str = "https://github.com/CodSpeedHQ/valgrind-codspeed/releases/download/{version}/valgrind_{version}_ubuntu-{distro_version}_{arch}.deb";
 
 impl ValgrindTarget {

src/executor/valgrind/setup.rs

@@ -1,6 +1,6 @@
 use crate::binary_pins::{
-    Arch, DistroVersion, PinnedBinary, VALGRIND_CODSPEED_VERSION, VALGRIND_CODSPEED_VERSION_STRING,
-    ValgrindTarget,
+    Arch, DistroVersion, PinnedBinary, VALGRIND_CODSPEED_ITERATION, VALGRIND_CODSPEED_VERSION,
+    VALGRIND_CODSPEED_VERSION_STRING, ValgrindTarget,
 };
 use crate::cli::run::helpers::download_pinned_file;
 use crate::executor::helpers::apt;
@@ -53,26 +53,38 @@ pub(super) fn is_codspeed_valgrind_installation_supported(system_info: &SystemIn
     get_codspeed_valgrind_target(system_info).is_ok()
 }
 
-/// Parse a valgrind version string and extract the semantic version.
-/// Expected format: "valgrind-3.25.1.codspeed" or "3.25.1.codspeed"
-/// Returns Some(Version) if parsing succeeds, None otherwise.
-fn parse_valgrind_codspeed_version(version_str: &str) -> Option<Version> {
-    let version_str = version_str.trim();
+#[derive(Debug, Clone, PartialEq, Eq)]
+struct ValgrindVersion {
+    semver: Version,
+    codspeed_iteration: Option<u32>,
+}
+
+/// Parse a valgrind version string and extract the upstream semver plus the
+/// optional CodSpeed iteration number.
+///
+/// Accepted formats:
+/// - `valgrind-3.25.1.codspeed`  (legacy, no iteration)
+/// - `valgrind-3.25.1.codspeed2` (with iteration)
+/// - same forms without the `valgrind-` prefix
+fn parse_valgrind_codspeed_version(version_str: &str) -> Option<ValgrindVersion> {
+    let stripped = version_str
+        .trim()
+        .strip_prefix("valgrind-")
+        .unwrap_or(version_str.trim());
+
+    let (semver_part, iteration_part) = stripped.split_once(".codspeed")?;
+    let semver = Version::parse(semver_part).ok()?;
 
-    // Extract the version numbers before .codspeed
-    let version_part = if let Some(codspeed_idx) = version_str.find(".codspeed") {
-        &version_str[..codspeed_idx]
+    let codspeed_iteration = if iteration_part.is_empty() {
+        None
     } else {
-        return None;
+        Some(iteration_part.parse::<u32>().ok()?)
     };
 
-    // Remove "valgrind-" prefix if present
-    let version_part = version_part
-        .strip_prefix("valgrind-")
-        .unwrap_or(version_part);
-
-    // Parse using semver
-    Version::parse(version_part).ok()
+    Some(ValgrindVersion {
+        semver,
+        codspeed_iteration,
+    })
 }
 
 const TOOL_NAME: &str = "valgrind";
@@ -113,46 +125,52 @@ pub fn get_valgrind_status() -> ToolStatus {
     let version = String::from_utf8_lossy(&version_output.stdout)
         .trim()
         .to_string();
+    debug!(
+        "Found installed valgrind version: {version} (expecting {} or higher)",
+        VALGRIND_CODSPEED_VERSION_STRING.as_str()
+    );
+
+    ToolStatus {
+        tool_name,
+        status: classify_valgrind_version(version),
+    }
+}
 
+/// Classify a trimmed `valgrind --version` output against the pinned CodSpeed
+/// valgrind version.
+fn classify_valgrind_version(version: String) -> ToolInstallStatus {
     // Check if it's a codspeed version
     if !version.contains(".codspeed") {
-        return ToolStatus {
-            tool_name,
-            status: ToolInstallStatus::IncorrectVersion {
-                version,
-                message: "not a CodSpeed build".to_string(),
-            },
+        return ToolInstallStatus::IncorrectVersion {
+            version,
+            message: "not a CodSpeed build".to_string(),
         };
     }
 
     // Parse the installed version
     let Some(installed_version) = parse_valgrind_codspeed_version(&version) else {
-        return ToolStatus {
-            tool_name,
-            status: ToolInstallStatus::IncorrectVersion {
-                version,
-                message: "could not parse version".to_string(),
-            },
+        return ToolInstallStatus::IncorrectVersion {
+            version,
+            message: "could not parse version".to_string(),
         };
     };
 
-    if installed_version < VALGRIND_CODSPEED_VERSION {
-        return ToolStatus {
-            tool_name,
-            status: ToolInstallStatus::IncorrectVersion {
-                version,
-                message: format!(
-                    "version too old, expecting {} or higher",
-                    VALGRIND_CODSPEED_VERSION_STRING.as_str()
-                ),
-            },
+    // Legacy `.codspeed` builds (no iteration suffix) predate iteration
+    // tracking, so they count as iteration 0.
+    let installed_iteration = installed_version.codspeed_iteration.unwrap_or(0);
+    let is_version_outdated = (installed_version.semver, installed_iteration)
+        < (VALGRIND_CODSPEED_VERSION, VALGRIND_CODSPEED_ITERATION);
+    if is_version_outdated {
+        return ToolInstallStatus::IncorrectVersion {
+            version,
+            message: format!(
+                "version too old, expecting {} or higher",
+                VALGRIND_CODSPEED_VERSION_STRING.as_str()
+            ),
         };
     }
 
-    ToolStatus {
-        tool_name,
-        status: ToolInstallStatus::Installed { version },
-    }
+    ToolInstallStatus::Installed { version }
 }
 
 fn is_valgrind_installed() -> bool {
@@ -275,32 +293,100 @@ mod tests {
     #[test]
     fn test_parse_valgrind_codspeed_version_with_prefix() {
         let version = parse_valgrind_codspeed_version("valgrind-3.25.1.codspeed").unwrap();
-        assert_eq!(version, Version::new(3, 25, 1));
+        assert_eq!(version.semver, Version::new(3, 25, 1));
+        assert_eq!(version.codspeed_iteration, None);
     }
 
     #[test]
     fn test_parse_valgrind_codspeed_version_without_prefix() {
         let version = parse_valgrind_codspeed_version("3.25.1.codspeed").unwrap();
-        assert_eq!(version, Version::new(3, 25, 1));
+        assert_eq!(version.semver, Version::new(3, 25, 1));
+        assert_eq!(version.codspeed_iteration, None);
     }
 
     #[test]
     fn test_parse_valgrind_codspeed_version_higher_patch() {
         let version = parse_valgrind_codspeed_version("valgrind-3.25.2.codspeed").unwrap();
-        assert_eq!(version, Version::new(3, 25, 2));
+        assert_eq!(version.semver, Version::new(3, 25, 2));
     }
 
     #[test]
     fn test_parse_valgrind_codspeed_version_with_newline() {
         let version = parse_valgrind_codspeed_version("valgrind-3.25.1.codspeed\n").unwrap();
-        assert_eq!(version, Version::new(3, 25, 1));
+        assert_eq!(version.semver, Version::new(3, 25, 1));
     }
 
     #[test]
     fn test_parse_valgrind_codspeed_version_without_codspeed_suffix() {
         assert_eq!(parse_valgrind_codspeed_version("valgrind-3.25.1"), None);
     }
 
+    #[test]
+    fn test_parse_valgrind_codspeed_version_iterations_are_differentiated() {
+        let legacy = parse_valgrind_codspeed_version("valgrind-3.26.0.codspeed").unwrap();
+        let v1 = parse_valgrind_codspeed_version("valgrind-3.26.0.codspeed1").unwrap();
+        let v2 = parse_valgrind_codspeed_version("valgrind-3.26.0.codspeed2").unwrap();
+
+        assert_eq!(legacy.semver, v1.semver);
+        assert_eq!(legacy.codspeed_iteration, None);
+        assert_eq!(v1.codspeed_iteration, Some(1));
+        assert_eq!(v2.codspeed_iteration, Some(2));
+
+        assert_ne!(legacy, v1);
+        assert_ne!(v1, v2);
+    }
+
+    #[test]
+    fn test_classify_same_version_older_iteration_is_rejected() {
+        // Pinned is 3.26.0-0codspeed3: a cached 3.26.0.codspeed2 build is the
+        // same upstream valgrind but an older repackaging, so it must be
+        // rejected and reinstalled.
+        let status = classify_valgrind_version("valgrind-3.26.0.codspeed2".to_string());
+        assert!(
+            matches!(status, ToolInstallStatus::IncorrectVersion { .. }),
+            "stale codspeed iteration must be rejected, got: Installed"
+        );
+    }
+
+    #[test]
+    fn test_classify_legacy_suffix_with_pinned_semver_is_rejected() {
+        // Old builds report just `.codspeed` (no iteration); for the pinned
+        // upstream version they predate the pinned iteration.
+        let version = format!("valgrind-{VALGRIND_CODSPEED_VERSION}.codspeed");
+        let status = classify_valgrind_version(version);
+        assert!(matches!(status, ToolInstallStatus::IncorrectVersion { .. }));
+    }
+
+    #[test]
+    fn test_classify_pinned_iteration_is_installed() {
+        let version =
+            format!("valgrind-{VALGRIND_CODSPEED_VERSION}.codspeed{VALGRIND_CODSPEED_ITERATION}");
+        let status = classify_valgrind_version(version);
+        assert!(matches!(status, ToolInstallStatus::Installed { .. }));
+    }
+
+    #[test]
+    fn test_classify_newer_iteration_is_installed() {
+        let version = format!(
+            "valgrind-{VALGRIND_CODSPEED_VERSION}.codspeed{}",
+            VALGRIND_CODSPEED_ITERATION + 1
+        );
+        let status = classify_valgrind_version(version);
+        assert!(matches!(status, ToolInstallStatus::Installed { .. }));
+    }
+
+    #[test]
+    fn test_classify_newer_semver_with_legacy_suffix_is_installed() {
+        let status = classify_valgrind_version("valgrind-3.99.0.codspeed".to_string());
+        assert!(matches!(status, ToolInstallStatus::Installed { .. }));
+    }
+
+    #[test]
+    fn test_classify_non_codspeed_build_is_rejected() {
+        let status = classify_valgrind_version("valgrind-3.26.0".to_string());
+        assert!(matches!(status, ToolInstallStatus::IncorrectVersion { .. }));
+    }
+
     #[test]
     fn test_parse_valgrind_codspeed_version_invalid_format() {
         assert_eq!(