fix: find clang tools for a full version like =12.0.1

clang-tools-manager/build.rs

@@ -22,6 +22,8 @@ struct StaticBinariesIndex {
 
 #[derive(Debug, serde::Deserialize)]
 struct VersionEntry {
+    /// The full LLVM version (for example, `12.0.1`).
+    version: String,
     /// The release tag that holds this LLVM version's binaries.
     tag: String,
 }
@@ -80,8 +82,8 @@ fn main() {
     // Each LLVM version is published in its own release.
     let release_tags = index
         .versions
-        .iter()
-        .map(|(major, entry)| format!("    ({major}, {:?}),\n", entry.tag))
+        .values()
+        .map(|entry| format!("    ({:?}, {:?}),\n", entry.version, entry.tag))
         .collect::<String>();
     let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap());
     fs::write(

clang-tools-manager/src/downloader/static_dist.rs

@@ -101,15 +101,16 @@ mod supported_platform {
 
     use super::{StaticDistDownloadError, StaticDistDownloader};
     use crate::{
-        Cacher, ClangTool,
+        Cacher, ClangTool, DownloadError,
         downloader::{download, hashing::HashAlgorithm, resource_headers},
         utils::lock_path,
     };
 
     const CLANG_TOOLS_REPO: &str = "https://github.com/cpp-linter/clang-tools-static-binaries";
 
-    /// Each available LLVM major version and the release tag that holds its binaries.
-    const RELEASE_TAGS: &[(u8, &str)] = include!(concat!(env!("OUT_DIR"), "/release_tags.rs"));
+    /// Each available LLVM version (one per major version)
+    /// and the release tag that holds its binaries.
+    const RELEASE_TAGS: &[(&str, &str)] = include!(concat!(env!("OUT_DIR"), "/release_tags.rs"));
 
     impl StaticDistDownloader {
         /// Finds the newest available version that satisfies `req_ver`,
@@ -123,7 +124,7 @@ mod supported_platform {
             RELEASE_TAGS
                 .iter()
                 .rev()
-                .map(|(major, tag)| (Version::new(*major as u64, 0, 0), *tag))
+                .filter_map(|(ver, tag)| Some((Version::parse(ver).ok()?, *tag)))
                 .find(|(ver, _)| req_ver.matches(ver))
         }
 
@@ -206,16 +207,21 @@ mod supported_platform {
                 Some(dir) => dir.join(&bin_name),
             };
             let file_lock = lock_path(&download_path)?;
-            if download_path.exists() {
+            let download_bin = || async {
+                log::info!("Downloading static binary for {tool} version {ver} from {url}");
+                download(&url, &download_path, 60 * 2).await?;
+                #[cfg(unix)]
+                super::super::chmod_file(&download_path, None)?;
+                Ok::<(), StaticDistDownloadError>(())
+            };
+            let cached = download_path.exists();
+            if cached {
                 log::info!(
                     "Using cached static binary for {tool} version {ver_str} from {:?}",
                     download_path.to_string_lossy()
                 );
             } else {
-                log::info!("Downloading static binary for {tool} version {ver_str} from {url}");
-                download(&url, &download_path, 60 * 2).await?;
-                #[cfg(unix)]
-                super::super::chmod_file(&download_path, None)?;
+                download_bin().await?;
             }
             let sha512_cache_path = Self::sha512_cache_path(release_tag);
             if sha512_cache_path.exists() {
@@ -228,7 +234,20 @@ mod supported_platform {
                 log::info!("Downloading SHA512 checksum for static binaries from {sha512_url}");
                 download(&sha512_url, &sha512_cache_path, 10).await?;
             }
-            Self::verify_sha512(&download_path, &tool_url_path, &sha512_cache_path)?;
+            match Self::verify_sha512(&download_path, &tool_url_path, &sha512_cache_path) {
+                // The binaries are named by major version only, so the cached binary
+                // may be from an older release of the same major version.
+                Err(StaticDistDownloadError::DownloadError(DownloadError::HashMismatch {
+                    ..
+                })) if cached => {
+                    log::warn!(
+                        "The cached {tool} static binary is not from the {release_tag} release; downloading it again."
+                    );
+                    download_bin().await?;
+                    Self::verify_sha512(&download_path, &tool_url_path, &sha512_cache_path)?;
+                }
+                result => result?,
+            }
             if tool.needs_resource_headers()
                 && !resource_headers::has_resource_headers(&download_path, &ver)
             {
@@ -315,6 +334,51 @@ mod supported_platform {
             }
         }
 
+        /// A cached binary of the same major version from another release
+        /// (so its checksum does not match) is downloaded again.
+        #[tokio::test]
+        async fn cached_from_other_release() {
+            let cache = TempDir::new().unwrap();
+            unsafe {
+                std::env::set_var("CPP_LINTER_CACHE", cache.path());
+            }
+            let out_dir = TempDir::new().unwrap();
+            let suffix = std::env::consts::EXE_SUFFIX;
+            let bin = out_dir.path().join(format!("clang-format-17{suffix}"));
+            fs::write(&bin, b"clang-format from another release").unwrap();
+
+            let req = VersionReq::parse("17").unwrap();
+            let out_dir_path = out_dir.path().to_path_buf();
+            let result = StaticDistDownloader::download_tool(
+                &ClangTool::ClangFormat,
+                &req,
+                Some(&out_dir_path),
+            )
+            .await
+            .unwrap();
+            assert_eq!(result, bin);
+            assert_ne!(
+                fs::read(&bin).unwrap(),
+                b"clang-format from another release"
+            );
+        }
+
+        #[test]
+        fn find_exact_version() {
+            let (newest, tag) =
+                StaticDistDownloader::find_suitable_version(&VersionReq::parse("*").unwrap())
+                    .unwrap();
+            // the full version, not just `major.0.0`
+            let req_ver = VersionReq::parse(&format!("={newest}")).unwrap();
+            assert_eq!(
+                StaticDistDownloader::find_suitable_version(&req_ver),
+                Some((newest.clone(), tag))
+            );
+            // a newer version of the same major that is not published
+            let req_ver = VersionReq::parse(&format!(">{newest}, <{}", newest.major + 1)).unwrap();
+            assert_eq!(StaticDistDownloader::find_suitable_version(&req_ver), None);
+        }
+
         #[test]
         fn find_none() {
             let req_ver = VersionReq::parse("=8").unwrap();

clang-tools-manager/src/tool.rs

@@ -10,7 +10,7 @@ use regex::Regex;
 use semver::Version;
 use which::{which, which_in};
 
-use crate::RequestedVersion;
+use crate::{RequestedVersion, version::lowest_match_in_major};
 
 /// Error that occur when trying to get the path to a clang tool executable.
 #[derive(Debug, thiserror::Error)]
@@ -122,18 +122,24 @@ impl ClangTool {
                 let mut majors = vec![];
                 while highest_major > 0 {
                     // check if the current major version satisfies the requirement.
-                    if req.matches(&Version::new(highest_major, 0, 0)) {
+                    if lowest_match_in_major(req, highest_major).is_some() {
                         majors.push(highest_major);
                     }
                     highest_major -= 1;
                 }
 
                 // now we're ready to search for the binary exe with the major version suffixed.
+                // A suffixed binary whose version does not satisfy the requirement is skipped,
+                // so the plain `clang-format` (or `clang-tidy`) below is still considered.
                 for major in majors {
                     if let Ok(cmd) = which(format!("{self}-{major}")) {
-                        return Ok(cmd);
+                        match self.capture_version(&cmd) {
+                            Ok(version) if !req.matches(&version) => continue,
+                            _ => return Ok(cmd),
+                        }
                     }
                 }
+
                 // failed to find a binary where the major version number is suffixed to the tool name.
 
                 // This line essentially ignores the version specified as a fail-safe.

clang-tools-manager/src/version.rs

@@ -120,11 +120,18 @@ impl RequestedVersion {
                 // check if cache has a suitable version
                 let bin_ext = if cfg!(windows) { ".exe" } else { "" };
                 let min_ver = get_min_ver(version_req).ok_or(GetToolError::UnsupportedVersion)?;
-                // Downloaded binaries are named with only the major version.
-                let cached_bin = StaticDistDownloader::get_cache_dir()
-                    .join("bin")
-                    .join(format!("{tool}-{}{bin_ext}", min_ver.major));
-                if cached_bin.exists() {
+                // Downloaded binaries are named with only the major version,
+                // so check each major version that can satisfy the requirement (newest first).
+                for major in StaticDistDownloader::get_major_version_range()
+                    .rev()
+                    .filter(|major| lowest_match_in_major(version_req, *major as u64).is_some())
+                {
+                    let cached_bin = StaticDistDownloader::get_cache_dir()
+                        .join("bin")
+                        .join(format!("{tool}-{major}{bin_ext}"));
+                    if !cached_bin.exists() {
+                        continue;
+                    }
                     let version = tool.capture_version(&cached_bin)?;
                     // A cached clang-tidy without resource headers is downloaded again,
                     // so the headers get installed.
@@ -194,16 +201,33 @@ impl RequestedVersion {
     }
 }
 
+/// Returns the lowest supported version that satisfies `version_req`.
 pub fn get_min_ver(version_req: &VersionReq) -> Option<Version> {
-    let mut result = None;
-    let supported_version_range = StaticDistDownloader::get_major_version_range();
-    for major in supported_version_range.rev() {
-        let ver = Version::new(major as u64, 0, 0);
-        if version_req.matches(&ver) {
-            result = Some(ver);
-        }
+    StaticDistDownloader::get_major_version_range()
+        .find_map(|major| lowest_match_in_major(version_req, major as u64))
+}
+
+/// Returns the lowest version with the given `major` number that satisfies `version_req`.
+///
+/// Checking only `major.0.0` is not enough: a requirement like `=12.0.1`
+/// or `17.0.6` (which means `^17.0.6`) does not match `12.0.0` or `17.0.0`.
+///
+/// The lowest match is either `major.0.0` or the lower bound of one of
+/// the requirement's comparators, so only those versions are checked.
+pub(crate) fn lowest_match_in_major(version_req: &VersionReq, major: u64) -> Option<Version> {
+    let mut candidates = vec![Version::new(major, 0, 0)];
+    for comparator in version_req.comparators.iter().filter(|c| c.major == major) {
+        let minor = comparator.minor.unwrap_or_default();
+        let patch = comparator.patch.unwrap_or_default();
+        candidates.push(Version::new(major, minor, patch));
+        // lower bounds of `>major.minor.patch` and `>major.minor`
+        candidates.push(Version::new(major, minor, patch + 1));
+        candidates.push(Version::new(major, minor + 1, 0));
     }
-    result
+    candidates
+        .into_iter()
+        .filter(|ver| version_req.matches(ver))
+        .min()
 }
 
 /// Represents an error that occurred while parsing a requested version.
@@ -270,6 +294,37 @@ mod tests {
 
     // See also crate::tool::tests module for other `RequestedVersion::from_str()` tests.
 
+    #[test]
+    fn lowest_match_in_major() {
+        for (req, major, expected) in [
+            ("17", 17, Some("17.0.0")),
+            ("=12.0.1", 12, Some("12.0.1")),
+            ("17.0.6", 17, Some("17.0.6")),
+            ("~17.0.6", 17, Some("17.0.6")),
+            (">17.0.6", 17, Some("17.0.7")),
+            (">17.0", 17, Some("17.1.0")),
+            (">=16.0.2, <18", 17, Some("17.0.0")),
+            (">=16.0.2, <18", 16, Some("16.0.2")),
+            ("=12.0.1", 13, None),
+            (">17", 17, None),
+        ] {
+            let req = VersionReq::parse(req).unwrap();
+            assert_eq!(
+                super::lowest_match_in_major(&req, major),
+                expected.map(|v| semver::Version::parse(v).unwrap()),
+                "{req} in major {major}"
+            );
+        }
+    }
+
+    #[test]
+    fn min_ver_of_exact_version() {
+        let min_ver = |req: &str| super::get_min_ver(&VersionReq::parse(req).unwrap());
+        assert_eq!(min_ver("=17.0.6"), Some(semver::Version::new(17, 0, 6)));
+        assert_eq!(min_ver("17.0.6"), Some(semver::Version::new(17, 0, 6)));
+        assert_eq!(min_ver("=8.0.1"), None);
+    }
+
     #[test]
     fn validate_version_path() {
         let this_path_str = "src/version.rs";
@@ -529,6 +584,23 @@ mod tests {
             assert_version(&exe, result, "17.0.6");
         }
 
+        #[tokio::test]
+        async fn eval_installed_exact_requirement() {
+            let env = TestEnv::new();
+            let exe = fake_clang_tool(env.bin.path(), "clang-format-17", "17.0.6");
+            let result = env.eval(req("=17.0.6"), false).await.unwrap().unwrap();
+            assert_version(&exe, result, "17.0.6");
+        }
+
+        #[tokio::test]
+        async fn eval_skips_suffixed_mismatch() {
+            let env = TestEnv::new();
+            fake_clang_tool(env.bin.path(), "clang-format-17", "17.0.5");
+            let exe = fake_clang_tool(env.bin.path(), "clang-format", "17.0.6");
+            let result = env.eval(req("=17.0.6"), false).await.unwrap().unwrap();
+            assert_version(&exe, result, "17.0.6");
+        }
+
         #[tokio::test]
         async fn eval_unsupported_requirement() {
             let env = TestEnv::new();
@@ -548,6 +620,17 @@ mod tests {
             assert_version(&cached, result, "17.0.6");
         }
 
+        #[tokio::test]
+        async fn eval_cached_bin_newer_major() {
+            let env = TestEnv::new();
+            let cached_bin_dir = env.cache.path().join("bin");
+            fs::create_dir_all(&cached_bin_dir).unwrap();
+            // the lowest match is 16.0.1, but the static binaries for 17 also match
+            let cached = fake_clang_tool(&cached_bin_dir, "clang-format-17", "17.0.6");
+            let result = env.eval(req(">16.0.0, <18"), false).await.unwrap().unwrap();
+            assert_version(&cached, result, "17.0.6");
+        }
+
         #[tokio::test]
         async fn eval_cached_tidy_needs_resource_headers() {
             let env = TestEnv::new();
@@ -598,6 +681,23 @@ mod tests {
             );
         }
 
+        #[tokio::test]
+        async fn eval_static_dist_exact_requirement() {
+            let env = TestEnv::new();
+            env.seed_empty_pypi();
+            env.seed_static_dist(true);
+            // the exact version of the LLVM 17 static binaries
+            let (ver, _) =
+                StaticDistDownloader::find_suitable_version(&VersionReq::parse("17").unwrap())
+                    .unwrap();
+            let result = env
+                .eval(req(&format!("={ver}")), false)
+                .await
+                .unwrap()
+                .unwrap();
+            assert_version(&env.out.path().join("clang-format-17"), result, "17.0.6");
+        }
+
         #[tokio::test]
         async fn eval_static_dist_failure() {
             let env = TestEnv::new();