Skip to content

Commit

Permalink
Better logging for lintcheck
Browse files Browse the repository at this point in the history
Use `simplelog` to handle logs, as `env_logger` does not handle writing to file for the moment, see rust-cli/env_logger#208

Do not push most verbose logs on stdout, only push them in the log file
  • Loading branch information
kraktus committed Sep 10, 2022
1 parent f5d225d commit 5461e99
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 18 deletions.
2 changes: 2 additions & 0 deletions lintcheck/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ publish = false
cargo_metadata = "0.14"
clap = "3.2"
crossbeam-channel = "0.5.6"
simplelog = "0.12.0"
flate2 = "1.0"
log = "0.4"
rayon = "1.5.1"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0.85"
Expand Down
31 changes: 30 additions & 1 deletion lintcheck/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use clap::{Arg, ArgAction, ArgMatches, Command};
use log::LevelFilter;
use simplelog::{ColorChoice, CombinedLogger, Config, TermLogger, TerminalMode, WriteLogger};
use std::env;
use std::fs::{self, File};
use std::path::PathBuf;

fn get_clap_config() -> ArgMatches {
Expand Down Expand Up @@ -39,6 +42,11 @@ fn get_clap_config() -> ArgMatches {
.help("Run clippy on the dependencies of crates specified in crates-toml")
.conflicts_with("threads")
.conflicts_with("fix"),
Arg::new("verbose")
.short('v')
.long("--verbose")
.action(ArgAction::Count)
.help("Verbosity to use, default to WARN"),
])
.get_matches()
}
Expand Down Expand Up @@ -66,6 +74,27 @@ pub(crate) struct LintcheckConfig {
impl LintcheckConfig {
pub fn new() -> Self {
let clap_config = get_clap_config();
let level_filter = match clap_config.get_count("verbose") {
0 => LevelFilter::Warn,
1 => LevelFilter::Info,
2 => LevelFilter::Debug,
_ => LevelFilter::Trace,
};
// using `create_dir_all` as it does not error when the dir already exists
fs::create_dir_all("lintcheck-logs").expect("Creating the log dir failed");
let _ = CombinedLogger::init(vec![
TermLogger::new(
std::cmp::min(level_filter, LevelFilter::Info), // do not print more verbose log than `INFO` to stdout,
Config::default(),
TerminalMode::Mixed,
ColorChoice::Auto,
),
WriteLogger::new(
level_filter,
Config::default(),
File::create("lintcheck-logs/lintcheck.log").unwrap(),
),
]);

// first, check if we got anything passed via the LINTCHECK_TOML env var,
// if not, ask clap if we got any value for --crates-toml <foo>
Expand All @@ -84,7 +113,7 @@ impl LintcheckConfig {
// wasd.toml, use "wasd"...)
let filename: PathBuf = sources_toml_path.file_stem().unwrap().into();
let lintcheck_results_path = PathBuf::from(format!(
"lintcheck-logs/{}_logs.{}",
"lintcheck-logs/{}_results.{}",
filename.display(),
if markdown { "md" } else { "txt" }
));
Expand Down
34 changes: 17 additions & 17 deletions lintcheck/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use std::time::Duration;

use cargo_metadata::diagnostic::{Diagnostic, DiagnosticLevel};
use cargo_metadata::Message;
use log::{debug, error, trace, warn};
use rayon::prelude::*;
use serde::{Deserialize, Serialize};
use walkdir::{DirEntry, WalkDir};
Expand Down Expand Up @@ -163,10 +164,10 @@ fn get(path: &str) -> Result<ureq::Response, ureq::Error> {
match ureq::get(path).call() {
Ok(res) => return Ok(res),
Err(e) if retries >= MAX_RETRIES => return Err(e),
Err(ureq::Error::Transport(e)) => eprintln!("Error: {e}"),
Err(ureq::Error::Transport(e)) => error!("{}", e),
Err(e) => return Err(e),
}
eprintln!("retrying in {retries} seconds...");
warn!("retrying in {retries} seconds...");
thread::sleep(Duration::from_secs(u64::from(retries)));
retries += 1;
}
Expand Down Expand Up @@ -234,7 +235,7 @@ impl CrateSource {
.expect("Failed to clone git repo!")
.success()
{
eprintln!("Failed to clone {url} into {}", repo_path.display());
warn!("Failed to clone {url} into {}", repo_path.display());
}
}
// check out the commit/branch/whatever
Expand All @@ -247,7 +248,7 @@ impl CrateSource {
.expect("Failed to check out commit")
.success()
{
eprintln!("Failed to checkout {commit} of repo at {}", repo_path.display());
warn!("Failed to checkout {commit} of repo at {}", repo_path.display());
}

Crate {
Expand Down Expand Up @@ -390,6 +391,8 @@ impl Crate {

cargo_clippy_args.extend(clippy_args);

debug!("Arguments passed to cargo clippy driver: {:?}", cargo_clippy_args);

let all_output = Command::new(&cargo_clippy_path)
// use the looping index to create individual target dirs
.env("CARGO_TARGET_DIR", shared_target_dir.join(format!("_{thread_index:?}")))
Expand All @@ -409,21 +412,19 @@ impl Crate {
let status = &all_output.status;

if !status.success() {
eprintln!(
"\nWARNING: bad exit status after checking {} {} \n",
self.name, self.version
);
warn!("bad exit status after checking {} {} \n", self.name, self.version);
}

if config.fix {
trace!("{}", stderr);
if let Some(stderr) = stderr
.lines()
.find(|line| line.contains("failed to automatically apply fixes suggested by rustc to crate"))
{
let subcrate = &stderr[63..];
println!(
"ERROR: failed to apply some suggetion to {} / to (sub)crate {subcrate}",
self.name
error!(
"failed to apply some suggetion to {} / to (sub)crate {}",
self.name, subcrate
);
}
// fast path, we don't need the warnings anyway
Expand All @@ -449,7 +450,7 @@ fn build_clippy() {
.status()
.expect("Failed to build clippy!");
if !status.success() {
eprintln!("Error: Failed to compile Clippy!");
error!("Failed to compile Clippy!");
std::process::exit(1);
}
}
Expand Down Expand Up @@ -553,7 +554,7 @@ fn main() {

// assert that we launch lintcheck from the repo root (via cargo lintcheck)
if std::fs::metadata("lintcheck/Cargo.toml").is_err() {
eprintln!("lintcheck needs to be run from clippy's repo root!\nUse `cargo lintcheck` alternatively.");
error!("lintcheck needs to be run from clippy's repo root!\nUse `cargo lintcheck` alternatively.");
std::process::exit(3);
}

Expand Down Expand Up @@ -615,8 +616,8 @@ fn main() {
.collect();

if crates.is_empty() {
eprintln!(
"ERROR: could not find crate '{}' in lintcheck/lintcheck_crates.toml",
error!(
"could not find crate '{}' in lintcheck/lintcheck_crates.toml",
config.only.unwrap(),
);
std::process::exit(1);
Expand Down Expand Up @@ -693,8 +694,7 @@ fn main() {
let _ = write!(text, "{cratename}: '{msg}'");
}

println!("Writing logs to {}", config.lintcheck_results_path.display());
fs::create_dir_all(config.lintcheck_results_path.parent().unwrap()).unwrap();
println!("Writing results to {}", config.lintcheck_results_path.display());
fs::write(&config.lintcheck_results_path, text).unwrap();

print_stats(old_stats, new_stats, &config.lint_filter);
Expand Down

0 comments on commit 5461e99

Please sign in to comment.