diff --git a/.config/nextest.toml b/.config/nextest.toml index d750fa1c..06e61cc5 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -32,10 +32,10 @@ default-filter = "(kind(test) + test(use_extra_args)) - package(clang-tools-mana # show log output from each failing test failure-output = "final" +# set slow-timeout to 30 seconds and +# terminate tests after hitting the period 8 times (30s x 8 = 4 minutes). +slow-timeout = { period = "30s", terminate-after = 8 } + [profile.all] # A profile to run all tests (including tests that run longer than 10 seconds) default-filter = "all() - test(#*eval_version)" - -# Revert slow-timeout to nextest default value. -# Otherwise, default profile value (10s) is inherited. -slow-timeout = "60s" diff --git a/Cargo.lock b/Cargo.lock index 7ce2d880..f9aca660 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -383,7 +383,6 @@ dependencies = [ "chrono", "clang-tools-manager", "clap", - "colored", "fast-glob", "futures", "git-bot-feedback", diff --git a/Cargo.toml b/Cargo.toml index 1bb9f4ca..21f93187 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,7 +22,6 @@ strip = "symbols" [workspace.dependencies] anyhow = "1.0.102" clap = { version = "4.6.1", features = ["derive"] } -colored = "3.1.1" log = { version = "0.4.31", features = ["std"] } mockito = "1.7.2" pyo3 = {version = "0.29.0", features = ["extension-module"] } diff --git a/clang-tools-manager/Cargo.toml b/clang-tools-manager/Cargo.toml index baae2962..1c1a9a63 100644 --- a/clang-tools-manager/Cargo.toml +++ b/clang-tools-manager/Cargo.toml @@ -24,8 +24,8 @@ required-features = ["bin"] [dependencies] anyhow = { workspace = true, optional = true } blake2 = "0.10.6" -clap = { workspace = true, features = ["derive"], optional = true } -colored = { workspace = true, optional = true } +clap = { workspace = true, optional = true } +colored = { version = "3.1.1", optional = true } directories = "6.0.0" log = { workspace = true } regex = { workspace = true } @@ -41,7 +41,6 @@ which = "8.0.2" zip = { version = "8.6.0", default-features = false, features = ["deflate"] } [dev-dependencies] -colored = { workspace = true } mockito = { workspace = true } reqwest = { workspace = true, features = ["default-tls"] } tempfile = { workspace = true } diff --git a/clang-tools-manager/README.md b/clang-tools-manager/README.md index c269bbdc..65d0101e 100644 --- a/clang-tools-manager/README.md +++ b/clang-tools-manager/README.md @@ -41,8 +41,15 @@ The `clang-tools` binary's Command Line Interface (CLI) is rather simple.
clang-tools --help

-```sh -Usage: clang-tools [OPTIONS] +```txt +Usage: clang-tools.exe [OPTIONS] [TOOL]... + +Arguments: + [TOOL]... + The clang tool to install + + [default: clang-format clang-tidy] + [possible values: clang-tidy, clang-format] Options: -v, --version [] @@ -56,12 +63,6 @@ Options: This will include more DEBUG level log messages. Without it, log level is set to INFO by default. - -t, --tool - The clang tool to install - - [default: "clang-format clang-tidy"] - [possible values: clang-tidy, clang-format] - -d, --directory The directory where the clang tools should be installed @@ -70,6 +71,24 @@ Options: This will only overwrite an existing symlink. + --mod-sys + 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). + + --no-mod-sys + 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. + -h, --help Print help (see a summary with '-h') ``` diff --git a/clang-tools-manager/src/lib.rs b/clang-tools-manager/src/lib.rs index 5fc2e4eb..3568cfd8 100644 --- a/clang-tools-manager/src/lib.rs +++ b/clang-tools-manager/src/lib.rs @@ -23,3 +23,5 @@ pub use version::{ClangVersion, GetToolError, RequestedVersion, RequestedVersion mod progress_bar; pub use progress_bar::ProgressBar; + +pub mod logger; diff --git a/cpp-linter/src/logger.rs b/clang-tools-manager/src/logger.rs similarity index 80% rename from cpp-linter/src/logger.rs rename to clang-tools-manager/src/logger.rs index 436f04ea..c90d47d8 100644 --- a/cpp-linter/src/logger.rs +++ b/clang-tools-manager/src/logger.rs @@ -6,9 +6,26 @@ use std::{ io::{Write, stdout}, }; +use clap::builder::{ + Styles, + styling::{AnsiColor, Color, Style}, +}; use colored::{Colorize, control::set_override}; use log::{Level, LevelFilter, Log, Metadata, Record}; +/// A color scheme to use for CLI `--help` output. +pub const CLI_HELP_STYLE: Styles = Styles::styled() + .usage(Style::new().fg_color(Some(Color::Ansi(AnsiColor::BrightBlue)))) + .header( + Style::new() + .fg_color(Some(Color::Ansi(AnsiColor::BrightBlue))) + .underline(), + ) + .placeholder(Style::new().fg_color(Some(Color::Ansi(AnsiColor::Yellow)))) + .literal(Style::new().fg_color(Some(Color::Ansi(AnsiColor::BrightGreen)))) + .context_value(Style::new().fg_color(Some(Color::Ansi(AnsiColor::Cyan)))) + .invalid(Style::new().fg_color(Some(Color::Ansi(AnsiColor::BrightRed)))); + #[derive(Default)] struct SimpleLogger; @@ -41,7 +58,7 @@ impl Log for SimpleLogger { .expect("Failed to flush log command in stdout"); } else if self.enabled(record.metadata()) { let module = record.module_path(); - if module.is_none_or(|v| v.starts_with("cpp_linter")) { + if module.is_none_or(|v| v.starts_with("cpp_linter") || v.starts_with("clang_tools")) { writeln!( stdout, "[{}]: {}", @@ -74,7 +91,7 @@ impl Log for SimpleLogger { /// The logging level defaults to [`LevelFilter::Info`]. /// This logs a debug message about [`SetLoggerError`](struct@log::SetLoggerError) /// if the `LOGGER` is already initialized. -pub fn try_init() { +pub fn try_init_logger() { let logger: SimpleLogger = SimpleLogger; if matches!( env::var("CPP_LINTER_COLOR").as_deref(), @@ -95,15 +112,14 @@ pub fn try_init() { mod test { use std::env; - use super::{SimpleLogger, try_init}; + use super::try_init_logger; #[test] fn trace_log() { unsafe { env::set_var("CPP_LINTER_COLOR", "true"); } - try_init(); - assert!(SimpleLogger::level_color(&log::Level::Trace).contains("TRACE")); + try_init_logger(); log::set_max_level(log::LevelFilter::Trace); log::trace!("A dummy log statement for code coverage"); } diff --git a/clang-tools-manager/src/main.rs b/clang-tools-manager/src/main.rs index 60875370..5ef1e0ad 100644 --- a/clang-tools-manager/src/main.rs +++ b/clang-tools-manager/src/main.rs @@ -1,101 +1,14 @@ use anyhow::Result; -use clang_tools_manager::{ClangTool, RequestedVersion}; +use clang_tools_manager::{ + ClangTool, RequestedVersion, + logger::{CLI_HELP_STYLE, try_init_logger}, +}; use clap::Parser; use std::{collections::HashMap, path::PathBuf, str::FromStr}; -mod logging { - use colored::{Colorize, control::set_override}; - use log::{Level, LevelFilter, Log, Metadata, Record}; - use std::{ - env, - io::{Write, stdout}, - }; - - struct SimpleLogger; - - impl SimpleLogger { - fn level_color(level: &Level) -> String { - let name = format!("{:>5}", level.as_str().to_uppercase()); - match level { - Level::Error => name.red().bold().to_string(), - Level::Warn => name.yellow().bold().to_string(), - Level::Info => name.green().bold().to_string(), - Level::Debug => name.blue().bold().to_string(), - Level::Trace => name.magenta().bold().to_string(), - } - } - } - - impl Log for SimpleLogger { - fn enabled(&self, metadata: &Metadata) -> bool { - metadata.level() <= log::max_level() - } - - fn log(&self, record: &Record) { - let mut stdout = stdout().lock(); - if record.target() == "CI_LOG_GROUPING" { - // this log is meant to manipulate a CI workflow's log grouping - writeln!(stdout, "{}", record.args()) - .expect("Failed to write log command to stdout"); - stdout - .flush() - .expect("Failed to flush log command in stdout"); - } else if self.enabled(record.metadata()) { - let module = record.module_path(); - if module.is_none_or(|v| { - v.starts_with("clang_tools_manager") || v.starts_with("clang_tools") - }) { - writeln!( - stdout, - "[{}]: {}", - Self::level_color(&record.level()), - record.args() - ) - .expect("Failed to write log message to stdout"); - } else if let Some(module) = module { - writeln!( - stdout, - "[{}]{{{}:{}}}: {}", - Self::level_color(&record.level()), - module, - record.line().unwrap_or_default(), - record.args() - ) - .expect("Failed to write detailed log message to stdout"); - } - stdout - .flush() - .expect("Failed to flush log message in stdout"); - } - } - - fn flush(&self) {} - } - - /// A function to initialize the private `LOGGER`. - /// - /// The logging level defaults to [`LevelFilter::Info`]. - /// This logs a debug message about [`SetLoggerError`](struct@log::SetLoggerError) - /// if the `LOGGER` is already initialized. - pub fn initialize_logger() { - let logger: SimpleLogger = SimpleLogger; - if env::var("CPP_LINTER_COLOR") - .as_deref() - .is_ok_and(|v| matches!(v, "on" | "1" | "true")) - { - set_override(true); - } - if let Err(e) = - log::set_boxed_logger(Box::new(logger)).map(|()| log::set_max_level(LevelFilter::Info)) - { - // logger singleton already instantiated. - // we'll just use whatever the current config is. - log::debug!("{e:?}"); - } - } -} #[derive(clap::Parser, Debug)] +#[command(styles(CLI_HELP_STYLE))] pub struct CliOptions { /// The desired version of clang to install. #[arg( @@ -122,12 +35,10 @@ pub struct CliOptions { /// The clang tool to install. #[arg( - short, - long, - value_delimiter = ' ', - default_value = "clang-format clang-tidy" + num_args = 0.., + default_values_t = vec![ClangTool::ClangFormat, ClangTool::ClangTidy], )] - pub tool: Option>, + pub tool: Vec, /// The directory where the clang tools should be installed. #[arg(short, long)] @@ -162,14 +73,11 @@ pub struct CliOptions { #[tokio::main] async fn main() -> Result<()> { - logging::initialize_logger(); + try_init_logger(); let options = CliOptions::parse(); if options.verbose { log::set_max_level(log::LevelFilter::Debug); } - let tool = options - .tool - .expect("--tool should have a default value: [clang-format, clang-tidy]"); match options.version.unwrap_or(RequestedVersion::default()) { RequestedVersion::NoValue => { log::info!( @@ -179,7 +87,7 @@ async fn main() -> Result<()> { } req_ver => { let mut map_tools = HashMap::new(); - for t in tool { + for t in options.tool { if let Some(version) = req_ver .eval_tool( &t, diff --git a/clang-tools-manager/tests/common.rs b/clang-tools-manager/tests/common.rs deleted file mode 100644 index 6bc9dadc..00000000 --- a/clang-tools-manager/tests/common.rs +++ /dev/null @@ -1,89 +0,0 @@ -use std::{ - env, - io::{Write, stdout}, -}; - -use colored::{Colorize, control::set_override}; -use log::{Level, LevelFilter, Log, Metadata, Record}; - -struct SimpleLogger; - -impl SimpleLogger { - fn level_color(level: &Level) -> String { - let name = format!("{:>5}", level.as_str().to_uppercase()); - match level { - Level::Error => name.red().bold().to_string(), - Level::Warn => name.yellow().bold().to_string(), - Level::Info => name.green().bold().to_string(), - Level::Debug => name.blue().bold().to_string(), - Level::Trace => name.magenta().bold().to_string(), - } - } -} - -impl Log for SimpleLogger { - fn enabled(&self, metadata: &Metadata) -> bool { - metadata.level() <= log::max_level() - } - - fn log(&self, record: &Record) { - let mut stdout = stdout().lock(); - if record.target() == "CI_LOG_GROUPING" { - // this log is meant to manipulate a CI workflow's log grouping - writeln!(stdout, "{}", record.args()).expect("Failed to write log command to stdout"); - stdout - .flush() - .expect("Failed to flush log command in stdout"); - } else if self.enabled(record.metadata()) { - let module = record.module_path(); - if module.is_none_or(|v| { - v.starts_with("clang_tools_manager") || v.starts_with("clang_tools") - }) { - writeln!( - stdout, - "[{}]: {}", - Self::level_color(&record.level()), - record.args() - ) - .expect("Failed to write log message to stdout"); - } else if let Some(module) = module { - writeln!( - stdout, - "[{}]{{{}:{}}}: {}", - Self::level_color(&record.level()), - module, - record.line().unwrap_or_default(), - record.args() - ) - .expect("Failed to write detailed log message to stdout"); - } - stdout - .flush() - .expect("Failed to flush log message in stdout"); - } - } - - fn flush(&self) {} -} - -/// A function to initialize the private `LOGGER`. -/// -/// The logging level defaults to [`LevelFilter::Info`]. -/// This logs a debug message about [`SetLoggerError`](struct@log::SetLoggerError) -/// if the `LOGGER` is already initialized. -pub fn initialize_logger() { - let logger: SimpleLogger = SimpleLogger; - if env::var("CPP_LINTER_COLOR") - .as_deref() - .is_ok_and(|v| matches!(v, "on" | "1" | "true")) - { - set_override(true); - } - if let Err(e) = - log::set_boxed_logger(Box::new(logger)).map(|()| log::set_max_level(LevelFilter::Info)) - { - // logger singleton already instantiated. - // we'll just use whatever the current config is. - log::debug!("{e:?}"); - } -} diff --git a/clang-tools-manager/tests/pypi.rs b/clang-tools-manager/tests/pypi.rs index 7c9b7a1c..49e43c9e 100644 --- a/clang-tools-manager/tests/pypi.rs +++ b/clang-tools-manager/tests/pypi.rs @@ -1,14 +1,18 @@ use std::{env, process::Command}; -use semver::VersionReq; - +#[cfg(feature = "bin")] +use clang_tools_manager::logger::try_init_logger; use clang_tools_manager::{ClangTool, PyPiDownloader}; + +use semver::VersionReq; use tempfile::TempDir; -mod common; async fn setup() { - common::initialize_logger(); - log::set_max_level(log::LevelFilter::Debug); + #[cfg(feature = "bin")] + { + try_init_logger(); + log::set_max_level(log::LevelFilter::Debug); + } let tmp_cache_dir = TempDir::new().unwrap(); // Override cache directory to ensure test isolation and avoid interference with other tests' caches diff --git a/clang-tools-manager/tests/static_dist.rs b/clang-tools-manager/tests/static_dist.rs index 85d9b579..1459f45a 100644 --- a/clang-tools-manager/tests/static_dist.rs +++ b/clang-tools-manager/tests/static_dist.rs @@ -1,13 +1,18 @@ use std::env; +#[cfg(feature = "bin")] +use clang_tools_manager::logger::try_init_logger; use clang_tools_manager::{ClangTool, StaticDistDownloader}; + use semver::VersionReq; use tempfile::TempDir; -mod common; async fn setup(ver_spec: &str) { - common::initialize_logger(); - log::set_max_level(log::LevelFilter::Debug); + #[cfg(feature = "bin")] + { + try_init_logger(); + log::set_max_level(log::LevelFilter::Debug); + } let tmp_cache_dir = TempDir::new().unwrap(); // Override cache directory to ensure test isolation and avoid interference with other tests' caches diff --git a/cpp-linter/Cargo.toml b/cpp-linter/Cargo.toml index dc6d7b6d..4984c681 100644 --- a/cpp-linter/Cargo.toml +++ b/cpp-linter/Cargo.toml @@ -17,9 +17,8 @@ license.workspace = true anyhow = { workspace = true, optional = true } async-trait = "0.1.89" chrono = "0.4.44" -clang-tools-manager = { path = "../clang-tools-manager", version = "0.2.1" } +clang-tools-manager = { path = "../clang-tools-manager", features = ["bin"], version = "0.2.1" } clap = { workspace = true, optional = true } -colored = { workspace = true, optional = true } fast-glob = "1.0.1" futures = "0.3.32" gix-imara-diff = { version = "0.2.2", features = ["unified_diff"] } @@ -45,7 +44,7 @@ tempfile = { workspace = true } reqwest = { workspace = true, features = ["default-tls"] } [features] -bin = ["reqwest/default-tls", "dep:clap", "dep:colored", "dep:anyhow"] +bin = ["reqwest/default-tls", "dep:clap", "dep:anyhow"] [lib] bench = false diff --git a/cpp-linter/src/clang_tools/mod.rs b/cpp-linter/src/clang_tools/mod.rs index a6afc8f6..93527b13 100644 --- a/cpp-linter/src/clang_tools/mod.rs +++ b/cpp-linter/src/clang_tools/mod.rs @@ -364,11 +364,10 @@ mod tests { use std::{env, fs, path::Path}; + use clang_tools_manager::logger::try_init_logger; use git_bot_feedback::ReviewComment; use super::*; - #[cfg(feature = "bin")] - use crate::logger::try_init; async fn test_db_parse>(path: P) -> Result { let clang_params = ClangParams { @@ -382,8 +381,7 @@ mod tests { env::remove_var("GITHUB_ACTIONS"); } let rest_client = RestClient::new().unwrap(); - #[cfg(feature = "bin")] - try_init(); + try_init_logger(); capture_clang_tools_output(&[], &version, clang_params, &rest_client, false).await } @@ -421,11 +419,8 @@ mod tests { }; let total_review_comments = 2; let summary_only = false; - #[cfg(feature = "bin")] - { - crate::logger::try_init(); - log::set_max_level(log::LevelFilter::Info); - } + try_init_logger(); + log::set_max_level(log::LevelFilter::Info); let review_summary = ReviewComments::default().summarize( &clang_versions, &comments, diff --git a/cpp-linter/src/cli/mod.rs b/cpp-linter/src/cli/mod.rs index c605ef78..335a59b9 100644 --- a/cpp-linter/src/cli/mod.rs +++ b/cpp-linter/src/cli/mod.rs @@ -6,7 +6,7 @@ use std::str::FromStr; // non-std crates #[cfg(feature = "bin")] -use clang_tools_manager::RequestedVersion; +use clang_tools_manager::{RequestedVersion, logger::CLI_HELP_STYLE}; #[cfg(feature = "bin")] use clap::{ ArgAction, Args, Parser, Subcommand, ValueEnum, @@ -38,7 +38,7 @@ impl Verbosity { /// A structure to contain parsed CLI options. #[cfg(feature = "bin")] #[derive(Debug, Clone, Parser)] -#[command(author, about)] +#[command(author, about, color = clap::ColorChoice::Auto, styles(CLI_HELP_STYLE))] pub struct Cli { /// The CLI's general options, such as `--version` and `--verbosity`. #[command(flatten)] diff --git a/cpp-linter/src/lib.rs b/cpp-linter/src/lib.rs index 9241a984..ca48fb8c 100644 --- a/cpp-linter/src/lib.rs +++ b/cpp-linter/src/lib.rs @@ -20,7 +20,6 @@ pub mod cli; pub mod common_fs; pub mod error; mod git; -pub mod logger; pub mod rest_client; pub mod run; diff --git a/cpp-linter/src/rest_client.rs b/cpp-linter/src/rest_client.rs index 4c2d4df6..9ff82477 100644 --- a/cpp-linter/src/rest_client.rs +++ b/cpp-linter/src/rest_client.rs @@ -493,8 +493,8 @@ mod test { }, cli::FeedbackInput, common_fs::FileObj, - logger, }; + use clang_tools_manager::logger::try_init_logger; // ************************* tests for step-summary and output variables @@ -507,7 +507,8 @@ mod test { env::set_var("GITHUB_SHA", "deadbeef123"); } let mut rest_api_client = RestClient::new().unwrap(); - logger::try_init(); + #[cfg(feature = "bin")] + try_init_logger(); if env::var("ACTIONS_STEP_DEBUG").is_ok_and(|var| var == "true") { // assert!(rest_api_client.debug_enabled); log::set_max_level(log::LevelFilter::Debug); diff --git a/cpp-linter/src/run.rs b/cpp-linter/src/run.rs index 8b612db9..1f104249 100644 --- a/cpp-linter/src/run.rs +++ b/cpp-linter/src/run.rs @@ -12,7 +12,7 @@ use std::{ // non-std crates use anyhow::{Context, Result, anyhow}; -use clang_tools_manager::RequestedVersion; +use clang_tools_manager::{RequestedVersion, logger::try_init_logger}; use clap::Parser; use log::{LevelFilter, set_max_level}; @@ -21,7 +21,6 @@ use crate::{ clang_tools::capture_clang_tools_output, cli::{ClangParams, Cli, CliCommand, FeedbackInput, LinesChangedOnly}, common_fs::FileObj, - logger, rest_client::RestClient, }; use git_bot_feedback::FileFilter; @@ -57,7 +56,7 @@ pub async fn run_main(args: Vec) -> Result<()> { return Ok(()); } - logger::try_init(); + try_init_logger(); let mut rest_api_client = RestClient::new()?; set_max_level( diff --git a/cpp-linter/tests/paginated_changed_files.rs b/cpp-linter/tests/paginated_changed_files.rs index dd2b3757..72e28e56 100644 --- a/cpp-linter/tests/paginated_changed_files.rs +++ b/cpp-linter/tests/paginated_changed_files.rs @@ -6,7 +6,8 @@ use git_bot_feedback::{FileFilter, LinesChangedOnly}; use mockito::Matcher; use tempfile::{NamedTempFile, TempDir}; -use cpp_linter::{logger, rest_client::RestClient}; +use clang_tools_manager::logger::try_init_logger; +use cpp_linter::rest_client::RestClient; use std::{env, io::Write, path::Path}; #[derive(PartialEq, Default)] @@ -83,7 +84,7 @@ async fn get_paginated_changes(lib_root: &Path, tmp_dir: &TempDir, test_params: unsafe { env::set_var("GITHUB_API_URL", server.url()); } - logger::try_init(); + try_init_logger(); log::set_max_level(log::LevelFilter::Debug); let gh_client = RestClient::new(); if test_params.fail_serde_event_payload || test_params.no_event_payload {