From ef58f0a7c85a45bf828a7758b07d8f8e8f801ed3 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Sat, 31 Oct 2026 07:42:43 +0000 Subject: [PATCH] feat: try system package managers before PyPI when allowed --- clang-tools-manager/README.md | 10 ++++---- clang-tools-manager/src/main.rs | 10 ++++---- clang-tools-manager/src/version.rs | 37 +++++++++++++++++++++++------- cpp-linter/src/cli/mod.rs | 10 ++++---- 4 files changed, 44 insertions(+), 23 deletions(-) diff --git a/clang-tools-manager/README.md b/clang-tools-manager/README.md index 4ffae2c4..2127923e 100644 --- a/clang-tools-manager/README.md +++ b/clang-tools-manager/README.md @@ -79,12 +79,12 @@ Options: By default, this matches the value of a CI environment variable. For non-CI contexts, this allows users to opt-in to using - system package managers as a fallback in case PyPI offerings are - unsatisfactory. + system package managers. - If system package managers are not allowed or fail, then - static binaries built by cpp-linter are sought - (for compatible platforms). + When allowed, system package managers are tried first. + If they are not allowed or fail, then the clang tools are + downloaded from PyPI, and then from the static binaries + built by cpp-linter (for compatible platforms). --no-mod-sys Strictly disallow using system package managers. diff --git a/clang-tools-manager/src/main.rs b/clang-tools-manager/src/main.rs index 6f3dc18e..484274e3 100644 --- a/clang-tools-manager/src/main.rs +++ b/clang-tools-manager/src/main.rs @@ -57,12 +57,12 @@ pub struct CliOptions { /// /// By default, this matches the value of a CI environment variable. /// For non-CI contexts, this allows users to opt-in to using - /// system package managers as a fallback in case PyPI offerings - /// are unsatisfactory. + /// system package managers. /// - /// If system package managers are not allowed or fail, then - /// static binaries built by cpp-linter are sought (for - /// compatible platforms). + /// When allowed, system package managers are tried first. + /// If they are not allowed or fail, then the clang tools are + /// downloaded from PyPI, and then from the static binaries + /// built by cpp-linter (for compatible platforms). #[arg(long, action = clap::ArgAction::SetTrue, conflicts_with = "no_mod_sys")] pub mod_sys: bool, diff --git a/clang-tools-manager/src/version.rs b/clang-tools-manager/src/version.rs index c91bc129..9c8cc527 100644 --- a/clang-tools-manager/src/version.rs +++ b/clang-tools-manager/src/version.rs @@ -143,17 +143,22 @@ impl RequestedVersion { } } - // try to download a suitable version + // 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}" + ), + } + } + + // 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}"); - if allow_system_package_manager - && let Some(result) = - try_install_package(tool, version_req, &min_ver).await? - { - return Ok(Some(result)); - } log::info!("Falling back to downloading {tool} static binaries."); match StaticDistDownloader::download_tool(tool, version_req, directory) .await @@ -603,9 +608,25 @@ mod tests { } #[tokio::test] - async fn eval_package_manager() { + async fn eval_package_manager_error_falls_back() { let env = TestEnv::new(); env.seed_empty_pypi(); + env.seed_static_dist(true); + // A fake `brew` that "installs" clang-format, but no clang-format executable appears. + fake_exe( + env.bin.path(), + "brew", + "if [ \"$1\" = \"list\" ]; then exit 1; fi", + ); + let result = env.eval(req("17"), true).await.unwrap().unwrap(); + assert_version(&env.out.path().join("clang-format-17"), result, "17.0.6"); + } + + #[tokio::test] + async fn eval_package_manager() { + let env = TestEnv::new(); + // PyPI is not seeded: the package manager is tried first, + // so nothing is downloaded from PyPI. let marker = env.bin.path().join("installed"); let marker = marker.to_string_lossy(); // A fake `brew` that "installs" clang-format v17 (by creating a marker file). diff --git a/cpp-linter/src/cli/mod.rs b/cpp-linter/src/cli/mod.rs index ef7e52e6..ac969c3f 100644 --- a/cpp-linter/src/cli/mod.rs +++ b/cpp-linter/src/cli/mod.rs @@ -141,12 +141,12 @@ pub struct GeneralOptions { /// /// By default, this matches the value of a CI environment variable. /// For non-CI contexts, this allows users to opt-in to using - /// system package managers as a fallback in case PyPI offerings - /// are unsatisfactory. + /// system package managers. /// - /// If system package managers are not allowed or fail, then - /// static binaries built by cpp-linter are sought (for - /// compatible platforms). + /// When allowed, system package managers are tried first. + /// If they are not allowed or fail, then the clang tools are + /// downloaded from PyPI, and then from the static binaries + /// built by cpp-linter (for compatible platforms). #[arg( long, default_missing_value = "true",