Skip to content

Commit fd15454

Browse files
2bndy52bndy5
authored
feat: allow explicit use of package managers (#372)
The adds 2 new CLI options (for both `cpp-linter` and `clang-tools` binaries): 1. `--mod-sys` defaults to the value of a `CI` env var (if present); allows enabling package managers in non-CI contexts. 2. `--no-mod-sys` strictly disallow use of package-managers; overrides the default value of `--mod-sys`. Using both options in 1 invocation causes a conflict error. resolves #364 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `--mod-sys` and `--no-mod-sys` CLI flags to control whether system package managers are used for clang tool installation. System package manager usage is automatically enabled in CI environments unless explicitly disabled via `--no-mod-sys`. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 959ae38 commit fd15454

6 files changed

Lines changed: 104 additions & 15 deletions

File tree

‎clang-tools-manager/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ required-features = ["bin"]
2121
[dependencies]
2222
anyhow = { workspace = true, optional = true }
2323
blake2 = "0.10.6"
24-
clap = { workspace = true, features = ["derive"], optional = true }
24+
clap = { workspace = true, features = ["derive", "env"], optional = true }
2525
colored = { workspace = true, optional = true }
2626
directories = "6.0.0"
2727
log = { workspace = true }

‎clang-tools-manager/src/main.rs‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,26 @@ pub struct CliOptions {
138138
/// This will only overwrite an existing symlink.
139139
#[arg(short, long)]
140140
pub force: bool,
141+
142+
/// Whether to use the system's available package managers.
143+
///
144+
/// By default, this matches the value of a CI environment variable.
145+
/// For non-CI contexts, this allows users to opt-in to using
146+
/// system package managers as a fallback in case PyPI offerings
147+
/// are unsatisfactory.
148+
///
149+
/// If system package managers are not allowed or fail, then
150+
/// static binaries built by cpp-linter are sought (for
151+
/// compatible platforms).
152+
#[arg(long, action = clap::ArgAction::SetTrue, conflicts_with = "no_mod_sys")]
153+
pub mod_sys: bool,
154+
155+
/// Strictly disallow using system package managers.
156+
///
157+
/// This can be used to override the default behavior of `--mod-sys`,
158+
/// useful in sensitive CI environments, like self-hosted runners.
159+
#[arg(long, action = clap::ArgAction::SetTrue, conflicts_with = "mod_sys")]
160+
pub no_mod_sys: bool,
141161
}
142162

143163
#[tokio::main]
@@ -161,7 +181,19 @@ async fn main() -> Result<()> {
161181
let mut map_tools = HashMap::new();
162182
for t in tool {
163183
if let Some(version) = req_ver
164-
.eval_tool(&t, options.force, options.directory.as_ref())
184+
.eval_tool(
185+
&t,
186+
options.force,
187+
options.directory.as_ref(),
188+
if options.no_mod_sys {
189+
false // explicitly false
190+
} else {
191+
options.mod_sys // explicitly true
192+
|| std::env::var("CI").is_ok_and(|v| {
193+
["true", "on", "1"].contains(&v.to_lowercase().as_str())
194+
}) // implicitly true in CI environments
195+
},
196+
)
165197
.await?
166198
{
167199
map_tools.entry(t).or_insert(version);

‎clang-tools-manager/src/version.rs‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ impl RequestedVersion {
7878
tool: &ClangTool,
7979
overwrite_symlink: bool,
8080
directory: Option<&PathBuf>,
81+
allow_system_package_manager: bool,
8182
) -> Result<Option<ClangVersion>, GetToolError> {
8283
match self {
8384
RequestedVersion::Path(_) => {
@@ -139,8 +140,9 @@ impl RequestedVersion {
139140
Ok(bin) => bin,
140141
Err(e) => {
141142
log::error!("Failed to download {tool} {version_req} from PyPi: {e}");
142-
if let Some(result) =
143-
try_install_package(tool, version_req, &min_ver).await?
143+
if allow_system_package_manager
144+
&& let Some(result) =
145+
try_install_package(tool, version_req, &min_ver).await?
144146
{
145147
return Ok(Some(result));
146148
}
@@ -245,7 +247,7 @@ impl FromStr for RequestedVersion {
245247

246248
#[cfg(test)]
247249
mod tests {
248-
use std::{path::PathBuf, str::FromStr};
250+
use std::{env, path::PathBuf, str::FromStr};
249251

250252
use semver::VersionReq;
251253
use tempfile::TempDir;
@@ -281,7 +283,7 @@ mod tests {
281283
#[tokio::test]
282284
async fn eval_no_value() {
283285
let result = RequestedVersion::NoValue
284-
.eval_tool(&ClangTool::ClangFormat, false, None)
286+
.eval_tool(&ClangTool::ClangFormat, false, None, false)
285287
.await
286288
.unwrap();
287289
assert!(result.is_none());
@@ -301,14 +303,19 @@ mod tests {
301303
let version_req =
302304
VersionReq::parse(option_env!("MIN_CLANG_TOOLS_VERSION").unwrap_or("16")).unwrap();
303305
let downloaded_clang = RequestedVersion::Requirement(version_req.clone())
304-
.eval_tool(&tool, false, Some(&PathBuf::from(tmp_cache_dir.path())))
306+
.eval_tool(
307+
&tool,
308+
false,
309+
Some(&PathBuf::from(tmp_cache_dir.path())),
310+
false,
311+
)
305312
.await
306313
.unwrap()
307314
.unwrap();
308315
println!("Downloaded clang-format: {downloaded_clang:?}");
309316
let req_ver = RequestedVersion::Path(downloaded_clang.path.parent().unwrap().to_owned());
310317
let result = req_ver
311-
.eval_tool(&tool, false, None)
318+
.eval_tool(&tool, false, None, false)
312319
.await
313320
.unwrap()
314321
.unwrap();
@@ -331,7 +338,13 @@ mod tests {
331338
let version_req = VersionReq::parse(clang_version).unwrap();
332339
println!("Installing {tool} with version requirement: {version_req}");
333340
let clang_path = RequestedVersion::Requirement(version_req.clone())
334-
.eval_tool(&tool, false, None)
341+
.eval_tool(
342+
&tool,
343+
false,
344+
None,
345+
env::var("CI")
346+
.is_ok_and(|v| ["true", "on", "1"].contains(&v.to_lowercase().as_str())),
347+
)
335348
.await
336349
.unwrap()
337350
.unwrap();

‎cpp-linter/src/clang_tools/mod.rs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,21 +94,27 @@ pub struct ClangVersions {
9494

9595
/// Runs clang-tidy and/or clang-format and returns the version used for each.
9696
///
97-
/// If `tidy_checks` is `"-*"` then clang-tidy is not executed.
98-
/// If `style` is a blank string (`""`), then clang-format is not executed.
97+
/// If [`ClangParams::tidy_checks`] is `"-*"` then clang-tidy is not executed.
98+
/// If [`ClangParams::style`] is a blank string (`""`), then clang-format is not executed.
99+
///
100+
/// The `modify_system` parameter controls whether or not to use a systems' available
101+
/// package managers when installing the specified `version` of clang tools.
102+
///
103+
/// The provided `rest_api_client` is only used for consistent logging messages.
99104
pub async fn capture_clang_tools_output(
100105
files: &[Arc<Mutex<FileObj>>],
101106
version: &RequestedVersion,
102107
mut clang_params: ClangParams,
103108
rest_api_client: &RestClient,
109+
modify_system: bool,
104110
) -> Result<ClangVersions, ClangTaskError> {
105111
let mut clang_versions = ClangVersions::default();
106112
// find the executable paths for clang-tidy and/or clang-format and show version
107113
// info as debugging output.
108114
if clang_params.tidy_checks != "-*" {
109115
let tool = ClangTool::ClangTidy;
110116
let tool_info = version
111-
.eval_tool(&tool, false, None)
117+
.eval_tool(&tool, false, None, modify_system)
112118
.await?
113119
.ok_or(ClangTaskError::FindToolError(tool.as_str()))?;
114120
log::info!(
@@ -123,7 +129,7 @@ pub async fn capture_clang_tools_output(
123129
if !clang_params.style.is_empty() {
124130
let tool = ClangTool::ClangFormat;
125131
let tool_info = version
126-
.eval_tool(&tool, false, None)
132+
.eval_tool(&tool, false, None, modify_system)
127133
.await?
128134
.ok_or(ClangTaskError::FindToolError(tool.as_str()))?;
129135
log::info!(
@@ -378,7 +384,7 @@ mod tests {
378384
let rest_client = RestClient::new().unwrap();
379385
#[cfg(feature = "bin")]
380386
try_init();
381-
capture_clang_tools_output(&[], &version, clang_params, &rest_client).await
387+
capture_clang_tools_output(&[], &version, clang_params, &rest_client, false).await
382388
}
383389

384390
#[tokio::test]

‎cpp-linter/src/cli/mod.rs‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ pub struct GeneralOptions {
120120
)]
121121
pub version: RequestedVersion,
122122

123-
/// This controls the action's verbosity in the workflow's logs.
123+
/// This controls the log messages' verbosity.
124124
///
125125
/// This option does not affect the verbosity of resulting
126126
/// thread comments or file annotations.
@@ -136,6 +136,38 @@ pub struct GeneralOptions {
136136
)
137137
)]
138138
pub verbosity: Verbosity,
139+
140+
/// Whether to use the system's available package managers.
141+
///
142+
/// By default, this matches the value of a CI environment variable.
143+
/// For non-CI contexts, this allows users to opt-in to using
144+
/// system package managers as a fallback in case PyPI offerings
145+
/// are unsatisfactory.
146+
///
147+
/// If system package managers are not allowed or fail, then
148+
/// static binaries built by cpp-linter are sought (for
149+
/// compatible platforms).
150+
#[arg(
151+
long,
152+
default_missing_value = "false",
153+
action = ArgAction::SetTrue,
154+
value_parser = FalseyValueParser::new(),
155+
conflicts_with = "no_mod_sys",
156+
)]
157+
pub mod_sys: bool,
158+
159+
/// Strictly disallow using the system's package managers.
160+
///
161+
/// This can be used to override the default behavior of `--mod-sys`,
162+
/// useful in sensitive CI environments like self-hosted runners.
163+
#[arg(
164+
long,
165+
default_missing_value = "false",
166+
action = ArgAction::SetTrue,
167+
value_parser = FalseyValueParser::new(),
168+
conflicts_with = "mod_sys",
169+
)]
170+
pub no_mod_sys: bool,
139171
}
140172

141173
/// A struct to describe the CLI's source options.

‎cpp-linter/src/run.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,12 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
172172
&cli.general_options.version,
173173
clang_params,
174174
&rest_api_client,
175+
if cli.general_options.no_mod_sys {
176+
false // explicitly false
177+
} else {
178+
cli.general_options.mod_sys // explicitly true
179+
|| env::var("CI").is_ok_and(|v| ["true", "on", "1"].contains(&v.to_lowercase().as_str())) // implicitly true in CI environments
180+
},
175181
)
176182
.await?;
177183
rest_api_client.start_log_group("Posting feedback");

0 commit comments

Comments
 (0)