From 3e27b92f3e85d17c75909c6e2950cb319f9c1566 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:34:08 +0000 Subject: [PATCH 1/7] fix: install clang resource headers with downloaded clang-tidy clang-tidy looks for clang's builtin headers (like `stddef.h`) in `/../lib/clang//include`. Only the executable was installed from a PyPI wheel or a static binary release, so any source that includes a standard header failed to compile. Those errors are in system headers, so cpp-linter dropped them and quietly reported fewer diagnostics. - Extract `/data/lib/clang/**` from clang-tidy wheels (wheels of v15 and older have no headers; a warning is logged). - Download, verify (SHA512SUMS) and extract `clang-resource-headers-.tar.gz` for static binaries. - A cached clang-tidy without headers is downloaded again. - The cached binary lookup used `-.0.0`, but downloads are named `-`, so the cache was never used. --- Cargo.lock | 22 ++ clang-tools-manager/Cargo.toml | 2 + clang-tools-manager/README.md | 5 +- clang-tools-manager/src/downloader/mod.rs | 1 + clang-tools-manager/src/downloader/pypi.rs | 105 ++++-- .../src/downloader/resource_headers.rs | 299 ++++++++++++++++++ .../src/downloader/static_dist.rs | 125 +++++++- clang-tools-manager/src/lib.rs | 1 + clang-tools-manager/src/main.rs | 3 + clang-tools-manager/src/tool.rs | 8 + clang-tools-manager/src/version.rs | 49 ++- cspell.config.yml | 2 + 12 files changed, 595 insertions(+), 27 deletions(-) create mode 100644 clang-tools-manager/src/downloader/resource_headers.rs diff --git a/Cargo.lock b/Cargo.lock index 68b8f165..da1b1b09 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -310,6 +310,7 @@ dependencies = [ "clap", "colored", "directories", + "flate2", "log", "mockito", "regex", @@ -318,6 +319,7 @@ dependencies = [ "serde", "serde_json", "sha2", + "tar", "tempfile", "thiserror", "tokio", @@ -682,6 +684,16 @@ version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" +[[package]] +name = "filetime" +version = "0.2.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c287a33c7f0a620c38e641e7f60827713987b3c0f26e8ddc9462cc69cf75759" +dependencies = [ + "cfg-if", + "libc", +] + [[package]] name = "find-msvc-tools" version = "0.1.14" @@ -2457,6 +2469,16 @@ dependencies = [ "libc", ] +[[package]] +name = "tar" +version = "0.4.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f6221d9a6003c78398e3b239969f352578258df48c8eb051caadae0015bc840" +dependencies = [ + "filetime", + "libc", +] + [[package]] name = "target-lexicon" version = "0.13.5" diff --git a/clang-tools-manager/Cargo.toml b/clang-tools-manager/Cargo.toml index 20fcd3b7..e9c977f2 100644 --- a/clang-tools-manager/Cargo.toml +++ b/clang-tools-manager/Cargo.toml @@ -27,6 +27,7 @@ blake2 = "0.11.0" clap = { workspace = true, optional = true } colored = { workspace = true, optional = true } directories = "6.0.0" +flate2 = { version = "1.1.10", default-features = false, features = ["zlib-rs"] } log = { workspace = true } regex = { workspace = true } reqwest = { workspace = true } @@ -34,6 +35,7 @@ semver = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } sha2 = "0.11.1" +tar = { version = "0.4.46", default-features = false } thiserror = { workspace = true } tokio = { workspace = true, features = ["macros"], optional = true } url = "2.5.8" diff --git a/clang-tools-manager/README.md b/clang-tools-manager/README.md index 65d0101e..4ffae2c4 100644 --- a/clang-tools-manager/README.md +++ b/clang-tools-manager/README.md @@ -64,7 +64,10 @@ Options: Without it, log level is set to INFO by default. -d, --directory - The directory where the clang tools should be installed + The directory where the clang tools should be installed. + + clang-tidy's resource headers (like `stddef.h`) are + installed in `/../lib/clang`. -f, --force Force overwriting symlink to the installed binary. diff --git a/clang-tools-manager/src/downloader/mod.rs b/clang-tools-manager/src/downloader/mod.rs index cf623f60..1b5da608 100644 --- a/clang-tools-manager/src/downloader/mod.rs +++ b/clang-tools-manager/src/downloader/mod.rs @@ -14,6 +14,7 @@ pub mod caching; pub mod hashing; pub mod native_packages; pub mod pypi; +pub mod resource_headers; pub mod static_dist; /// An enumeration of possible errors during download operations. diff --git a/clang-tools-manager/src/downloader/pypi.rs b/clang-tools-manager/src/downloader/pypi.rs index be9bb1a5..86d6d56c 100644 --- a/clang-tools-manager/src/downloader/pypi.rs +++ b/clang-tools-manager/src/downloader/pypi.rs @@ -1,4 +1,10 @@ -use super::{DownloadError, caching::Cacher, download, hashing::HashAlgorithm}; +use super::{ + DownloadError, + caching::Cacher, + download, + hashing::HashAlgorithm, + resource_headers::{self, ResourceHeadersError}, +}; use crate::{ClangTool, progress_bar::ProgressBar, utils::lock_path}; use semver::{Version, VersionReq}; @@ -54,6 +60,10 @@ pub enum PyPiDownloadError { /// Error that indicates the expected executable was not found in the downloaded wheel. #[error("Expected executable not found in the downloaded wheel")] ExecutableNotFound, + + /// Errors that occur when extracting clang's resource headers from the downloaded wheel. + #[error("Failed to extract clang resource headers: {0}")] + ResourceHeaders(#[from] ResourceHeadersError), } /// Represents the information of a package on PyPI @@ -540,6 +550,19 @@ impl PyPiDownloader { Some(dir) => dir.join(&bin_name), }; Self::extract_bin(clang_tool, &cached_wheel, &extracted_bin)?; + if clang_tool.needs_resource_headers() + && !resource_headers::has_resource_headers(&extracted_bin, ver.major) + { + log::info!("Extracting clang resource headers from the {clang_tool} wheel"); + let package = clang_tool.as_str().replace('-', "_"); + match resource_headers::extract_from_wheel(&cached_wheel, &package, &extracted_bin) { + Err(ResourceHeadersError::NotFound(_)) => log::warn!( + "The {clang_tool} v{ver_str} wheel has no clang resource headers (like stddef.h). \ + {clang_tool} may fail to parse sources that include standard headers." + ), + result => result?, + } + } file_lock.unlock()?; Ok(extracted_bin) } @@ -876,7 +899,7 @@ mod test { use super::{BIN_PATH_IN_WHEEL, make_wheel}; use crate::{ Cacher, ClangTool, PyPiDownloader, - downloader::pypi::LinuxLibC, + downloader::{pypi::LinuxLibC, resource_headers}, test_utils_unix::{enable_logs, fake_exe, isolate_path, set_cache_dir}, }; @@ -924,18 +947,14 @@ mod test { assert_eq!(LinuxLibC::get_glibc_version(), None); } - /// Uses a pre-seeded cache (PyPI info and wheel), so no network access is needed. - #[tokio::test] - async fn download_cached_wheel_without_digest() { - let tmp = setup(Some("ldd (Ubuntu GLIBC 2.39-0ubuntu8.7) 2.39")); - let cache = TempDir::new().unwrap(); - set_cache_dir(cache.path()); + /// Seeds the cache with PyPI info about `tool` v17.0.6 (without digest) + /// and its wheel (with the given `entries`), so no network access is needed. + fn seed_cached_wheel(tool: &ClangTool, entries: &[(&str, &[u8])]) { let pypi_cache = PyPiDownloader::get_cache_dir().join("pypi"); fs::create_dir_all(&pypi_cache).unwrap(); - let filename = format!( - "clang_format-17.0.6-py2.py3-none-manylinux_2_17_{}.whl", - std::env::consts::ARCH - ); + let package = tool.as_str().replace('-', "_"); + let arch = std::env::consts::ARCH; + let filename = format!("{package}-17.0.6-py2.py3-none-manylinux_2_17_{arch}.whl"); let json = serde_json::json!({ "releases": { "17.0.6": [{ @@ -946,14 +965,22 @@ mod test { "yanked": false }] } - }) - .to_string(); - fs::write(pypi_cache.join("clang-format_pypi.json"), json).unwrap(); + }); + fs::write( + pypi_cache.join(format!("{tool}_pypi.json")), + json.to_string(), + ) + .unwrap(); + make_wheel(&pypi_cache.join(format!("{tool}_17.0.6.whl")), entries); + } + + #[tokio::test] + async fn download_cached_wheel_without_digest() { + let tmp = setup(Some("ldd (Ubuntu GLIBC 2.39-0ubuntu8.7) 2.39")); + let cache = TempDir::new().unwrap(); + set_cache_dir(cache.path()); let content = b"#!/bin/sh\necho \"clang-format version 17.0.6\"\n"; - make_wheel( - &pypi_cache.join("clang-format_17.0.6.whl"), - &[(BIN_PATH_IN_WHEEL, content)], - ); + seed_cached_wheel(&ClangTool::ClangFormat, &[(BIN_PATH_IN_WHEEL, content)]); let out_dir = tmp.path().join("out"); let req = VersionReq::parse("17").unwrap(); for _ in 0..2 { @@ -964,6 +991,46 @@ mod test { assert_eq!(bin, out_dir.join("clang-format-17")); assert_eq!(fs::read(&bin).unwrap(), content); } + // clang-format does not need the resource headers + assert!(!tmp.path().join("lib").exists()); + } + + #[tokio::test] + async fn download_tidy_with_resource_headers() { + let tmp = setup(Some("ldd (Ubuntu GLIBC 2.39-0ubuntu8.7) 2.39")); + let cache = TempDir::new().unwrap(); + set_cache_dir(cache.path()); + let header = "clang_tidy/data/lib/clang/17/include/stddef.h"; + seed_cached_wheel( + &ClangTool::ClangTidy, + &[("clang_tidy/data/bin/clang-tidy", b"bin"), (header, b"h")], + ); + let bin_dir = tmp.path().join("bin"); + let req = VersionReq::parse("17").unwrap(); + let bin = PyPiDownloader::download_tool(&ClangTool::ClangTidy, &req, Some(&bin_dir)) + .await + .unwrap(); + assert!(resource_headers::has_resource_headers(&bin, 17)); + assert!(tmp.path().join("lib/clang/17/include/stddef.h").is_file()); + } + + #[tokio::test] + async fn download_tidy_without_resource_headers() { + let tmp = setup(Some("ldd (Ubuntu GLIBC 2.39-0ubuntu8.7) 2.39")); + let cache = TempDir::new().unwrap(); + set_cache_dir(cache.path()); + seed_cached_wheel( + &ClangTool::ClangTidy, + &[("clang_tidy/data/bin/clang-tidy", b"bin")], + ); + let bin_dir = tmp.path().join("bin"); + let req = VersionReq::parse("17").unwrap(); + // only a warning is logged about the missing headers + let bin = PyPiDownloader::download_tool(&ClangTool::ClangTidy, &req, Some(&bin_dir)) + .await + .unwrap(); + assert_eq!(fs::read(&bin).unwrap(), b"bin"); + assert!(!resource_headers::has_resource_headers(&bin, 17)); } } } diff --git a/clang-tools-manager/src/downloader/resource_headers.rs b/clang-tools-manager/src/downloader/resource_headers.rs new file mode 100644 index 00000000..4acce99c --- /dev/null +++ b/clang-tools-manager/src/downloader/resource_headers.rs @@ -0,0 +1,299 @@ +//! A module to install clang's resource headers next to a downloaded clang-tidy binary. +//! +//! clang-tidy needs clang's builtin headers (like `stddef.h`) to parse C/C++ sources. +//! It looks for them in `/../lib/clang//include`, where `` +//! is the folder that holds the clang-tidy executable. +//! Without these headers, any source that includes a standard header fails to compile. + +use std::{ + fs, io, + path::{Component, Path, PathBuf}, +}; + +use flate2::read::GzDecoder; +use zip::{ZipArchive, result::ZipError}; + +use crate::utils::normalize_path; + +/// Errors that occur while installing clang's resource headers. +#[derive(Debug, thiserror::Error)] +pub enum ResourceHeadersError { + /// Failed to read the archive or write the extracted headers. + #[error("Failed to read or write clang resource headers: {0}")] + Io(#[from] io::Error), + + /// Failed to read the wheel (a ZIP archive) that holds the headers. + #[error("Failed to read clang resource headers from wheel: {0}")] + ZipArchive(#[from] ZipError), + + /// The archive has an entry that would be written outside the `lib/clang` folder. + #[error("Refusing to extract an unsafe path from archive: {0}")] + UnsafePath(PathBuf), + + /// The archive has no clang resource headers. + #[error("No clang resource headers found in {0}")] + NotFound(PathBuf), +} + +/// The `lib/clang` folder in which the executable at `bin` looks for its resource headers. +pub fn lib_clang_dir(bin: &Path) -> io::Result { + let bin = std::path::absolute(bin)?; + let bin_dir = bin.parent().unwrap_or(&bin); + Ok(normalize_path( + &bin_dir.join("..").join("lib").join("clang"), + )) +} + +/// Are the resource headers for LLVM's `major` version installed for the executable at `bin`? +/// +/// LLVM 15 and older name the headers' folder with the full version (`lib/clang/15.0.7`). +/// Newer versions use only the major version (`lib/clang/16`). +pub fn has_resource_headers(bin: &Path, major: u64) -> bool { + let Ok(entries) = lib_clang_dir(bin).and_then(fs::read_dir) else { + return false; + }; + let major = major.to_string(); + let prefix = format!("{major}."); + entries.flatten().any(|entry| { + let name = entry.file_name().to_string_lossy().to_string(); + (name == major || name.starts_with(&prefix)) + && entry.path().join("include").join("stddef.h").is_file() + }) +} + +/// Joins the `relative` path of an archive entry to the `lib_clang` folder. +/// +/// Fails if the `relative` path could point outside of the `lib_clang` folder. +fn dest_path(lib_clang: &Path, relative: &Path) -> Result { + if relative + .components() + .all(|c| matches!(c, Component::Normal(_))) + { + Ok(lib_clang.join(relative)) + } else { + Err(ResourceHeadersError::UnsafePath(relative.to_path_buf())) + } +} + +/// Extracts the resource headers from a clang-tidy `wheel` (`/data/lib/clang/**`) +/// into the [`lib_clang_dir()`] of the executable at `bin`. +/// +/// Wheels of clang-tidy v15 and older on PyPI do not have the headers. +/// In that case, a [`ResourceHeadersError::NotFound`] error is returned. +pub fn extract_from_wheel( + wheel: &Path, + package: &str, + bin: &Path, +) -> Result<(), ResourceHeadersError> { + let prefix = Path::new(package).join("data").join("lib").join("clang"); + let lib_clang = lib_clang_dir(bin)?; + let mut archive = ZipArchive::new(fs::File::open(wheel)?)?; + let mut found = false; + for i in 0..archive.len() { + let mut file = archive.by_index(i)?; + if !file.is_file() { + continue; + } + let Some(relative) = file + .enclosed_name() + .and_then(|name| name.strip_prefix(&prefix).ok().map(Path::to_path_buf)) + else { + continue; + }; + let dest = dest_path(&lib_clang, &relative)?; + if let Some(parent) = dest.parent() { + fs::create_dir_all(parent)?; + } + io::copy(&mut file, &mut fs::File::create(&dest)?)?; + found = true; + } + if found { + Ok(()) + } else { + Err(ResourceHeadersError::NotFound(wheel.to_path_buf())) + } +} + +/// Extracts a `clang-resource-headers-.tar.gz` `archive` (holding `lib/clang/**`) +/// into the [`lib_clang_dir()`] of the executable at `bin`. +/// +/// These archives are published with the static binaries since LLVM v12. +pub fn extract_from_tar_gz(archive: &Path, bin: &Path) -> Result<(), ResourceHeadersError> { + let lib_clang = lib_clang_dir(bin)?; + let mut tar = tar::Archive::new(GzDecoder::new(fs::File::open(archive)?)); + let mut found = false; + for entry in tar.entries()? { + let mut entry = entry?; + if !entry.header().entry_type().is_file() { + continue; + } + let path = entry + .path()? + .components() + .filter(|c| !matches!(c, Component::CurDir)) + .collect::(); + let Ok(relative) = path.strip_prefix(Path::new("lib").join("clang")) else { + continue; + }; + let dest = dest_path(&lib_clang, relative)?; + if let Some(parent) = dest.parent() { + fs::create_dir_all(parent)?; + } + entry.unpack(&dest)?; + found = true; + } + if found { + Ok(()) + } else { + Err(ResourceHeadersError::NotFound(archive.to_path_buf())) + } +} + +#[cfg(test)] +mod tests { + use std::{fs, io::Write, path::Path}; + + use flate2::{Compression, write::GzEncoder}; + use tempfile::TempDir; + + use super::{ + ResourceHeadersError, extract_from_tar_gz, extract_from_wheel, has_resource_headers, + lib_clang_dir, + }; + + const STDDEF: &[u8] = b"/* stddef.h */"; + + /// Creates a gzipped tar file at `path` with the given `entries` (name, content). + fn make_tar_gz(path: &Path, entries: &[(&str, &[u8])]) { + let mut tar = tar::Builder::new(GzEncoder::new( + fs::File::create(path).unwrap(), + Compression::fast(), + )); + for (name, content) in entries { + let mut header = tar::Header::new_gnu(); + header.set_size(content.len() as u64); + header.set_mode(0o644); + // `append_data()` would reject unsafe paths, so the name is set directly. + header.as_gnu_mut().unwrap().name[..name.len()].copy_from_slice(name.as_bytes()); + header.set_cksum(); + tar.append(&header, *content).unwrap(); + } + tar.into_inner().unwrap().finish().unwrap(); + } + + /// Creates a (uncompressed) zip file at `path` with the given `entries` (name, content). + fn make_wheel(path: &Path, entries: &[(&str, &[u8])]) { + let mut zip = zip::ZipWriter::new(fs::File::create(path).unwrap()); + let options = zip::write::SimpleFileOptions::default() + .compression_method(zip::CompressionMethod::Stored); + for (name, content) in entries { + zip.start_file(*name, options).unwrap(); + zip.write_all(content).unwrap(); + } + zip.finish().unwrap(); + } + + #[test] + fn lib_clang_next_to_bin_dir() { + let tmp = TempDir::new().unwrap(); + let bin = tmp.path().join("bin").join("clang-tidy-21"); + assert_eq!( + lib_clang_dir(&bin).unwrap(), + tmp.path().join("lib").join("clang") + ); + } + + #[test] + fn headers_by_version() { + let tmp = TempDir::new().unwrap(); + let bin = tmp.path().join("bin").join("clang-tidy-21"); + assert!(!has_resource_headers(&bin, 21)); + let lib_clang = tmp.path().join("lib").join("clang"); + + // a folder without `stddef.h` does not count + fs::create_dir_all(lib_clang.join("21").join("include")).unwrap(); + assert!(!has_resource_headers(&bin, 21)); + + // LLVM 16+ use only the major version + fs::write(lib_clang.join("21/include/stddef.h"), STDDEF).unwrap(); + assert!(has_resource_headers(&bin, 21)); + assert!(!has_resource_headers(&bin, 2)); + + // LLVM 15 and older use the full version + fs::create_dir_all(lib_clang.join("15.0.7").join("include")).unwrap(); + fs::write(lib_clang.join("15.0.7/include/stddef.h"), STDDEF).unwrap(); + assert!(has_resource_headers(&bin, 15)); + assert!(!has_resource_headers(&bin, 1)); + } + + #[test] + fn from_wheel() { + let tmp = TempDir::new().unwrap(); + let wheel = tmp.path().join("clang_tidy.whl"); + make_wheel( + &wheel, + &[ + ("clang_tidy/data/bin/clang-tidy", b"binary"), + ("clang_tidy/data/lib/clang/21/include/stddef.h", STDDEF), + ("clang_tidy/__init__.py", b"ignored"), + ], + ); + let bin = tmp.path().join("bin").join("clang-tidy-21"); + extract_from_wheel(&wheel, "clang_tidy", &bin).unwrap(); + assert!(has_resource_headers(&bin, 21)); + assert_eq!( + fs::read(tmp.path().join("lib/clang/21/include/stddef.h")).unwrap(), + STDDEF + ); + assert!(!tmp.path().join("lib/clang/__init__.py").exists()); + } + + #[test] + fn from_wheel_without_headers() { + let tmp = TempDir::new().unwrap(); + let wheel = tmp.path().join("clang_tidy.whl"); + make_wheel(&wheel, &[("clang_tidy/data/bin/clang-tidy", b"binary")]); + let bin = tmp.path().join("bin").join("clang-tidy-14"); + let err = extract_from_wheel(&wheel, "clang_tidy", &bin).unwrap_err(); + assert!(matches!(err, ResourceHeadersError::NotFound(_))); + assert!(!has_resource_headers(&bin, 14)); + } + + #[test] + fn from_tar_gz() { + let tmp = TempDir::new().unwrap(); + let archive = tmp.path().join("clang-resource-headers-15.tar.gz"); + make_tar_gz( + &archive, + &[ + ("./lib/clang/15.0.7/include/stddef.h", STDDEF), + ("LICENSE.TXT", b"ignored"), + ], + ); + let bin = tmp.path().join("bin").join("clang-tidy-15"); + extract_from_tar_gz(&archive, &bin).unwrap(); + assert!(has_resource_headers(&bin, 15)); + assert!(!tmp.path().join("lib/clang/LICENSE.TXT").exists()); + } + + #[test] + fn from_tar_gz_without_headers() { + let tmp = TempDir::new().unwrap(); + let archive = tmp.path().join("empty.tar.gz"); + make_tar_gz(&archive, &[("LICENSE.TXT", b"ignored")]); + let bin = tmp.path().join("bin").join("clang-tidy-21"); + let err = extract_from_tar_gz(&archive, &bin).unwrap_err(); + assert!(matches!(err, ResourceHeadersError::NotFound(_))); + } + + #[test] + fn from_tar_gz_unsafe_path() { + let tmp = TempDir::new().unwrap(); + let archive = tmp.path().join("evil.tar.gz"); + make_tar_gz(&archive, &[("lib/clang/../../../evil.h", b"evil")]); + let bin = tmp.path().join("out").join("bin").join("clang-tidy-21"); + let err = extract_from_tar_gz(&archive, &bin).unwrap_err(); + assert!(matches!(err, ResourceHeadersError::UnsafePath(_))); + assert!(!tmp.path().join("evil.h").exists()); + } +} diff --git a/clang-tools-manager/src/downloader/static_dist.rs b/clang-tools-manager/src/downloader/static_dist.rs index 436f051c..301dbfbf 100644 --- a/clang-tools-manager/src/downloader/static_dist.rs +++ b/clang-tools-manager/src/downloader/static_dist.rs @@ -5,7 +5,7 @@ use std::{ ops::RangeInclusive, }; -use crate::{Cacher, DownloadError}; +use crate::{Cacher, DownloadError, ResourceHeadersError}; /// An error that can occur while downloading a static binary. #[derive(Debug, thiserror::Error)] @@ -34,6 +34,10 @@ pub enum StaticDistDownloadError { /// Failed to parse the SHA512 sum file. #[error("Failed to parse the SHA512 sum file")] Sha512Corruption, + + /// Failed to install clang's resource headers. + #[error("Failed to install clang resource headers: {0}")] + ResourceHeaders(#[from] ResourceHeadersError), } const MIN_CLANG_TOOLS_VERSION: &str = env!("MIN_CLANG_TOOLS_VERSION"); @@ -98,7 +102,7 @@ mod supported_platform { use super::{StaticDistDownloadError, StaticDistDownloader}; use crate::{ Cacher, ClangTool, - downloader::{download, hashing::HashAlgorithm}, + downloader::{download, hashing::HashAlgorithm, resource_headers}, utils::lock_path, }; @@ -225,9 +229,57 @@ mod supported_platform { download(&sha512_url, &sha512_cache_path, 10).await?; } Self::verify_sha512(&download_path, &tool_url_path, &sha512_cache_path)?; + if tool.needs_resource_headers() + && !resource_headers::has_resource_headers(&download_path, ver.major) + { + Self::install_resource_headers( + &ver_str, + release_tag, + &base_url, + &sha512_cache_path, + &download_path, + ) + .await?; + } file_lock.unlock()?; Ok(download_path) } + + /// Installs clang's resource headers for the binary at `bin` + /// (see [`resource_headers::lib_clang_dir()`]). + /// + /// The headers' archive is downloaded from the release tagged `release_tag` + /// (at `base_url`) and verified with the release's `SHA512SUMS` file (at `sha512_path`). + async fn install_resource_headers( + major: &str, + release_tag: &str, + base_url: &str, + sha512_path: &Path, + bin: &Path, + ) -> Result<(), StaticDistDownloadError> { + let name = format!("clang-resource-headers-{major}.tar.gz"); + let archive = Self::get_cache_dir() + .join("static_dist") + .join(release_tag) + .join(&name); + if archive.exists() { + log::info!( + "Using cached clang resource headers from {:?}", + archive.to_string_lossy() + ); + } else { + let url = Url::parse(format!("{base_url}{name}").as_str())?; + log::info!("Downloading clang resource headers from {url}"); + download(&url, &archive, 60).await?; + } + Self::verify_sha512(&archive, &name, sha512_path)?; + log::info!( + "Extracting clang resource headers to {:?}", + resource_headers::lib_clang_dir(bin)?.to_string_lossy() + ); + resource_headers::extract_from_tar_gz(&archive, bin)?; + Ok(()) + } } #[cfg(test)] @@ -323,5 +375,74 @@ mod supported_platform { .unwrap_err(); assert!(matches!(err, StaticDistDownloadError::Sha512Corruption)); } + + /// Uses a pre-seeded cache (binary, resource headers and SHA512SUMS), + /// so no network access is needed. + #[tokio::test] + async fn cached_tidy_with_resource_headers() { + use flate2::{Compression, write::GzEncoder}; + use sha2::{Digest, Sha512}; + + let sha512 = |content: &[u8]| { + Sha512::digest(content) + .iter() + .map(|b| format!("{b:02x}")) + .collect::() + }; + let cache = TempDir::new().unwrap(); + unsafe { + std::env::set_var("CPP_LINTER_CACHE", cache.path()); + } + let req = VersionReq::parse("17").unwrap(); + let release_tag = StaticDistDownloader::find_suitable_version(&req).unwrap().1; + let release_cache = cache.path().join("static_dist").join(release_tag); + fs::create_dir_all(&release_cache).unwrap(); + + // the resource headers' archive + let mut tar = tar::Builder::new(GzEncoder::new(Vec::new(), Compression::fast())); + let mut header = tar::Header::new_gnu(); + header.set_size(1); + tar.append_data(&mut header, "lib/clang/17/include/stddef.h", &b"h"[..]) + .unwrap(); + let archive = tar.into_inner().unwrap().finish().unwrap(); + let archive_name = "clang-resource-headers-17.tar.gz"; + fs::write(release_cache.join(archive_name), &archive).unwrap(); + + // the binary + let out_dir = TempDir::new().unwrap(); + let bin_dir = out_dir.path().join("bin"); + fs::create_dir_all(&bin_dir).unwrap(); + let suffix = std::env::consts::EXE_SUFFIX; + let bin = bin_dir.join(format!("clang-tidy-17{suffix}")); + let content = b"not really clang-tidy"; + fs::write(&bin, content).unwrap(); + + let mut sums = format!("{} {archive_name}\n", sha512(&archive)); + for platform in ["linux", "macos", "windows"] { + for arch in ["amd64", "arm64"] { + sums.push_str(&format!( + "{} clang-tidy-17_{platform}-{arch}{suffix}\n", + sha512(content) + )); + } + } + fs::write(StaticDistDownloader::sha512_cache_path(release_tag), sums).unwrap(); + + let tool = ClangTool::ClangTidy; + let result = StaticDistDownloader::download_tool(&tool, &req, Some(&bin_dir)) + .await + .unwrap(); + assert_eq!(result, bin); + let stddef = out_dir.path().join("lib/clang/17/include/stddef.h"); + assert_eq!(fs::read(stddef).unwrap(), b"h"); + + // a corrupt archive is rejected + fs::remove_dir_all(out_dir.path().join("lib")).unwrap(); + fs::write(release_cache.join(archive_name), b"corrupt").unwrap(); + let err = StaticDistDownloader::download_tool(&tool, &req, Some(&bin_dir)) + .await + .unwrap_err(); + assert!(matches!(err, StaticDistDownloadError::DownloadError(_))); + } } } diff --git a/clang-tools-manager/src/lib.rs b/clang-tools-manager/src/lib.rs index ebab2bcb..71623f75 100644 --- a/clang-tools-manager/src/lib.rs +++ b/clang-tools-manager/src/lib.rs @@ -10,6 +10,7 @@ pub use downloader::{ DownloadError, caching::Cacher, pypi::{PyPiDownloadError, PyPiDownloader}, + resource_headers::ResourceHeadersError, static_dist::{StaticDistDownloadError, StaticDistDownloader}, }; diff --git a/clang-tools-manager/src/main.rs b/clang-tools-manager/src/main.rs index 5ef1e0ad..6f3dc18e 100644 --- a/clang-tools-manager/src/main.rs +++ b/clang-tools-manager/src/main.rs @@ -41,6 +41,9 @@ pub struct CliOptions { pub tool: Vec, /// The directory where the clang tools should be installed. + /// + /// clang-tidy's resource headers (like `stddef.h`) are + /// installed in `/../lib/clang`. #[arg(short, long)] pub directory: Option, diff --git a/clang-tools-manager/src/tool.rs b/clang-tools-manager/src/tool.rs index 5aff0e66..bd8c29bf 100644 --- a/clang-tools-manager/src/tool.rs +++ b/clang-tools-manager/src/tool.rs @@ -75,6 +75,14 @@ impl ClangTool { } } + /// Does this tool need clang's resource headers (like `stddef.h`) to work? + /// + /// clang-tidy parses C/C++ sources, so it needs them. + /// clang-format does not. + pub const fn needs_resource_headers(&self) -> bool { + matches!(self, ClangTool::ClangTidy) + } + /// Fetch the path to an executable clang tool for the specified `version`. /// /// If the executable is not found using the specified `version`, then the tool is diff --git a/clang-tools-manager/src/version.rs b/clang-tools-manager/src/version.rs index c9c832cc..131f24f8 100644 --- a/clang-tools-manager/src/version.rs +++ b/clang-tools-manager/src/version.rs @@ -2,7 +2,9 @@ use std::{path::PathBuf, str::FromStr}; use crate::{ Cacher, ClangTool, PyPiDownloadError, PyPiDownloader, - downloader::{native_packages::try_install_package, static_dist::StaticDistDownloader}, + downloader::{ + native_packages::try_install_package, resource_headers, static_dist::StaticDistDownloader, + }, tool::{GetClangPathError, GetClangVersionError}, utils::normalize_path, }; @@ -118,10 +120,16 @@ 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}-{min_ver}{bin_ext}")); - if cached_bin.exists() { + .join(format!("{tool}-{}{bin_ext}", min_ver.major)); + // A cached clang-tidy without resource headers is downloaded again, + // so the headers get installed. + if cached_bin.exists() + && (!tool.needs_resource_headers() + || resource_headers::has_resource_headers(&cached_bin, min_ver.major)) + { let version = tool.capture_version(&cached_bin)?; if version_req.matches(&version) { log::info!( @@ -530,18 +538,49 @@ mod tests { fake_clang_tool(env.bin.path(), "clang-format", "16.0.0"); let cached_bin_dir = env.cache.path().join("bin"); fs::create_dir_all(&cached_bin_dir).unwrap(); - let cached = fake_clang_tool(&cached_bin_dir, "clang-format-17.0.0", "17.0.6"); + let cached = fake_clang_tool(&cached_bin_dir, "clang-format-17", "17.0.6"); let result = env.eval(req("17"), 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(); + let cached_bin_dir = env.cache.path().join("bin"); + fs::create_dir_all(&cached_bin_dir).unwrap(); + let cached = fake_clang_tool(&cached_bin_dir, "clang-tidy-17", "17.0.6"); + let requested = req("17"); + let eval = || requested.eval_tool(&ClangTool::ClangTidy, false, None, false); + // A cached clang-tidy without resource headers is not used. + // Instead, it is downloaded again, which fails here because + // PyPI has no releases and the static binary's checksum does not match. + let pypi = env.cache.path().join("pypi"); + fs::create_dir_all(&pypi).unwrap(); + let no_releases = serde_json::json!({"releases": {}}).to_string(); + fs::write(pypi.join("clang-tidy_pypi.json"), no_releases).unwrap(); + let (_, release_tag) = + StaticDistDownloader::find_suitable_version(&VersionReq::parse("17").unwrap()) + .unwrap(); + let sums = StaticDistDownloader::sha512_cache_path(release_tag); + fs::create_dir_all(sums.parent().unwrap()).unwrap(); + fs::write(sums, "deadbeef clang-tidy-17_linux-amd64\n").unwrap(); + let err = eval().await.unwrap_err(); + assert!(matches!(err, GetToolError::NotFound)); + + let include = env.cache.path().join("lib/clang/17/include"); + fs::create_dir_all(&include).unwrap(); + fs::write(include.join("stddef.h"), "").unwrap(); + let result = eval().await.unwrap().unwrap(); + assert_version(&cached, result, "17.0.6"); + } + #[tokio::test] async fn eval_static_dist_fallback() { let env = TestEnv::new(); // cached version does not satisfy the requirement let cached_bin_dir = env.cache.path().join("bin"); fs::create_dir_all(&cached_bin_dir).unwrap(); - fake_clang_tool(&cached_bin_dir, "clang-format-17.0.0", "16.0.0"); + fake_clang_tool(&cached_bin_dir, "clang-format-17", "16.0.0"); env.seed_empty_pypi(); env.seed_static_dist(true); // no package managers are available. diff --git a/cspell.config.yml b/cspell.config.yml index 9b589e4a..40b5b4f6 100644 --- a/cspell.config.yml +++ b/cspell.config.yml @@ -11,6 +11,7 @@ words: - cdylib - choco - chrono + - cksum - clippy - codecov - codspeed @@ -119,6 +120,7 @@ words: - splitn - SRCROOT - startswith + - stddef - superfences - tada - taiki From 081c63dfa7f7e989445399a2078d40b5e7d7a2e7 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 29 Oct 2026 22:12:30 +0000 Subject: [PATCH 2/7] refactor: let tests reach every new resource-header line Log messages now name the path inline, so their arguments sit on lines that run even when info logging is off. The parent folder of an extracted header always exists, so drop the never-taken branch. --- .../src/downloader/resource_headers.rs | 10 ++++------ clang-tools-manager/src/downloader/static_dist.rs | 11 +++-------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/clang-tools-manager/src/downloader/resource_headers.rs b/clang-tools-manager/src/downloader/resource_headers.rs index 4acce99c..5e9eed7f 100644 --- a/clang-tools-manager/src/downloader/resource_headers.rs +++ b/clang-tools-manager/src/downloader/resource_headers.rs @@ -101,9 +101,8 @@ pub fn extract_from_wheel( continue; }; let dest = dest_path(&lib_clang, &relative)?; - if let Some(parent) = dest.parent() { - fs::create_dir_all(parent)?; - } + // `dest` is always inside `lib_clang`, so it always has a parent. + fs::create_dir_all(dest.parent().unwrap_or(&lib_clang))?; io::copy(&mut file, &mut fs::File::create(&dest)?)?; found = true; } @@ -136,9 +135,8 @@ pub fn extract_from_tar_gz(archive: &Path, bin: &Path) -> Result<(), ResourceHea continue; }; let dest = dest_path(&lib_clang, relative)?; - if let Some(parent) = dest.parent() { - fs::create_dir_all(parent)?; - } + // `dest` is always inside `lib_clang`, so it always has a parent. + fs::create_dir_all(dest.parent().unwrap_or(&lib_clang))?; entry.unpack(&dest)?; found = true; } diff --git a/clang-tools-manager/src/downloader/static_dist.rs b/clang-tools-manager/src/downloader/static_dist.rs index 301dbfbf..e34d1007 100644 --- a/clang-tools-manager/src/downloader/static_dist.rs +++ b/clang-tools-manager/src/downloader/static_dist.rs @@ -263,20 +263,15 @@ mod supported_platform { .join(release_tag) .join(&name); if archive.exists() { - log::info!( - "Using cached clang resource headers from {:?}", - archive.to_string_lossy() - ); + log::info!("Using cached clang resource headers from {archive:?}"); } else { let url = Url::parse(format!("{base_url}{name}").as_str())?; log::info!("Downloading clang resource headers from {url}"); download(&url, &archive, 60).await?; } Self::verify_sha512(&archive, &name, sha512_path)?; - log::info!( - "Extracting clang resource headers to {:?}", - resource_headers::lib_clang_dir(bin)?.to_string_lossy() - ); + let lib_clang = resource_headers::lib_clang_dir(bin)?; + log::info!("Extracting clang resource headers to {lib_clang:?}"); resource_headers::extract_from_tar_gz(&archive, bin)?; Ok(()) } From f6887b684ef23f2fee5ab2c34f642adbf00e8d32 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 29 Oct 2026 22:14:01 +0000 Subject: [PATCH 3/7] chore: add flate (from flate2) to the cspell words --- cspell.config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/cspell.config.yml b/cspell.config.yml index 40b5b4f6..ec696a5f 100644 --- a/cspell.config.yml +++ b/cspell.config.yml @@ -39,6 +39,7 @@ words: - eprecated - esac - Falsey + - flate - fontawesome - gitea - gitmodules From 873692d34561a8f5ed56a051f5baf1770687f85a Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 29 Oct 2026 16:03:34 -0700 Subject: [PATCH 4/7] remove cksum from cspell config Instead, ignore the one line where `set_cksum()` is used. --- clang-tools-manager/src/downloader/resource_headers.rs | 2 +- cspell.config.yml | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/clang-tools-manager/src/downloader/resource_headers.rs b/clang-tools-manager/src/downloader/resource_headers.rs index 5e9eed7f..b6c3b55e 100644 --- a/clang-tools-manager/src/downloader/resource_headers.rs +++ b/clang-tools-manager/src/downloader/resource_headers.rs @@ -173,7 +173,7 @@ mod tests { header.set_mode(0o644); // `append_data()` would reject unsafe paths, so the name is set directly. header.as_gnu_mut().unwrap().name[..name.len()].copy_from_slice(name.as_bytes()); - header.set_cksum(); + header.set_cksum(); // spell-checker: disable-line tar.append(&header, *content).unwrap(); } tar.into_inner().unwrap().finish().unwrap(); diff --git a/cspell.config.yml b/cspell.config.yml index ec696a5f..9aede7e1 100644 --- a/cspell.config.yml +++ b/cspell.config.yml @@ -11,7 +11,6 @@ words: - cdylib - choco - chrono - - cksum - clippy - codecov - codspeed From 405bdb6c7fc10a4b93931b8f617eb49c143297e6 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:14:20 +0000 Subject: [PATCH 5/7] fix: never keep partly extracted resource headers Extract the headers into a staging folder (lib/clang.partial) first, then move each version folder into lib/clang. Before, an extraction that failed midway could leave stddef.h behind, and a later run would accept the incomplete folder as installed. --- .../src/downloader/resource_headers.rs | 175 ++++++++++++------ 1 file changed, 121 insertions(+), 54 deletions(-) diff --git a/clang-tools-manager/src/downloader/resource_headers.rs b/clang-tools-manager/src/downloader/resource_headers.rs index b6c3b55e..6bef2dcc 100644 --- a/clang-tools-manager/src/downloader/resource_headers.rs +++ b/clang-tools-manager/src/downloader/resource_headers.rs @@ -61,20 +61,56 @@ pub fn has_resource_headers(bin: &Path, major: u64) -> bool { }) } -/// Joins the `relative` path of an archive entry to the `lib_clang` folder. +/// Joins the `relative` path of an archive entry to the `dir` folder. /// -/// Fails if the `relative` path could point outside of the `lib_clang` folder. -fn dest_path(lib_clang: &Path, relative: &Path) -> Result { +/// Fails if the `relative` path could point outside of the `dir` folder. +fn dest_path(dir: &Path, relative: &Path) -> Result { if relative .components() .all(|c| matches!(c, Component::Normal(_))) { - Ok(lib_clang.join(relative)) + Ok(dir.join(relative)) } else { Err(ResourceHeadersError::UnsafePath(relative.to_path_buf())) } } +/// Installs resource headers into the `lib_clang` folder, all or nothing. +/// +/// `extract` writes the headers' version folders (like `21/include/stddef.h`) into +/// the given staging folder, and returns `false` if the `archive` has no headers. +/// Each extracted version folder then replaces the one in `lib_clang`. +/// So a failed extraction never leaves a partial folder that [`has_resource_headers()`] +/// would accept. +fn install_staged( + lib_clang: &Path, + archive: &Path, + extract: impl FnOnce(&Path) -> Result, +) -> Result<(), ResourceHeadersError> { + let staging = lib_clang.with_file_name("clang.partial"); + if staging.exists() { + // left over from an extraction that was interrupted + fs::remove_dir_all(&staging)?; + } + let found = extract(&staging).inspect_err(|_| { + let _ = fs::remove_dir_all(&staging); + })?; + if !found { + return Err(ResourceHeadersError::NotFound(archive.to_path_buf())); + } + fs::create_dir_all(lib_clang)?; + for entry in fs::read_dir(&staging)? { + let entry = entry?; + let dest = lib_clang.join(entry.file_name()); + if dest.is_dir() { + fs::remove_dir_all(&dest)?; + } + fs::rename(entry.path(), dest)?; + } + fs::remove_dir(&staging)?; + Ok(()) +} + /// Extracts the resource headers from a clang-tidy `wheel` (`/data/lib/clang/**`) /// into the [`lib_clang_dir()`] of the executable at `bin`. /// @@ -86,31 +122,28 @@ pub fn extract_from_wheel( bin: &Path, ) -> Result<(), ResourceHeadersError> { let prefix = Path::new(package).join("data").join("lib").join("clang"); - let lib_clang = lib_clang_dir(bin)?; - let mut archive = ZipArchive::new(fs::File::open(wheel)?)?; - let mut found = false; - for i in 0..archive.len() { - let mut file = archive.by_index(i)?; - if !file.is_file() { - continue; + install_staged(&lib_clang_dir(bin)?, wheel, |staging| { + let mut archive = ZipArchive::new(fs::File::open(wheel)?)?; + let mut found = false; + for i in 0..archive.len() { + let mut file = archive.by_index(i)?; + if !file.is_file() { + continue; + } + let Some(relative) = file + .enclosed_name() + .and_then(|name| name.strip_prefix(&prefix).ok().map(Path::to_path_buf)) + else { + continue; + }; + let dest = dest_path(staging, &relative)?; + // `dest` is always inside `staging`, so it always has a parent. + fs::create_dir_all(dest.parent().unwrap_or(staging))?; + io::copy(&mut file, &mut fs::File::create(&dest)?)?; + found = true; } - let Some(relative) = file - .enclosed_name() - .and_then(|name| name.strip_prefix(&prefix).ok().map(Path::to_path_buf)) - else { - continue; - }; - let dest = dest_path(&lib_clang, &relative)?; - // `dest` is always inside `lib_clang`, so it always has a parent. - fs::create_dir_all(dest.parent().unwrap_or(&lib_clang))?; - io::copy(&mut file, &mut fs::File::create(&dest)?)?; - found = true; - } - if found { - Ok(()) - } else { - Err(ResourceHeadersError::NotFound(wheel.to_path_buf())) - } + Ok(found) + }) } /// Extracts a `clang-resource-headers-.tar.gz` `archive` (holding `lib/clang/**`) @@ -118,33 +151,30 @@ pub fn extract_from_wheel( /// /// These archives are published with the static binaries since LLVM v12. pub fn extract_from_tar_gz(archive: &Path, bin: &Path) -> Result<(), ResourceHeadersError> { - let lib_clang = lib_clang_dir(bin)?; - let mut tar = tar::Archive::new(GzDecoder::new(fs::File::open(archive)?)); - let mut found = false; - for entry in tar.entries()? { - let mut entry = entry?; - if !entry.header().entry_type().is_file() { - continue; + install_staged(&lib_clang_dir(bin)?, archive, |staging| { + let mut tar = tar::Archive::new(GzDecoder::new(fs::File::open(archive)?)); + let mut found = false; + for entry in tar.entries()? { + let mut entry = entry?; + if !entry.header().entry_type().is_file() { + continue; + } + let path = entry + .path()? + .components() + .filter(|c| !matches!(c, Component::CurDir)) + .collect::(); + let Ok(relative) = path.strip_prefix(Path::new("lib").join("clang")) else { + continue; + }; + let dest = dest_path(staging, relative)?; + // `dest` is always inside `staging`, so it always has a parent. + fs::create_dir_all(dest.parent().unwrap_or(staging))?; + entry.unpack(&dest)?; + found = true; } - let path = entry - .path()? - .components() - .filter(|c| !matches!(c, Component::CurDir)) - .collect::(); - let Ok(relative) = path.strip_prefix(Path::new("lib").join("clang")) else { - continue; - }; - let dest = dest_path(&lib_clang, relative)?; - // `dest` is always inside `lib_clang`, so it always has a parent. - fs::create_dir_all(dest.parent().unwrap_or(&lib_clang))?; - entry.unpack(&dest)?; - found = true; - } - if found { - Ok(()) - } else { - Err(ResourceHeadersError::NotFound(archive.to_path_buf())) - } + Ok(found) + }) } #[cfg(test)] @@ -294,4 +324,41 @@ mod tests { assert!(matches!(err, ResourceHeadersError::UnsafePath(_))); assert!(!tmp.path().join("evil.h").exists()); } + + #[test] + fn failed_extraction_leaves_no_partial_headers() { + let tmp = TempDir::new().unwrap(); + let archive = tmp.path().join("bad.tar.gz"); + make_tar_gz( + &archive, + &[ + ("lib/clang/21/include/stddef.h", STDDEF), + ("lib/clang/../../evil.h", b"evil"), + ], + ); + let bin = tmp.path().join("bin").join("clang-tidy-21"); + let err = extract_from_tar_gz(&archive, &bin).unwrap_err(); + assert!(matches!(err, ResourceHeadersError::UnsafePath(_))); + assert!(!has_resource_headers(&bin, 21)); + assert!(!tmp.path().join("lib/clang.partial").exists()); + } + + #[test] + fn extraction_replaces_old_headers() { + let tmp = TempDir::new().unwrap(); + let lib = tmp.path().join("lib"); + // a partial install from an older version of this crate + fs::create_dir_all(lib.join("clang/21/include")).unwrap(); + fs::write(lib.join("clang/21/include/stale.h"), b"stale").unwrap(); + // a staging folder left over from an interrupted extraction + fs::create_dir_all(lib.join("clang.partial/21")).unwrap(); + + let archive = tmp.path().join("clang-resource-headers-21.tar.gz"); + make_tar_gz(&archive, &[("lib/clang/21/include/stddef.h", STDDEF)]); + let bin = tmp.path().join("bin").join("clang-tidy-21"); + extract_from_tar_gz(&archive, &bin).unwrap(); + assert!(has_resource_headers(&bin, 21)); + assert!(!lib.join("clang/21/include/stale.h").exists()); + assert!(!lib.join("clang.partial").exists()); + } } From 3300cc3a53951a948fc37b6dbbf539ad57f89a1f Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:16:56 +0000 Subject: [PATCH 6/7] fix: match the exact resource headers folder and drop bad archives LLVM 15 and older look for their headers in lib/clang/, so a 15.0.6 headers folder does not serve a 15.0.7 binary. Check the exact folder name of the binary's version (cached binaries are now checked after reading their version). Also delete a cached headers archive that fails its checksum, so the next run downloads it again instead of failing the same way. --- clang-tools-manager/src/downloader/pypi.rs | 12 +++- .../src/downloader/resource_headers.rs | 58 +++++++++++-------- .../src/downloader/static_dist.rs | 8 ++- clang-tools-manager/src/version.rs | 14 ++--- 4 files changed, 56 insertions(+), 36 deletions(-) diff --git a/clang-tools-manager/src/downloader/pypi.rs b/clang-tools-manager/src/downloader/pypi.rs index 86d6d56c..10a92d2a 100644 --- a/clang-tools-manager/src/downloader/pypi.rs +++ b/clang-tools-manager/src/downloader/pypi.rs @@ -551,7 +551,7 @@ impl PyPiDownloader { }; Self::extract_bin(clang_tool, &cached_wheel, &extracted_bin)?; if clang_tool.needs_resource_headers() - && !resource_headers::has_resource_headers(&extracted_bin, ver.major) + && !resource_headers::has_resource_headers(&extracted_bin, &ver) { log::info!("Extracting clang resource headers from the {clang_tool} wheel"); let package = clang_tool.as_str().replace('-', "_"); @@ -1010,7 +1010,10 @@ mod test { let bin = PyPiDownloader::download_tool(&ClangTool::ClangTidy, &req, Some(&bin_dir)) .await .unwrap(); - assert!(resource_headers::has_resource_headers(&bin, 17)); + assert!(resource_headers::has_resource_headers( + &bin, + &semver::Version::new(17, 0, 0) + )); assert!(tmp.path().join("lib/clang/17/include/stddef.h").is_file()); } @@ -1030,7 +1033,10 @@ mod test { .await .unwrap(); assert_eq!(fs::read(&bin).unwrap(), b"bin"); - assert!(!resource_headers::has_resource_headers(&bin, 17)); + assert!(!resource_headers::has_resource_headers( + &bin, + &semver::Version::new(17, 0, 0) + )); } } } diff --git a/clang-tools-manager/src/downloader/resource_headers.rs b/clang-tools-manager/src/downloader/resource_headers.rs index 6bef2dcc..58b74e16 100644 --- a/clang-tools-manager/src/downloader/resource_headers.rs +++ b/clang-tools-manager/src/downloader/resource_headers.rs @@ -11,6 +11,7 @@ use std::{ }; use flate2::read::GzDecoder; +use semver::Version; use zip::{ZipArchive, result::ZipError}; use crate::utils::normalize_path; @@ -44,20 +45,26 @@ pub fn lib_clang_dir(bin: &Path) -> io::Result { )) } -/// Are the resource headers for LLVM's `major` version installed for the executable at `bin`? +/// The name of the folder (in [`lib_clang_dir()`]) with the resource headers of LLVM `version`. /// -/// LLVM 15 and older name the headers' folder with the full version (`lib/clang/15.0.7`). -/// Newer versions use only the major version (`lib/clang/16`). -pub fn has_resource_headers(bin: &Path, major: u64) -> bool { - let Ok(entries) = lib_clang_dir(bin).and_then(fs::read_dir) else { - return false; - }; - let major = major.to_string(); - let prefix = format!("{major}."); - entries.flatten().any(|entry| { - let name = entry.file_name().to_string_lossy().to_string(); - (name == major || name.starts_with(&prefix)) - && entry.path().join("include").join("stddef.h").is_file() +/// LLVM 15 and older name it with the full version (`15.0.7`). +/// Newer versions use only the major version (`16`). +fn resource_dir_name(version: &Version) -> String { + if version.major > 15 { + version.major.to_string() + } else { + format!("{}.{}.{}", version.major, version.minor, version.patch) + } +} + +/// Are the resource headers for LLVM `version` installed for the executable at `bin`? +pub fn has_resource_headers(bin: &Path, version: &Version) -> bool { + lib_clang_dir(bin).is_ok_and(|lib_clang| { + lib_clang + .join(resource_dir_name(version)) + .join("include") + .join("stddef.h") + .is_file() }) } @@ -182,6 +189,7 @@ mod tests { use std::{fs, io::Write, path::Path}; use flate2::{Compression, write::GzEncoder}; + use semver::Version; use tempfile::TempDir; use super::{ @@ -235,23 +243,25 @@ mod tests { fn headers_by_version() { let tmp = TempDir::new().unwrap(); let bin = tmp.path().join("bin").join("clang-tidy-21"); - assert!(!has_resource_headers(&bin, 21)); + let v21 = Version::new(21, 1, 8); + assert!(!has_resource_headers(&bin, &v21)); let lib_clang = tmp.path().join("lib").join("clang"); // a folder without `stddef.h` does not count fs::create_dir_all(lib_clang.join("21").join("include")).unwrap(); - assert!(!has_resource_headers(&bin, 21)); + assert!(!has_resource_headers(&bin, &v21)); // LLVM 16+ use only the major version fs::write(lib_clang.join("21/include/stddef.h"), STDDEF).unwrap(); - assert!(has_resource_headers(&bin, 21)); - assert!(!has_resource_headers(&bin, 2)); + assert!(has_resource_headers(&bin, &v21)); + assert!(has_resource_headers(&bin, &Version::new(21, 1, 0))); + assert!(!has_resource_headers(&bin, &Version::new(2, 0, 0))); // LLVM 15 and older use the full version fs::create_dir_all(lib_clang.join("15.0.7").join("include")).unwrap(); fs::write(lib_clang.join("15.0.7/include/stddef.h"), STDDEF).unwrap(); - assert!(has_resource_headers(&bin, 15)); - assert!(!has_resource_headers(&bin, 1)); + assert!(has_resource_headers(&bin, &Version::new(15, 0, 7))); + assert!(!has_resource_headers(&bin, &Version::new(15, 0, 6))); } #[test] @@ -268,7 +278,7 @@ mod tests { ); let bin = tmp.path().join("bin").join("clang-tidy-21"); extract_from_wheel(&wheel, "clang_tidy", &bin).unwrap(); - assert!(has_resource_headers(&bin, 21)); + assert!(has_resource_headers(&bin, &Version::new(21, 0, 0))); assert_eq!( fs::read(tmp.path().join("lib/clang/21/include/stddef.h")).unwrap(), STDDEF @@ -284,7 +294,7 @@ mod tests { let bin = tmp.path().join("bin").join("clang-tidy-14"); let err = extract_from_wheel(&wheel, "clang_tidy", &bin).unwrap_err(); assert!(matches!(err, ResourceHeadersError::NotFound(_))); - assert!(!has_resource_headers(&bin, 14)); + assert!(!has_resource_headers(&bin, &Version::new(14, 0, 0))); } #[test] @@ -300,7 +310,7 @@ mod tests { ); let bin = tmp.path().join("bin").join("clang-tidy-15"); extract_from_tar_gz(&archive, &bin).unwrap(); - assert!(has_resource_headers(&bin, 15)); + assert!(has_resource_headers(&bin, &Version::new(15, 0, 7))); assert!(!tmp.path().join("lib/clang/LICENSE.TXT").exists()); } @@ -339,7 +349,7 @@ mod tests { let bin = tmp.path().join("bin").join("clang-tidy-21"); let err = extract_from_tar_gz(&archive, &bin).unwrap_err(); assert!(matches!(err, ResourceHeadersError::UnsafePath(_))); - assert!(!has_resource_headers(&bin, 21)); + assert!(!has_resource_headers(&bin, &Version::new(21, 0, 0))); assert!(!tmp.path().join("lib/clang.partial").exists()); } @@ -357,7 +367,7 @@ mod tests { make_tar_gz(&archive, &[("lib/clang/21/include/stddef.h", STDDEF)]); let bin = tmp.path().join("bin").join("clang-tidy-21"); extract_from_tar_gz(&archive, &bin).unwrap(); - assert!(has_resource_headers(&bin, 21)); + assert!(has_resource_headers(&bin, &Version::new(21, 0, 0))); assert!(!lib.join("clang/21/include/stale.h").exists()); assert!(!lib.join("clang.partial").exists()); } diff --git a/clang-tools-manager/src/downloader/static_dist.rs b/clang-tools-manager/src/downloader/static_dist.rs index e34d1007..fb7ba112 100644 --- a/clang-tools-manager/src/downloader/static_dist.rs +++ b/clang-tools-manager/src/downloader/static_dist.rs @@ -230,7 +230,7 @@ mod supported_platform { } Self::verify_sha512(&download_path, &tool_url_path, &sha512_cache_path)?; if tool.needs_resource_headers() - && !resource_headers::has_resource_headers(&download_path, ver.major) + && !resource_headers::has_resource_headers(&download_path, &ver) { Self::install_resource_headers( &ver_str, @@ -269,7 +269,10 @@ mod supported_platform { log::info!("Downloading clang resource headers from {url}"); download(&url, &archive, 60).await?; } - Self::verify_sha512(&archive, &name, sha512_path)?; + Self::verify_sha512(&archive, &name, sha512_path).inspect_err(|_| { + // so the next run downloads it again + let _ = fs::remove_file(&archive); + })?; let lib_clang = resource_headers::lib_clang_dir(bin)?; log::info!("Extracting clang resource headers to {lib_clang:?}"); resource_headers::extract_from_tar_gz(&archive, bin)?; @@ -438,6 +441,7 @@ mod supported_platform { .await .unwrap_err(); assert!(matches!(err, StaticDistDownloadError::DownloadError(_))); + assert!(!release_cache.join(archive_name).exists()); } } } diff --git a/clang-tools-manager/src/version.rs b/clang-tools-manager/src/version.rs index 131f24f8..c91bc129 100644 --- a/clang-tools-manager/src/version.rs +++ b/clang-tools-manager/src/version.rs @@ -124,14 +124,14 @@ impl RequestedVersion { let cached_bin = StaticDistDownloader::get_cache_dir() .join("bin") .join(format!("{tool}-{}{bin_ext}", min_ver.major)); - // A cached clang-tidy without resource headers is downloaded again, - // so the headers get installed. - if cached_bin.exists() - && (!tool.needs_resource_headers() - || resource_headers::has_resource_headers(&cached_bin, min_ver.major)) - { + if cached_bin.exists() { let version = tool.capture_version(&cached_bin)?; - if version_req.matches(&version) { + // A cached clang-tidy without resource headers is downloaded again, + // so the headers get installed. + if version_req.matches(&version) + && (!tool.needs_resource_headers() + || resource_headers::has_resource_headers(&cached_bin, &version)) + { log::info!( "Found {tool} version {version} in cache at path: {:?}", cached_bin.to_string_lossy() From 60c23f2f64489beca418f7d2c73ea0d2c515d26b Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:21:17 +0000 Subject: [PATCH 7/7] fix: lock lib/clang while installing resource headers Binaries of different versions in one folder share lib/clang and its staging folder, and their downloads hold different locks. --- clang-tools-manager/src/downloader/resource_headers.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clang-tools-manager/src/downloader/resource_headers.rs b/clang-tools-manager/src/downloader/resource_headers.rs index 58b74e16..f2c3c59e 100644 --- a/clang-tools-manager/src/downloader/resource_headers.rs +++ b/clang-tools-manager/src/downloader/resource_headers.rs @@ -14,7 +14,7 @@ use flate2::read::GzDecoder; use semver::Version; use zip::{ZipArchive, result::ZipError}; -use crate::utils::normalize_path; +use crate::utils::{lock_path, normalize_path}; /// Errors that occur while installing clang's resource headers. #[derive(Debug, thiserror::Error)] @@ -94,6 +94,9 @@ fn install_staged( archive: &Path, extract: impl FnOnce(&Path) -> Result, ) -> Result<(), ResourceHeadersError> { + // Binaries of different versions in one folder share `lib_clang` and the staging folder. + // The lock is released when dropped (at return). + let _lock = lock_path(lib_clang)?; let staging = lib_clang.with_file_name("clang.partial"); if staging.exists() { // left over from an extraction that was interrupted