diff --git a/clang-tools-manager/src/downloader/native_packages/windows.rs b/clang-tools-manager/src/downloader/native_packages/windows.rs index 61451157..1582d05e 100644 --- a/clang-tools-manager/src/downloader/native_packages/windows.rs +++ b/clang-tools-manager/src/downloader/native_packages/windows.rs @@ -1,5 +1,8 @@ #![cfg(target_os = "windows")] -use std::{fmt::Display, process::Command}; +use std::{ + fmt::Display, + process::{Command, Output}, +}; use semver::{Comparator, Version}; @@ -36,6 +39,61 @@ impl WindowsPackageManager { // WindowsPackageManager::Scoop => "scoop", } } + + /// Is the `package_name` listed in the `stdout` of this package manager's `list` command? + /// + /// If a `version` is given, then the listed version must be compatible with it + /// (as in `^version`). A missing or invalid listed version is treated as not installed. + fn is_listed(&self, stdout: &str, package_name: &str, version: Option<&Version>) -> bool { + let (header_lines, name_column) = match self { + // `choco list` starts with a line about chocolatey's version, + // then packages are listed as ` ` + WindowsPackageManager::Chocolatey => (1, 0), + // `winget list` starts with a table header and divider, + // then packages are listed as ` ` + WindowsPackageManager::Winget => (2, 1), + }; + for line in stdout.lines().skip(header_lines) { + let mut columns = line.split_ascii_whitespace().skip(name_column); + if columns.next() != Some(package_name) { + continue; + } + // found the package, check version + let Some(version) = version else { + // version not specified, just check if package is listed + return true; + }; + let ver_cmp = Comparator { + op: semver::Op::Caret, + major: version.major, + minor: Some(version.minor), + patch: Some(version.patch), + pre: version.pre.clone(), + }; + return columns + .next() + .and_then(|ver_str| Version::parse(ver_str).ok()) + .is_some_and(|ver| ver_cmp.matches(&ver)); + } + false + } + + /// Converts the `output` of this package manager's `install` command into a [`Result`]. + fn check_install( + &self, + package_name: &str, + output: &Output, + ) -> Result<(), PackageManagerError> { + if output.status.success() { + Ok(()) + } else { + Err(PackageManagerError::InstallationError { + manager: self.as_str().to_string(), + package: package_name.to_string(), + stderr: String::from_utf8_lossy(&output.stderr).to_string(), + }) + } + } } impl PackageManager for WindowsPackageManager { @@ -55,80 +113,15 @@ impl PackageManager for WindowsPackageManager { fn is_installed_package(&self, package_name: &str, version: Option<&Version>) -> bool { let mut cmd = Command::new(self.as_str()); - let ver_cmp = version.map(|v| Comparator { - op: semver::Op::Caret, - major: v.major, - minor: Some(v.minor), - patch: Some(v.patch), - pre: v.pre.clone(), - }); match self { - WindowsPackageManager::Chocolatey => { - let output = cmd.arg("list").arg(package_name).output(); - if let Ok(out) = output - && out.status.success() - { - let stdout = String::from_utf8_lossy(&out.stdout).to_string(); - // skip line about chocolatey version - let lines = stdout.lines().skip(1); - for line in lines { - // packages are listed as ` ` - let mut l = line.split_ascii_whitespace(); - if let Some(name) = l.next() - && name == package_name - { - // found the package, check version - if let Some(ver_cmp) = ver_cmp.clone() { - if let Some(ver_str) = l.next() - && let Ok(ver) = Version::parse(ver_str) - { - return ver_cmp.matches(&ver); - } else { - // version not found or invalid, treat as not installed - return false; - } - } else { - // version not specified, just check if package is listed - return true; - } - } - } - } - false - } - WindowsPackageManager::Winget => { - let output = cmd.arg("list").arg("--id").arg(package_name).output(); - if let Ok(out) = output - && out.status.success() - { - // skip first 2 lines of table header and divider - let stdout = String::from_utf8_lossy(&out.stdout).to_string(); - let lines = stdout.lines().skip(2); - for line in lines { - // packages are listed as ` ` - let mut l = line.split_ascii_whitespace().skip(1); - if let Some(name) = l.next() - && name == package_name - { - // found the package, check version - if let Some(ver_cmp) = ver_cmp.clone() { - if let Some(ver_str) = l.next() - && let Ok(ver) = Version::parse(ver_str) - { - return ver_cmp.matches(&ver); - } else { - // version not found or invalid, treat as not installed - return false; - } - } else { - // version not specified, just check if package is listed - return true; - } - } - } - } - false + WindowsPackageManager::Chocolatey => cmd.arg("list").arg(package_name), + WindowsPackageManager::Winget => cmd.arg("list").arg("--id").arg(package_name), + }; + match cmd.output() { + Ok(out) if out.status.success() => { + self.is_listed(&String::from_utf8_lossy(&out.stdout), package_name, version) } + _ => false, } } @@ -151,36 +144,95 @@ impl PackageManager for WindowsPackageManager { if let Some(version) = version { cmd.arg("--version").arg(version.to_string()); } - let output = cmd.output()?; - if output.status.success() { - Ok(()) - } else { - Err(PackageManagerError::InstallationError { - manager: self.as_str().to_string(), - package: package_name.to_string(), - stderr: String::from_utf8_lossy(&output.stderr).to_string(), - }) - } } Self::Winget => { - let mut cmd = Command::new("winget"); cmd.arg("install").arg("--id").arg(package_name); if let Some(version) = version { // winget does not allow installing an older version of a package that // is already installed (with a newer version). So use `--force` to reinstall the specified version. cmd.arg("--version").arg(version.to_string()).arg("--force"); } - let output = cmd.output()?; - if output.status.success() { - Ok(()) - } else { - Err(PackageManagerError::InstallationError { - manager: self.as_str().to_string(), - package: package_name.to_string(), - stderr: String::from_utf8_lossy(&output.stderr).to_string(), - }) - } } } + let output = cmd.output()?; + self.check_install(package_name, &output) + } +} + +#[cfg(test)] +mod tests { + use std::{os::windows::process::ExitStatusExt, process::ExitStatus}; + + use semver::Version; + + use super::{Output, PackageManagerError, WindowsPackageManager}; + + const CHOCO_LIST: &str = "Chocolatey v2.4.3\n\ + llvm 18.1.8\n\ + llvm-nightly not-a-version\n\ + 2 packages installed.\n"; + + const WINGET_LIST: &str = "Name Id Version Source\n\ + -----------------------------\n\ + LLVM LLVM.LLVM 18.1.8 winget\n\ + Broken LLVM.Broken\n"; + + #[test] + fn choco_listed() { + let choco = WindowsPackageManager::Chocolatey; + assert!(choco.is_listed(CHOCO_LIST, "llvm", None)); + assert!(choco.is_listed(CHOCO_LIST, "llvm", Some(&Version::new(18, 1, 0)))); + assert!(!choco.is_listed(CHOCO_LIST, "llvm", Some(&Version::new(19, 1, 0)))); + // the listed version is not valid semver + assert!(choco.is_listed(CHOCO_LIST, "llvm-nightly", None)); + assert!(!choco.is_listed(CHOCO_LIST, "llvm-nightly", Some(&Version::new(18, 1, 0)))); + // the first line is about chocolatey itself, not a package + assert!(!choco.is_listed(CHOCO_LIST, "Chocolatey", None)); + assert!(!choco.is_listed(CHOCO_LIST, "cmake", None)); + } + + #[test] + fn winget_listed() { + let winget = WindowsPackageManager::Winget; + assert!(winget.is_listed(WINGET_LIST, "LLVM.LLVM", None)); + assert!(winget.is_listed(WINGET_LIST, "LLVM.LLVM", Some(&Version::new(18, 1, 0)))); + assert!(!winget.is_listed(WINGET_LIST, "LLVM.LLVM", Some(&Version::new(17, 0, 6)))); + // no version is listed + assert!(winget.is_listed(WINGET_LIST, "LLVM.Broken", None)); + assert!(!winget.is_listed(WINGET_LIST, "LLVM.Broken", Some(&Version::new(18, 1, 0)))); + // the table header and divider are not packages + assert!(!winget.is_listed(WINGET_LIST, "Id", None)); + // the name column is not the ID + assert!(!winget.is_listed(WINGET_LIST, "LLVM", None)); + } + + fn output(code: u32, stderr: &str) -> Output { + Output { + status: ExitStatus::from_raw(code), + stdout: vec![], + stderr: stderr.as_bytes().to_vec(), + } + } + + #[test] + fn install_result() { + for mgr in [ + WindowsPackageManager::Chocolatey, + WindowsPackageManager::Winget, + ] { + assert!(mgr.check_install("llvm", &output(0, "")).is_ok()); + let err = mgr + .check_install("llvm", &output(1, "no such package")) + .unwrap_err(); + assert!(matches!(err, PackageManagerError::InstallationError { .. })); + // the message names the manager and package, and shows the stderr + assert_eq!( + err.to_string(), + format!( + "{} failed to install llvm package: no such package", + mgr.as_str() + ) + ); + } } } diff --git a/clang-tools-manager/src/downloader/pypi.rs b/clang-tools-manager/src/downloader/pypi.rs index 4714290b..be9bb1a5 100644 --- a/clang-tools-manager/src/downloader/pypi.rs +++ b/clang-tools-manager/src/downloader/pypi.rs @@ -233,23 +233,27 @@ struct PlatformTag { impl PlatformTag { /// Checks if the platform tag is compatible with the current system. pub fn is_compatible_with_system(&self) -> bool { - self.os.is_compatible_with_system() && { - let sys_arch = std::env::consts::ARCH; - match std::env::consts::OS { - "windows" => match sys_arch { - "x86_64" => self.arch == "amd64", - "aarch64" => self.arch == "arm64", - "x86" => self.arch == "x86", - _ => false, - }, - "macos" => match sys_arch { - "x86_64" => self.arch == "x86_64" || self.arch == "universal2", - "aarch64" => self.arch == "arm64" || self.arch == "universal2", - _ => false, - }, - "linux" => self.arch == sys_arch, + self.os.is_compatible_with_system() + && self.is_arch_compatible(std::env::consts::OS, std::env::consts::ARCH) + } + + /// Checks if the wheel's architecture runs on the given `sys_os` and `sys_arch` + /// (named as in [`std::env::consts`]). + fn is_arch_compatible(&self, sys_os: &str, sys_arch: &str) -> bool { + match sys_os { + "windows" => match sys_arch { + "x86_64" => self.arch == "amd64", + "aarch64" => self.arch == "arm64", + "x86" => self.arch == "x86", _ => false, - } + }, + "macos" => match sys_arch { + "x86_64" => self.arch == "x86_64" || self.arch == "universal2", + "aarch64" => self.arch == "arm64" || self.arch == "universal2", + _ => false, + }, + "linux" => self.arch == sys_arch, + _ => false, } } } @@ -680,6 +684,40 @@ mod test { assert_eq!(tag.arch.as_str(), "amd64"); } + #[test] + fn arch_compatibility() { + let compatible = |tag: &str, sys_os: &str, sys_arch: &str| { + PlatformTag::from_str(tag) + .unwrap() + .is_arch_compatible(sys_os, sys_arch) + }; + for (tag, sys_os, sys_arch, expected) in [ + ("win_amd64", "windows", "x86_64", true), + ("win_arm64", "windows", "aarch64", true), + ("win32", "windows", "x86", true), + ("win32", "windows", "x86_64", false), + ("win_amd64", "windows", "aarch64", false), + ("win_amd64", "windows", "riscv64", false), + ("macosx_10_9_x86_64", "macos", "x86_64", true), + ("macosx_11_0_arm64", "macos", "aarch64", true), + ("macosx_10_9_universal2", "macos", "x86_64", true), + ("macosx_10_9_universal2", "macos", "aarch64", true), + ("macosx_11_0_arm64", "macos", "x86_64", false), + ("macosx_10_9_x86_64", "macos", "aarch64", false), + ("macosx_10_9_universal2", "macos", "powerpc", false), + ("manylinux_2_17_x86_64", "linux", "x86_64", true), + ("manylinux_2_17_aarch64", "linux", "aarch64", true), + ("manylinux_2_17_x86_64", "linux", "aarch64", false), + ("manylinux_2_17_x86_64", "freebsd", "x86_64", false), + ] { + assert_eq!( + compatible(tag, sys_os, sys_arch), + expected, + "{tag} on {sys_os} {sys_arch}" + ); + } + } + #[test] fn win_missing_arch_tag() { // "win" prefix without an "_" suffix is malformed. diff --git a/clang-tools-manager/src/downloader/static_dist.rs b/clang-tools-manager/src/downloader/static_dist.rs index 7cbf6d1e..436f051c 100644 --- a/clang-tools-manager/src/downloader/static_dist.rs +++ b/clang-tools-manager/src/downloader/static_dist.rs @@ -158,6 +158,25 @@ mod supported_platform { .join("SHA512SUMS") } + /// The release asset name of the static binary for the given `tool`'s `major` + /// version on the given `os` and `arch` (named as in [`std::env::consts`]). + /// + /// For example, `clang-format-21_linux-amd64` or `clang-tidy-21_windows-arm64.exe`. + fn asset_name(tool: &ClangTool, major: &str, os: &str, arch: &str) -> String { + // we already gated unsupported architectures above, + // so we can assume it's either x86_64 or aarch64 here + let arch = match arch { + "aarch64" => "arm64", + _ => "amd64", + }; + let (platform, suffix) = match os { + "windows" => ("windows", ".exe"), + "macos" => ("macos", ""), + _ => ("linux", ""), + }; + format!("{tool}-{major}_{platform}-{arch}{suffix}") + } + /// Downloads the `requested_version` of the specified `tool` from a distribution of statically linked binaries. /// /// The distribution is maintained at . @@ -171,28 +190,10 @@ mod supported_platform { let (ver, release_tag) = Self::find_suitable_version(requested_version) .ok_or(StaticDistDownloadError::UnsupportedVersion)?; let ver_str = ver.major.to_string(); - // we already gated unsupported architectures above, - // so we can assume it's either x86_64 or aarch64 here - let arch = if cfg!(target_arch = "aarch64") { - "arm64" - } else { - "amd64" - }; - let platform = if cfg!(target_os = "windows") { - "windows" - } else if cfg!(target_os = "macos") { - "macos" - } else { - "linux" - }; - let base_url = format!("{CLANG_TOOLS_REPO}/releases/download/{release_tag}/"); - let suffix = if cfg!(target_os = "windows") { - ".exe" - } else { - "" - }; - let tool_url_path = format!("{tool}-{ver_str}_{platform}-{arch}{suffix}"); + let suffix = std::env::consts::EXE_SUFFIX; + let tool_url_path = + Self::asset_name(tool, &ver_str, std::env::consts::OS, std::env::consts::ARCH); let url = Url::parse(format!("{base_url}{tool_url_path}").as_str())?; let cache_path = Self::get_cache_dir(); let bin_name = format!("{tool}-{ver_str}{suffix}"); @@ -246,6 +247,24 @@ mod supported_platform { assert!(tag.starts_with(&format!("llvm-{}.", ver.major))); } + #[test] + fn asset_names() { + let tool = ClangTool::ClangTidy; + for (os, arch, expected) in [ + ("linux", "x86_64", "clang-tidy-21_linux-amd64"), + ("linux", "aarch64", "clang-tidy-21_linux-arm64"), + ("macos", "x86_64", "clang-tidy-21_macos-amd64"), + ("macos", "aarch64", "clang-tidy-21_macos-arm64"), + ("windows", "x86_64", "clang-tidy-21_windows-amd64.exe"), + ("windows", "aarch64", "clang-tidy-21_windows-arm64.exe"), + ] { + assert_eq!( + StaticDistDownloader::asset_name(&tool, "21", os, arch), + expected + ); + } + } + #[test] fn find_none() { let req_ver = VersionReq::parse("=8").unwrap(); diff --git a/cpp-linter/src/clang_tools/clang_tidy/test.rs b/cpp-linter/src/clang_tools/clang_tidy/test.rs index 7c51d1f6..32bd57e0 100644 --- a/cpp-linter/src/clang_tools/clang_tidy/test.rs +++ b/cpp-linter/src/clang_tools/clang_tidy/test.rs @@ -590,6 +590,26 @@ fn apply_exported_fixes() { ); } +#[test] +fn apply_fixes_without_matching_note() { + let tmp = tempfile::tempdir().unwrap(); + let target = tmp.path().join("demo.cpp"); + let content = b"line\n int y;\n"; + let fixes: ExportedFixes = serde_saphyr::from_str(&dummy_fixes(&target)).unwrap(); + // the only note is about another line (eg. its diagnostic's note was filtered out) + let mut notes = vec![dummy_note("demo.cpp", vec![])]; + notes[0].line = 1; + let mut logs = vec![]; + let mut paths = FixPaths::new(&target, tmp.path()); + let (patched, tidy_fixes) = apply_fixes(&fixes, &mut paths, content, &mut notes, &mut logs); + // the fix is still applied, but no note claims it + assert_eq!(String::from_utf8(patched).unwrap(), "line\n auto\nx y;\n"); + assert!(notes[0].fixed_lines.is_empty()); + // the fix for another file is still returned, without a note to explain it + assert_eq!(tidy_fixes.others.len(), 1); + assert_eq!(tidy_fixes.others[0].note, None); +} + #[test] fn fix_paths_resolve() { let tmp = tempfile::tempdir().unwrap();