Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang-tools-manager/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ required-features = ["bin"]
[dependencies]
anyhow = { workspace = true, optional = true }
blake2 = "0.10.6"
clap = { workspace = true, features = ["derive"], optional = true }
clap = { workspace = true, features = ["derive", "env"], optional = true }
colored = { workspace = true, optional = true }
directories = "6.0.0"
log = { workspace = true }
Expand Down
34 changes: 33 additions & 1 deletion clang-tools-manager/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,26 @@ pub struct CliOptions {
/// This will only overwrite an existing symlink.
#[arg(short, long)]
pub force: bool,

/// Whether to use the system's available package managers.
///
/// 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.
///
/// If system package managers are not allowed or fail, then
/// static binaries built by cpp-linter are sought (for
/// compatible platforms).
#[arg(long, action = clap::ArgAction::SetTrue, conflicts_with = "no_mod_sys")]
pub mod_sys: bool,

/// Strictly disallow using system package managers.
///
/// This can be used to override the default behavior of `--mod-sys`,
/// useful in sensitive CI environments, like self-hosted runners.
#[arg(long, action = clap::ArgAction::SetTrue, conflicts_with = "mod_sys")]
pub no_mod_sys: bool,
}

#[tokio::main]
Expand All @@ -161,7 +181,19 @@ async fn main() -> Result<()> {
let mut map_tools = HashMap::new();
for t in tool {
if let Some(version) = req_ver
.eval_tool(&t, options.force, options.directory.as_ref())
.eval_tool(
&t,
options.force,
options.directory.as_ref(),
if options.no_mod_sys {
false // explicitly false
} else {
options.mod_sys // explicitly true
|| std::env::var("CI").is_ok_and(|v| {
["true", "on", "1"].contains(&v.to_lowercase().as_str())
}) // implicitly true in CI environments
},
)
.await?
{
map_tools.entry(t).or_insert(version);
Expand Down
27 changes: 20 additions & 7 deletions clang-tools-manager/src/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ impl RequestedVersion {
tool: &ClangTool,
overwrite_symlink: bool,
directory: Option<&PathBuf>,
allow_system_package_manager: bool,
) -> Result<Option<ClangVersion>, GetToolError> {
match self {
RequestedVersion::Path(_) => {
Expand Down Expand Up @@ -139,8 +140,9 @@ impl RequestedVersion {
Ok(bin) => bin,
Err(e) => {
log::error!("Failed to download {tool} {version_req} from PyPi: {e}");
if let Some(result) =
try_install_package(tool, version_req, &min_ver).await?
if allow_system_package_manager
&& let Some(result) =
try_install_package(tool, version_req, &min_ver).await?
{
return Ok(Some(result));
}
Expand Down Expand Up @@ -245,7 +247,7 @@ impl FromStr for RequestedVersion {

#[cfg(test)]
mod tests {
use std::{path::PathBuf, str::FromStr};
use std::{env, path::PathBuf, str::FromStr};

use semver::VersionReq;
use tempfile::TempDir;
Expand Down Expand Up @@ -281,7 +283,7 @@ mod tests {
#[tokio::test]
async fn eval_no_value() {
let result = RequestedVersion::NoValue
.eval_tool(&ClangTool::ClangFormat, false, None)
.eval_tool(&ClangTool::ClangFormat, false, None, false)
.await
.unwrap();
assert!(result.is_none());
Expand All @@ -301,14 +303,19 @@ mod tests {
let version_req =
VersionReq::parse(option_env!("MIN_CLANG_TOOLS_VERSION").unwrap_or("16")).unwrap();
let downloaded_clang = RequestedVersion::Requirement(version_req.clone())
.eval_tool(&tool, false, Some(&PathBuf::from(tmp_cache_dir.path())))
.eval_tool(
&tool,
false,
Some(&PathBuf::from(tmp_cache_dir.path())),
false,
)
.await
.unwrap()
.unwrap();
println!("Downloaded clang-format: {downloaded_clang:?}");
let req_ver = RequestedVersion::Path(downloaded_clang.path.parent().unwrap().to_owned());
let result = req_ver
.eval_tool(&tool, false, None)
.eval_tool(&tool, false, None, false)
.await
.unwrap()
.unwrap();
Expand All @@ -331,7 +338,13 @@ mod tests {
let version_req = VersionReq::parse(clang_version).unwrap();
println!("Installing {tool} with version requirement: {version_req}");
let clang_path = RequestedVersion::Requirement(version_req.clone())
.eval_tool(&tool, false, None)
.eval_tool(
&tool,
false,
None,
env::var("CI")
.is_ok_and(|v| ["true", "on", "1"].contains(&v.to_lowercase().as_str())),
)
.await
.unwrap()
.unwrap();
Expand Down
16 changes: 11 additions & 5 deletions cpp-linter/src/clang_tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,21 +94,27 @@ pub struct ClangVersions {

/// Runs clang-tidy and/or clang-format and returns the version used for each.
///
/// If `tidy_checks` is `"-*"` then clang-tidy is not executed.
/// If `style` is a blank string (`""`), then clang-format is not executed.
/// If [`ClangParams::tidy_checks`] is `"-*"` then clang-tidy is not executed.
/// If [`ClangParams::style`] is a blank string (`""`), then clang-format is not executed.
///
/// The `modify_system` parameter controls whether or not to use a systems' available
/// package managers when installing the specified `version` of clang tools.
///
/// The provided `rest_api_client` is only used for consistent logging messages.
pub async fn capture_clang_tools_output(
files: &[Arc<Mutex<FileObj>>],
version: &RequestedVersion,
mut clang_params: ClangParams,
rest_api_client: &RestClient,
modify_system: bool,
) -> Result<ClangVersions, ClangTaskError> {
let mut clang_versions = ClangVersions::default();
// find the executable paths for clang-tidy and/or clang-format and show version
// info as debugging output.
if clang_params.tidy_checks != "-*" {
let tool = ClangTool::ClangTidy;
let tool_info = version
.eval_tool(&tool, false, None)
.eval_tool(&tool, false, None, modify_system)
.await?
.ok_or(ClangTaskError::FindToolError(tool.as_str()))?;
log::info!(
Expand All @@ -123,7 +129,7 @@ pub async fn capture_clang_tools_output(
if !clang_params.style.is_empty() {
let tool = ClangTool::ClangFormat;
let tool_info = version
.eval_tool(&tool, false, None)
.eval_tool(&tool, false, None, modify_system)
.await?
.ok_or(ClangTaskError::FindToolError(tool.as_str()))?;
log::info!(
Expand Down Expand Up @@ -378,7 +384,7 @@ mod tests {
let rest_client = RestClient::new().unwrap();
#[cfg(feature = "bin")]
try_init();
capture_clang_tools_output(&[], &version, clang_params, &rest_client).await
capture_clang_tools_output(&[], &version, clang_params, &rest_client, false).await
}

#[tokio::test]
Expand Down
34 changes: 33 additions & 1 deletion cpp-linter/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ pub struct GeneralOptions {
)]
pub version: RequestedVersion,

/// This controls the action's verbosity in the workflow's logs.
/// This controls the log messages' verbosity.
///
/// This option does not affect the verbosity of resulting
/// thread comments or file annotations.
Expand All @@ -136,6 +136,38 @@ pub struct GeneralOptions {
)
)]
pub verbosity: Verbosity,

/// Whether to use the system's available package managers.
///
/// 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.
///
/// If system package managers are not allowed or fail, then
/// static binaries built by cpp-linter are sought (for
/// compatible platforms).
#[arg(
long,
default_missing_value = "false",
action = ArgAction::SetTrue,
value_parser = FalseyValueParser::new(),
conflicts_with = "no_mod_sys",
)]
pub mod_sys: bool,

/// Strictly disallow using the system's package managers.
///
/// This can be used to override the default behavior of `--mod-sys`,
/// useful in sensitive CI environments like self-hosted runners.
#[arg(
long,
default_missing_value = "false",
action = ArgAction::SetTrue,
value_parser = FalseyValueParser::new(),
conflicts_with = "mod_sys",
)]
pub no_mod_sys: bool,
}

/// A struct to describe the CLI's source options.
Expand Down
6 changes: 6 additions & 0 deletions cpp-linter/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
&cli.general_options.version,
clang_params,
&rest_api_client,
if cli.general_options.no_mod_sys {
false // explicitly false
} else {
cli.general_options.mod_sys // explicitly true
|| env::var("CI").is_ok_and(|v| ["true", "on", "1"].contains(&v.to_lowercase().as_str())) // implicitly true in CI environments
},
)
.await?;
rest_api_client.start_log_group("Posting feedback");
Expand Down
Loading