From a028857eb80d26dfb52e25e72bc130e0afbd3a6f Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Mon, 2 Nov 2026 19:07:51 +0000 Subject: [PATCH 1/5] fix: find clang tools for a full version like `=12.0.1` A version requirement was only checked against `MAJOR.0.0`, so a full version such as `=12.0.1` or `17.0.6` (which means `^17.0.6`) matched no supported major version. `--version` then failed with "UnsupportedVersion" instead of finding `clang-format-12` on PATH or downloading it. - `lowest_match_in_major()` finds the lowest version of a major version that satisfies the requirement; it is used by `get_min_ver()` and the PATH lookup. - The static binaries are matched by their full version from `index.json`, not `MAJOR.0.0`. This also gives the right resource headers folder for LLVM 15 and older (`lib/clang/12.0.1`). --- clang-tools-manager/build.rs | 6 +- .../src/downloader/static_dist.rs | 23 ++++- clang-tools-manager/src/tool.rs | 4 +- clang-tools-manager/src/version.rs | 89 +++++++++++++++++-- 4 files changed, 107 insertions(+), 15 deletions(-) diff --git a/clang-tools-manager/build.rs b/clang-tools-manager/build.rs index 11242a53..7fe63934 100644 --- a/clang-tools-manager/build.rs +++ b/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::(); let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap()); fs::write( diff --git a/clang-tools-manager/src/downloader/static_dist.rs b/clang-tools-manager/src/downloader/static_dist.rs index fb7ba112..9129b9ad 100644 --- a/clang-tools-manager/src/downloader/static_dist.rs +++ b/clang-tools-manager/src/downloader/static_dist.rs @@ -108,8 +108,9 @@ mod supported_platform { 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)) } @@ -315,6 +316,22 @@ mod supported_platform { } } + #[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(); diff --git a/clang-tools-manager/src/tool.rs b/clang-tools-manager/src/tool.rs index bd8c29bf..4f261fe1 100644 --- a/clang-tools-manager/src/tool.rs +++ b/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,7 +122,7 @@ 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; diff --git a/clang-tools-manager/src/version.rs b/clang-tools-manager/src/version.rs index 9c8cc527..1a6cba98 100644 --- a/clang-tools-manager/src/version.rs +++ b/clang-tools-manager/src/version.rs @@ -194,16 +194,33 @@ impl RequestedVersion { } } +/// Returns the lowest supported version that satisfies `version_req`. pub fn get_min_ver(version_req: &VersionReq) -> Option { - 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 { + 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 +287,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 +577,14 @@ 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_unsupported_requirement() { let env = TestEnv::new(); @@ -598,6 +654,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(); From b30127cb4a17b659cd4840b3dd4fbca647c33e5e Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Mon, 2 Nov 2026 19:15:32 +0000 Subject: [PATCH 2/5] fix: download a cached static binary again when its checksum does not match The static binaries are named by major version only. With an exact version like `=17.0.6`, a cached clang-format-17 from an older release is rejected by the cache lookup, but download_tool reused that file and failed the checksum of the new release every time. --- .../src/downloader/static_dist.rs | 61 ++++++++++++++++--- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/clang-tools-manager/src/downloader/static_dist.rs b/clang-tools-manager/src/downloader/static_dist.rs index 9129b9ad..f9a34e25 100644 --- a/clang-tools-manager/src/downloader/static_dist.rs +++ b/clang-tools-manager/src/downloader/static_dist.rs @@ -101,7 +101,7 @@ mod supported_platform { use super::{StaticDistDownloadError, StaticDistDownloader}; use crate::{ - Cacher, ClangTool, + Cacher, ClangTool, DownloadError, downloader::{download, hashing::HashAlgorithm, resource_headers}, utils::lock_path, }; @@ -207,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() { @@ -229,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} 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) { @@ -316,6 +334,35 @@ 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) = From 6a7535a284feb593ab3433c7bdbffa835244f07b Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Mon, 2 Nov 2026 20:51:25 +0000 Subject: [PATCH 3/5] style: say "static binary" in the re-download warning --- clang-tools-manager/src/downloader/static_dist.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-manager/src/downloader/static_dist.rs b/clang-tools-manager/src/downloader/static_dist.rs index f9a34e25..c1aff0d3 100644 --- a/clang-tools-manager/src/downloader/static_dist.rs +++ b/clang-tools-manager/src/downloader/static_dist.rs @@ -241,7 +241,7 @@ mod supported_platform { .. })) if cached => { log::warn!( - "The cached {tool} binary is not from the {release_tag} release; downloading it again." + "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)?; From 031378361f692e2e3a8f157596c595f0e1454b1f Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Mon, 2 Nov 2026 21:06:40 +0000 Subject: [PATCH 4/5] fix: skip a suffixed tool with a non-matching version and check every cached major - A `clang-format-17` that reports 17.0.5 no longer hides a matching `clang-format` 17.0.6 on PATH for `=17.0.6`. - The cache lookup checks each major version that can satisfy the requirement, so `>16.0.0, <18` finds a cached `clang-format-17`. --- clang-tools-manager/src/tool.rs | 8 ++++++- clang-tools-manager/src/version.rs | 37 ++++++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/clang-tools-manager/src/tool.rs b/clang-tools-manager/src/tool.rs index 4f261fe1..0968584d 100644 --- a/clang-tools-manager/src/tool.rs +++ b/clang-tools-manager/src/tool.rs @@ -129,11 +129,17 @@ impl ClangTool { } // 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 unsuffixed binary 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. diff --git a/clang-tools-manager/src/version.rs b/clang-tools-manager/src/version.rs index 1a6cba98..919d0a61 100644 --- a/clang-tools-manager/src/version.rs +++ b/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. @@ -585,6 +592,15 @@ mod tests { 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(); @@ -604,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(); From f2e0f2006b64ecc2b2324778af9290753cbcd551 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Mon, 2 Nov 2026 21:08:01 +0000 Subject: [PATCH 5/5] style: reword a comment for the spell check --- clang-tools-manager/src/tool.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-manager/src/tool.rs b/clang-tools-manager/src/tool.rs index 0968584d..9effe9e5 100644 --- a/clang-tools-manager/src/tool.rs +++ b/clang-tools-manager/src/tool.rs @@ -130,7 +130,7 @@ impl ClangTool { // 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 unsuffixed binary below is still considered. + // so the plain `clang-format` (or `clang-tidy`) below is still considered. for major in majors { if let Ok(cmd) = which(format!("{self}-{major}")) { match self.capture_version(&cmd) {