fix: make clang tool install logs say what actually happened

clang-tools-manager/src/downloader/native_packages/mod.rs

@@ -77,51 +77,37 @@ pub async fn try_install_package(
 ) -> Result<Option<ClangVersion>, GetToolError> {
     let os_pkg_managers = get_available_package_managers();
     if os_pkg_managers.is_empty() {
-        log::error!("No supported package managers found on the system.");
+        log::warn!("No supported package managers found on the system.");
         return Ok(None);
-    } else {
-        for mgr in os_pkg_managers {
-            log::info!("Trying to install {tool} v{min_version} using {mgr} package manager.");
-            let pkg_name = mgr.get_package_name(tool);
-            if mgr.is_installed_package(&pkg_name, Some(min_version))
-                && let Ok(path) =
-                    tool.get_exe_path(&RequestedVersion::Requirement(version_req.clone()))
-                && let Ok(version) = tool.capture_version(&path)
-                && version_req.matches(&version)
-            {
-                log::info!(
-                    "Found {tool} version matching {version_req} installed via {mgr} package manager."
+    }
+    for mgr in os_pkg_managers {
+        let pkg_name = mgr.get_package_name(tool);
+        if mgr.is_installed_package(&pkg_name, Some(min_version))
+            && let Ok(path) = tool.get_exe_path(&RequestedVersion::Requirement(version_req.clone()))
+            && let Ok(version) = tool.capture_version(&path)
+            && version_req.matches(&version)
+        {
+            log::info!("Found {tool} version {version} installed via {mgr} package manager.");
+            return Ok(Some(ClangVersion { version, path }));
+        }
+        log::info!("Installing {tool} {version_req} using {mgr} package manager.");
+        match mgr.install_package(&pkg_name, Some(min_version)).await {
+            Ok(()) => {
+                let path =
+                    tool.get_exe_path(&RequestedVersion::Requirement(version_req.clone()))?;
+                let version = tool.capture_version(&path)?;
+                if version_req.matches(&version) {
+                    log::info!("Installed {tool} version {version} using {mgr} package manager.");
+                    return Ok(Some(ClangVersion { version, path }));
+                }
+                log::warn!(
+                    "{mgr} package manager provides {tool} version {version}, which does not match the requirement {version_req}."
                 );
-                return Ok(Some(ClangVersion { version, path }));
-            } else {
-                log::info!(
-                    "{mgr} package manager does not have a version of {tool} matching {version_req} installed."
+            }
+            Err(e) => {
+                log::warn!(
+                    "Failed to install {tool} {version_req} using {mgr} package manager: {e}"
                 );
-                match mgr.install_package(&pkg_name, Some(min_version)).await {
-                    Ok(()) => {
-                        log::info!(
-                            "Successfully installed {tool} v{min_version} using {mgr} package manager."
-                        );
-                        let path =
-                            tool.get_exe_path(&RequestedVersion::Requirement(version_req.clone()))?;
-                        let version = tool.capture_version(&path)?;
-                        if version_req.matches(&version) {
-                            log::info!(
-                                "Installed {tool} version {version} matches the requirement {version_req}."
-                            );
-                            return Ok(Some(ClangVersion { version, path }));
-                        } else {
-                            log::error!(
-                                "Installed {tool} version {version} does not match the requirement {version_req}."
-                            );
-                        }
-                    }
-                    Err(e) => {
-                        log::error!(
-                            "Failed to install {tool} v{min_version} using {mgr} package manager: {e}"
-                        );
-                    }
-                }
             }
         }
     }

clang-tools-manager/src/downloader/native_packages/unix.rs

@@ -155,12 +155,12 @@ impl PackageManager for UnixPackageManager {
             if matches!(self, UnixPackageManager::Apt)
                 && let Some(version) = version
             {
-                log::error!(
-                    "Failed to install {package_id} via apt: {}",
-                    String::from_utf8_lossy(&output.stderr)
-                );
                 log::info!(
-                    "trying to install from official LLVM PPA repository (for Debian-based `apt` package manager)"
+                    "`apt install {package_id}` failed; trying the official LLVM apt repository."
+                );
+                log::debug!(
+                    "`apt install {package_id}` failed: {}",
+                    String::from_utf8_lossy(&output.stderr)
                 );
                 return llvm_apt_install::install_llvm_via_apt(
                     version.major.to_string(),
@@ -182,10 +182,13 @@ impl PackageManager for UnixPackageManager {
         match self {
             #[cfg(target_os = "linux")]
             UnixPackageManager::Apt => {
-                let output = cmd
-                    .args(["list", "--installed", package_id.as_str()])
+                // `apt list --installed <pkg>` exits with 0 even if the package is not installed.
+                let output = Command::new("dpkg-query")
+                    .args(["--show", "--showformat=${db:Status-Status}"])
+                    .arg(package_id.as_str())
                     .output();
-                output.is_ok_and(|out| out.status.success())
+                output
+                    .is_ok_and(|out| out.status.success() && out.stdout.as_slice() == b"installed")
             }
             #[cfg(target_os = "linux")]
             UnixPackageManager::Dnf => {
@@ -387,6 +390,11 @@ mod tests {
         let tmp = setup();
         let ver = Version::new(17, 0, 0);
         for mgr in all_managers() {
+            #[cfg(target_os = "linux")]
+            if matches!(mgr, UnixPackageManager::Apt) {
+                // apt is checked with `dpkg-query`, which is tested separately.
+                continue;
+            }
             // package manager not found
             assert!(!mgr.is_installed_package("clang", Some(&ver)));
             fake_exe(tmp.path(), mgr.as_str(), SUCCESS);
@@ -396,6 +404,25 @@ mod tests {
         }
     }
 
+    #[cfg(target_os = "linux")]
+    #[test]
+    fn is_installed_apt_package() {
+        let tmp = setup();
+        let mgr = UnixPackageManager::Apt;
+        let ver = Version::new(17, 0, 0);
+        // `dpkg-query` not found
+        assert!(!mgr.is_installed_package("clang-format", Some(&ver)));
+        // package is installed
+        fake_exe(tmp.path(), "dpkg-query", "printf installed");
+        assert!(mgr.is_installed_package("clang-format", Some(&ver)));
+        // package is known but not installed (`dpkg-query` still exits with 0)
+        fake_exe(tmp.path(), "dpkg-query", "printf not-installed");
+        assert!(!mgr.is_installed_package("clang-format", Some(&ver)));
+        // package is unknown
+        fake_exe(tmp.path(), "dpkg-query", FAILURE);
+        assert!(!mgr.is_installed_package("clang-format", None));
+    }
+
     #[tokio::test]
     async fn install_with_sudo() {
         let tmp = setup();

clang-tools-manager/src/version.rs

@@ -115,6 +115,10 @@ impl RequestedVersion {
                         );
                         return Ok(Some(ClangVersion { version, path }));
                     }
+                    log::info!(
+                        "Found {tool} version {version} at path: {:?}, which does not match the requirement {version_req}.",
+                        path.to_string_lossy()
+                    );
                 }
 
                 // check if cache has a suitable version
@@ -152,20 +156,26 @@ impl RequestedVersion {
 
                 // try a system package manager first (if allowed)
                 if allow_system_package_manager {
-                    match try_install_package(tool, version_req, &min_ver).await {
-                        Ok(Some(result)) => return Ok(Some(result)),
-                        Ok(None) => {}
-                        Err(e) => log::error!(
-                            "Failed to install {tool} {version_req} using a system package manager: {e}"
-                        ),
+                    if requires_exact_version(version_req) {
+                        log::info!(
+                            "Skipping system package managers: they only install {tool} by major version, but {version_req} requires an exact version."
+                        );
+                    } else {
+                        match try_install_package(tool, version_req, &min_ver).await {
+                            Ok(Some(result)) => return Ok(Some(result)),
+                            Ok(None) => {}
+                            Err(e) => log::warn!(
+                                "Failed to install {tool} {version_req} using a system package manager: {e}"
+                            ),
+                        }
                     }
                 }
 
                 // then try to download a suitable version from PyPI or the static binaries
                 let bin = match PyPiDownloader::download_tool(tool, version_req, directory).await {
                     Ok(bin) => bin,
                     Err(e) => {
-                        log::error!("Failed to download {tool} {version_req} from PyPi: {e}");
+                        log::warn!("Failed to download {tool} {version_req} from PyPi: {e}");
                         log::info!("Falling back to downloading {tool} static binaries.");
                         match StaticDistDownloader::download_tool(tool, version_req, directory)
                             .await
@@ -201,6 +211,19 @@ impl RequestedVersion {
     }
 }
 
+/// Returns true if `version_req` only allows specific minor or patch releases (eg. `=18.1.8` or `=18.1`).
+///
+/// All comparators must match, so one exact comparator (with a minor version) is enough.
+///
+/// System package managers can only install a tool by its major version,
+/// so they could install (and leave behind) a version that does not satisfy such a requirement.
+fn requires_exact_version(version_req: &VersionReq) -> bool {
+    version_req
+        .comparators
+        .iter()
+        .any(|c| c.op == semver::Op::Exact && c.minor.is_some())
+}
+
 /// Returns the lowest supported version that satisfies `version_req`.
 pub fn get_min_ver(version_req: &VersionReq) -> Option<Version> {
     StaticDistDownloader::get_major_version_range()
@@ -289,11 +312,30 @@ mod tests {
     use semver::VersionReq;
     use tempfile::TempDir;
 
-    use super::{RequestedVersion, RequestedVersionParsingError};
+    use super::{RequestedVersion, RequestedVersionParsingError, requires_exact_version};
     use crate::{ClangTool, utils::normalize_path};
 
     // See also crate::tool::tests module for other `RequestedVersion::from_str()` tests.
 
+    #[test]
+    fn exact_version_requirements() {
+        for (req, exact) in [
+            ("=18.1.8", true),
+            ("=18.1", true),
+            ("=18", false),
+            ("=17.0.6, >=17", true),
+            ("18.1.8", false),
+            (">=18.1, <18.2", false),
+            ("*", false),
+        ] {
+            assert_eq!(
+                requires_exact_version(&VersionReq::parse(req).unwrap()),
+                exact,
+                "{req}"
+            );
+        }
+    }
+
     #[test]
     fn lowest_match_in_major() {
         for (req, major, expected) in [
@@ -707,6 +749,26 @@ mod tests {
             assert!(matches!(err, GetToolError::NotFound));
         }
 
+        #[tokio::test]
+        async fn eval_exact_requirement_skips_package_manager() {
+            let env = TestEnv::new();
+            env.seed_empty_pypi();
+            env.seed_static_dist(true);
+            let marker = env.bin.path().join("installed");
+            // A fake `brew` that would "install" clang-format (by creating a marker file).
+            fake_exe(
+                env.bin.path(),
+                "brew",
+                &format!(
+                    "if [ \"$1\" = \"list\" ]; then exit 1; fi\n: > \"{}\"",
+                    marker.to_string_lossy()
+                ),
+            );
+            let result = env.eval(req("=17.0.6"), true).await.unwrap().unwrap();
+            assert_version(&env.out.path().join("clang-format-17"), result, "17.0.6");
+            assert!(!marker.exists());
+        }
+
         #[tokio::test]
         async fn eval_package_manager_error_falls_back() {
             let env = TestEnv::new();

cpp-linter/src/cli/mod.rs

@@ -96,6 +96,10 @@ pub struct GeneralOptions {
     /// Accepted options are:
     ///
     /// - A semantic version specifier, eg. `>=10, <13`, `=12.0.1`, or simply `16`.
+    ///   A version without an operator is a minimum within that major version:
+    ///   `18.1.8` accepts 18.1.8 or a newer 18.x release, and `16` accepts any 16.x release.
+    ///   Use `=18.1.8` to require exactly that version. System package managers
+    ///   are not used for such a version because they only install by major version.
     /// - A blank string (`''`) to use the platform's default
     ///   installed version.
     /// - A path to where the clang tools are

cspell.config.yml

@@ -23,6 +23,7 @@ words:
   - declval
   - devel
   - Doherty
+  - dpkg
   - dtolnay
   - eabi
   - ecure
@@ -118,6 +119,7 @@ words:
   - serde
   - Shen
   - shenxianpeng
+  - showformat
   - splitn
   - SRCROOT
   - startswith