From a8908abe00c055ecfe5b5c1357796c16627acfae Mon Sep 17 00:00:00 2001 From: wzslr321 Date: Mon, 7 Oct 2024 23:04:45 +0200 Subject: [PATCH 1/9] feat: add basic `thiserror` support --- Cargo.lock | 16 +++-- Cargo.toml | 2 + src/cli.rs | 171 ++++++++++++++++++++++++++++---------------------- src/config.rs | 2 +- src/git.rs | 59 +++++++++-------- src/main.rs | 6 +- 6 files changed, 148 insertions(+), 108 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0f157c5..5855337 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -65,6 +65,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "anyhow" +version = "1.0.89" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "86fdf8605db99b54d3cd748a44c6d04df638eb5dafb219b135d0149bd0db01f6" + [[package]] name = "async-trait" version = "0.1.81" @@ -351,6 +357,7 @@ dependencies = [ name = "impactifier" version = "0.1.0" dependencies = [ + "anyhow", "clap", "config", "git2", @@ -359,6 +366,7 @@ dependencies = [ "serde", "serde_derive", "serde_yaml", + "thiserror", "toml", "tracing", "tracing-subscriber", @@ -872,18 +880,18 @@ checksum = "a38c90d48152c236a3ab59271da4f4ae63d678c5d7ad6b7714d7cb9760be5e4b" [[package]] name = "thiserror" -version = "1.0.63" +version = "1.0.64" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c0342370b38b6a11b6cc11d6a805569958d54cfa061a29969c3b5ce2ea405724" +checksum = "d50af8abc119fb8bb6dbabcfa89656f46f84aa0ac7688088608076ad2b459a84" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.63" +version = "1.0.64" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4558b58466b9ad7ca0f102865eccc95938dca1a74a856f2b57b6629050da261" +checksum = "08904e7672f5eb876eaaf87e0ce17857500934f4981c4a0ab2b4aa98baac7fc3" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 91725db..c1f2f85 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,6 +6,7 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +anyhow = "1.0.89" clap = { version = "4.5.16", features = ["derive"] } config = "0.14.0" git2 = "0.19.0" @@ -14,6 +15,7 @@ rhai = "1.19.0" serde = "1.0.208" serde_derive = "1.0.208" serde_yaml = "0.9.34" +thiserror = "1.0.64" toml = "0.8.19" tracing = "0.1.40" tracing-subscriber = "0.3.18" diff --git a/src/cli.rs b/src/cli.rs index 1ec7843..4f817a7 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,16 +1,18 @@ +use core::fmt; use std::path::Path; use clap::Parser; use git2::Repository; +use thiserror::Error; use tracing::{error, info, trace, Level}; +use url::Url; use uuid::Uuid; -use crate::config::{Config, RepositoryConfig}; -use crate::git::clone_repo; use crate::transform::init_registry; use crate::utils; +use crate::{config::Config, git::clone_repo}; -#[derive(Parser, Debug)] +#[derive(Parser, Debug, Clone)] #[command( version, about = "Impactifier is a tool for analyzing code changes and assessing their impact.", @@ -26,7 +28,7 @@ use crate::utils; creates repository struct from local path from_branch fallbacks to default after opening if branches specified & local changes detected, optionally includes those - if no branch & commit specified, tries to analyze local changes + if no branch & commit specified, tries (not yet) to analyze local changes if no local changes fails as there is nothing to compare "# )] @@ -38,16 +40,23 @@ struct Args { #[arg(short, long, default_value_t = String::from("impactifier-config.yaml"))] config: String, + /// From what branch changes should be compared. + /// + /// Defaults to the current branch. #[arg(short, long)] from_branch: Option, + /// To what branch changes should be compared #[arg(long)] to_branch: Option, + /// Commit of which changes should be analyzed. Takes precedence over + /// branch changes, if `from_branch` or `to_branch` is specified. #[arg(long)] of_commit: Option, - #[arg(long, help = "Fetch latest changes before comparison")] + /// Fetch last changes before impact analysis + #[arg(long)] fetch: bool, /// Sets max tracing level. Available options: @@ -61,10 +70,20 @@ struct Args { tracing_level: u8, } -pub fn run() -> Result<(), Box> { +pub fn run() -> Result<(), CliError> { let args = Args::parse(); setup_logging(args.tracing_level); + match check_args_validity(args.clone()) { + Ok(_) => { + trace!("args validation completed successfully. Continuing execution."); + } + Err(err) => { + error!("args are invalid. Exiting..."); + return Err(err) + } + } + let cfg = match load_config(Path::new(&args.config)) { Ok(config) => config, Err(e) => return Err(e), @@ -73,21 +92,16 @@ pub fn run() -> Result<(), Box> { init_registry(cfg.custom_transform_scripts()); let _repository = match &cfg.repository.url { - Some(url) => { - match try_retrieve_repo_from_url( - url.as_str(), - args.from_branch, - args.to_branch, - args.of_commit, - cfg.options.clone_into, - &cfg.repository, - ) { - Ok(repo) => repo, - Err(_) => { - return Err(Box::from("Either repository url or path must be specified")); - } + Some(url) => match try_retrieve_repo_from_url( + cfg.repository.access_token, + url, + cfg.options.clone_into, + ) { + Ok(repo) => repo, + Err(_) => { + return Err(CliError::Unknown); } - } + }, None => match &cfg.repository.path { Some(path) => { match try_retrieve_repo_from_path( @@ -97,13 +111,15 @@ pub fn run() -> Result<(), Box> { ) { Ok(repo) => repo, Err(_) => { - return Err(Box::from("Either repository url or path must be specified")); + return Err(CliError::IncorrectArgs { + msg: "Either repository url or path must be specified".to_string(), + }); } } } None => { error!("Repository url and path are unspecified"); - return Err(Box::from("Either repository url or path must be specified")); + return Err(CliError::InvalidConfigPath); } }, }; @@ -111,6 +127,29 @@ pub fn run() -> Result<(), Box> { Ok(()) } +fn check_args_validity(args: Args) -> Result<(), CliError> { + match (&args.from_branch, &args.to_branch) { + (None, None) => { + info!("No branches specified"); + match &args.of_commit { + Some(_) => Ok(()), + None => { + error!("No commit specified. Nothing to analyze"); + Err(CliError::InsufficientArgs) + } + } + } + (None, Some(_)) => Ok(()), + (Some(_), None) => { + error!("from_branch specified, but to_branch is missing"); + Err(CliError::IncorrectArgs { + msg: "Specifying `from_branch` requires `to_branch` to be specified".to_string(), + }) + } + (Some(_), Some(_)) => Ok(()), + } +} + fn try_retrieve_repo_from_path(path: &str) -> Result> { trace!( "attempt to retireve repo path specified repository.\nPath:{}", @@ -120,62 +159,32 @@ fn try_retrieve_repo_from_path(path: &str) -> Result, - to_branch: Option, - commit_id: Option, + access_token: Option, + url: &Url, clone_into: Option>, - config: &RepositoryConfig, ) -> Result> { trace!("attempt to start from url-specified repository"); - match (from_branch, to_branch) { - (None, None) => { - info!("No branches specified"); - match commit_id { - Some(_commit_id) => todo!(), //try_analyze_commit(&commit_id), - None => { - error!("No commit specified. Nothing to analyze"); - Err(Box::from( - "No branches and no commit specified. Nothing to analyze.", - )) - } - } - } - (None, Some(_)) => todo!(), - (Some(_), None) => { - error!("Incorrect CLI arguments. Specifying `from_branch` requires `to_branch` to be specified"); - Err(Box::from("Incorrect arguments")) - } - (Some(from_branch), Some(to_branch)) => { - info!( - "Attempting to compare branch {} with branch {}", - from_branch, to_branch - ); - info!("Attempting to clone repository from url: {}", url); - let clone_into_path = &clone_into.unwrap_or_else(|| { - let path = Path::new(&format!("repository{}", Uuid::new_v4())).into(); - trace!("set fallback clone_into path to {:?}", path); - path - }); - match utils::prepare_directory(&clone_into_path) { - Ok(_) => { - trace!("Starting to clone repository"); - let cloned_repo = - match clone_repo(&config, &clone_into_path, Some(&from_branch)) { - Ok(repo) => repo, - Err(e) => { - error!("Failed to clone repository. error: {}", e); - return Err(e.into()); - } - }; - info!("Repository cloned successfuly"); - Ok(cloned_repo) - } + let clone_into_path = &clone_into.unwrap_or_else(|| { + let path = Path::new(&format!("repository{}", Uuid::new_v4())).into(); + trace!("set fallback clone_into path to {:?}", path); + path + }); + match utils::prepare_directory(&clone_into_path) { + Ok(_) => { + trace!("Starting to clone repository"); + let cloned_repo = match clone_repo(access_token, url, &clone_into_path) { + Ok(repo) => repo, Err(e) => { - error!("Failed to prepare directory for cloning"); + error!("Failed to clone repository. error: {}", e); return Err(e.into()); } - } + }; + info!("Repository cloned successfuly"); + Ok(cloned_repo) + } + Err(e) => { + error!("Failed to prepare directory for cloning"); + return Err(e.into()); } } } @@ -203,16 +212,28 @@ fn setup_logging(tracing_level: u8) { .init(); } -fn load_config(path: &Path) -> Result> { +fn load_config(path: &Path) -> Result { trace!("Starting loading config from {:?}", path); match Config::load_from_file(path) { Ok(config) => { info!("Config loaded successfully"); Ok(config) } - Err(e) => { + Err(_) => { error!("Failed to read configuration from {:?}", path); - return Err(e.into()); + return Err(CliError::InvalidConfigPath); } } } + +#[derive(Error, Debug)] +pub enum CliError { + #[error("No branches and no commit specified. No local changes detected. Nothing to analyze.")] + InsufficientArgs, + #[error("Incorrect CLI arguments.{}", msg)] + IncorrectArgs { msg: String }, + #[error("Config can not be retrieved")] + InvalidConfigPath, + #[error("Unknown error")] + Unknown, +} diff --git a/src/config.rs b/src/config.rs index 75c9091..39079f7 100644 --- a/src/config.rs +++ b/src/config.rs @@ -20,7 +20,6 @@ pub struct RepositoryConfig { pub url: Option, pub path: Option>, pub access_token: Option, - pub branch: String, } #[derive(Debug, Deserialize)] @@ -88,6 +87,7 @@ pub struct CustomStep { } impl Config { + // TODO(wiktor.zajac) improve error handling pub fn load_from_file(file_path: &Path) -> Result> { let yaml_content = match std::fs::read_to_string(file_path) { Ok(content) => { diff --git a/src/git.rs b/src/git.rs index 0e4e1fc..74205ef 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1,35 +1,45 @@ use std::{env, path::Path}; +use thiserror::Error; use git2::{Cred, RemoteCallbacks, Repository}; -use tracing::error; - -use crate::config::RepositoryConfig; +use url::Url; + +#[derive(Error, Debug)] +pub enum GitError { + #[error("Failed to authorize git request, due to authentication failure")] + NoAccess, + #[error( + "Failed to clone repository from url {} to given path: {}.\nError: {}", + url, + path, + err, + )] + CloneFailure { + url: String, + path: String, + err: git2::Error, + }, +} pub fn clone_repo( - options: &RepositoryConfig, - clone_into: &Path, - branch: Option<&str>, -) -> Result> { - let token = match options.access_token.to_owned() { + access_token: Option, + url: &Url, + clone_into: &Box, +) -> Result { + let token = match access_token.to_owned() { Some(token) => token, None => match env::var("GITHUB_ACCESS_TOKEN_2") { Ok(token) => token, - Err(_) => { - return Err(Box::from("No access token provided")); - } + Err(_) => return Err(GitError::NoAccess), }, }; let mut callbacks = RemoteCallbacks::new(); callbacks.credentials(|_url, _username_from_url, _allowed_types| { - Cred::userpass_plaintext("wzslr321", &token) + Cred::userpass_plaintext("wzslr321", &token) }); let mut builder = git2::build::RepoBuilder::new(); - builder.bare(true); - if let Some(branch) = branch { - builder.branch(&branch); - } let mut fetch_options = git2::FetchOptions::new(); fetch_options.remote_callbacks(callbacks); @@ -37,14 +47,13 @@ pub fn clone_repo( builder.fetch_options(fetch_options); - match &options.url { - Some(url) => match builder.clone(url.as_str(), &clone_into) { - Ok(repository) => Ok(repository), - Err(e) => Err(e.into()), - }, - None => { - error!("Failed to clone the repository. Url not specified."); - Err(Box::from("Repository url not specified")) - } + + match builder.clone(url.as_str(), &clone_into) { + Ok(repository) => Ok(repository), + Err(e) => Err(GitError::CloneFailure{ + url: url.to_string(), + path: String::from(clone_into.to_string_lossy()), + err: e, + }), } } diff --git a/src/main.rs b/src/main.rs index 6e7638f..d8d2e36 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,8 +4,8 @@ mod git; mod utils; mod transform; -use std::error::Error; +use cli::CliError; -fn main() -> Result<(), Box> { - cli::run() +fn main() -> Result<(), CliError> { + cli::run() } From 1249dbc7d31fd25a72ac68b7291c281f75132574 Mon Sep 17 00:00:00 2001 From: wzslr321 Date: Tue, 8 Oct 2024 20:12:18 +0200 Subject: [PATCH 2/9] feat: partial error handling improvement with thiserror and anyhow --- src/cli.rs | 17 +++++++++-------- src/config.rs | 35 +++++++++++++++++------------------ src/git.rs | 50 +++++++++++++++++++++++++++++++++++++------------- 3 files changed, 63 insertions(+), 39 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 4f817a7..d92f167 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,4 +1,3 @@ -use core::fmt; use std::path::Path; use clap::Parser; @@ -80,7 +79,7 @@ pub fn run() -> Result<(), CliError> { } Err(err) => { error!("args are invalid. Exiting..."); - return Err(err) + return Err(err); } } @@ -94,6 +93,7 @@ pub fn run() -> Result<(), CliError> { let _repository = match &cfg.repository.url { Some(url) => match try_retrieve_repo_from_url( cfg.repository.access_token, + "wzslr321", url, cfg.options.clone_into, ) { @@ -119,7 +119,7 @@ pub fn run() -> Result<(), CliError> { } None => { error!("Repository url and path are unspecified"); - return Err(CliError::InvalidConfigPath); + return Err(CliError::InvalidConfigPath { err: None }); } }, }; @@ -160,6 +160,7 @@ fn try_retrieve_repo_from_path(path: &str) -> Result, + username: &str, url: &Url, clone_into: Option>, ) -> Result> { @@ -172,7 +173,7 @@ fn try_retrieve_repo_from_url( match utils::prepare_directory(&clone_into_path) { Ok(_) => { trace!("Starting to clone repository"); - let cloned_repo = match clone_repo(access_token, url, &clone_into_path) { + let cloned_repo = match clone_repo(access_token, username, url, &clone_into_path) { Ok(repo) => repo, Err(e) => { error!("Failed to clone repository. error: {}", e); @@ -212,16 +213,16 @@ fn setup_logging(tracing_level: u8) { .init(); } -fn load_config(path: &Path) -> Result { +fn load_config(path: &Path) -> anyhow::Result { trace!("Starting loading config from {:?}", path); match Config::load_from_file(path) { Ok(config) => { info!("Config loaded successfully"); Ok(config) } - Err(_) => { + Err(err) => { error!("Failed to read configuration from {:?}", path); - return Err(CliError::InvalidConfigPath); + return Err(CliError::InvalidConfigPath { err: Some(err) }); } } } @@ -233,7 +234,7 @@ pub enum CliError { #[error("Incorrect CLI arguments.{}", msg)] IncorrectArgs { msg: String }, #[error("Config can not be retrieved")] - InvalidConfigPath, + InvalidConfigPath { err: Option }, #[error("Unknown error")] Unknown, } diff --git a/src/config.rs b/src/config.rs index 39079f7..c94c62f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,12 +1,20 @@ +use anyhow::{Context, Result}; use serde::{Deserialize, Deserializer}; +use tracing::error; use std::cmp; -use std::env; use std::fmt; use std::path::Path; use std::path::PathBuf; +use thiserror::Error; use tracing::debug; use url::Url; +#[derive(Error, Debug)] +pub enum ConfigError { + #[error("Failed to read config from path: {}. Error:{}", path, msg)] + ReadFailure { path: String, msg: String }, +} + #[derive(Debug, Deserialize)] pub struct Config { pub repository: RepositoryConfig, @@ -88,18 +96,21 @@ pub struct CustomStep { impl Config { // TODO(wiktor.zajac) improve error handling - pub fn load_from_file(file_path: &Path) -> Result> { + pub fn load_from_file(file_path: &Path) -> Result { let yaml_content = match std::fs::read_to_string(file_path) { Ok(content) => { debug!("Succesfully read yaml config file:\n{}", content); content } Err(e) => { - return Err(e.into()); + error!("Failed to read yaml config"); + return Err(ConfigError::ReadFailure { + path: String::from(file_path.to_string_lossy()), + msg: e.to_string(), + } + .into()) } }; - let yaml_content = replace_env_vars(&yaml_content); - debug!("replaced env variables in yaml config"); let cfg = serde_yaml::from_str(&yaml_content)?; debug!("Deserialized config:\n{}", cfg); @@ -124,7 +135,7 @@ impl Config { .and_then(|args| args.get("script")) .and_then(|script_value| script_value.as_str()) .map(|s| s.to_string()) - .unwrap() + .unwrap(), }) }) .collect(); @@ -172,18 +183,6 @@ impl fmt::Display for Config { } } -// TODO: Probably could be globally handled with regex, -// but it first needs to be ensured that the performance -// will not be affected. If somehow the regex will affect -// performance strong enough, replacable variables should -// be predefined instead of being hardcoded here. -fn replace_env_vars(yaml_content: &str) -> String { - yaml_content.replace( - "${GITHUB_ACCESS_TOKEN}", - &env::var("GITHUB_ACCESS_TOKEN").unwrap_or_default(), - ) -} - fn deserialize_url<'a, D>(deserializer: D) -> Result, D::Error> where D: Deserializer<'a>, diff --git a/src/git.rs b/src/git.rs index 74205ef..d85c3a3 100644 --- a/src/git.rs +++ b/src/git.rs @@ -2,17 +2,18 @@ use std::{env, path::Path}; use thiserror::Error; use git2::{Cred, RemoteCallbacks, Repository}; +use tracing::{error, info, trace}; use url::Url; #[derive(Error, Debug)] pub enum GitError { - #[error("Failed to authorize git request, due to authentication failure")] - NoAccess, + #[error("Failed to authorize git request, due to authentication failure. Error:{msg}")] + NoAccess { msg: String }, #[error( "Failed to clone repository from url {} to given path: {}.\nError: {}", url, path, - err, + err )] CloneFailure { url: String, @@ -23,21 +24,36 @@ pub enum GitError { pub fn clone_repo( access_token: Option, + username: &str, url: &Url, clone_into: &Box, ) -> Result { + info!("start cloning repository"); + + trace!("try retrieve access token"); let token = match access_token.to_owned() { Some(token) => token, - None => match env::var("GITHUB_ACCESS_TOKEN_2") { + None => match env::var("GITHUB_ACCESS_TOKEN") { Ok(token) => token, - Err(_) => return Err(GitError::NoAccess), + Err(_) => { + error!("failed to retrieve access token"); + return Err(GitError::NoAccess { + msg: "Access token unspecified".to_string(), + }); + } }, }; + trace!("Retrieved token successfully"); let mut callbacks = RemoteCallbacks::new(); + // TODO(wiktor.zajac) [https://github.com/wzslr321/impactifier/issues/9] + // Support different credentials for git access + // + // Additionally, it can probably be extracted to a separate util callbacks.credentials(|_url, _username_from_url, _allowed_types| { - Cred::userpass_plaintext("wzslr321", &token) + Cred::userpass_plaintext(username, &token) }); + trace!("Callback credentials set to userpass_plaintext"); let mut builder = git2::build::RepoBuilder::new(); @@ -46,14 +62,22 @@ pub fn clone_repo( fetch_options.depth(1); builder.fetch_options(fetch_options); - + // TODO(wiktor.zajac) try to guard agains future changes, to update this trace automatically + // by using some implemented trait with display + trace!("FetchOptions set to depth=0"); match builder.clone(url.as_str(), &clone_into) { - Ok(repository) => Ok(repository), - Err(e) => Err(GitError::CloneFailure{ - url: url.to_string(), - path: String::from(clone_into.to_string_lossy()), - err: e, - }), + Ok(repository) => { + info!("repository cloned successfully"); + Ok(repository) + } + Err(e) => { + error!("failed to clone repository"); + Err(GitError::CloneFailure { + url: url.to_string(), + path: String::from(clone_into.to_string_lossy()), + err: e, + }) + } } } From 03b7090417e590f04850be63cd9a9bd742525da6 Mon Sep 17 00:00:00 2001 From: wzslr321 Date: Tue, 8 Oct 2024 21:52:12 +0200 Subject: [PATCH 3/9] feat: add open_repo functionality to handle repository retrieval from path --- src/cli.rs | 66 +++++++++++++++++++++++++----------------------------- src/git.rs | 20 +++++++++++++++++ 2 files changed, 51 insertions(+), 35 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index d92f167..4a3b8a2 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -9,7 +9,7 @@ use uuid::Uuid; use crate::transform::init_registry; use crate::utils; -use crate::{config::Config, git::clone_repo}; +use crate::{config::Config, git::clone_repo, git::open_repo}; #[derive(Parser, Debug, Clone)] #[command( @@ -90,11 +90,11 @@ pub fn run() -> Result<(), CliError> { init_registry(cfg.custom_transform_scripts()); - let _repository = match &cfg.repository.url { + let _repository = match cfg.repository.url { Some(url) => match try_retrieve_repo_from_url( cfg.repository.access_token, "wzslr321", - url, + &url, cfg.options.clone_into, ) { Ok(repo) => repo, @@ -102,21 +102,16 @@ pub fn run() -> Result<(), CliError> { return Err(CliError::Unknown); } }, - None => match &cfg.repository.path { - Some(path) => { - match try_retrieve_repo_from_path( - (*path) - .to_str() - .expect("Path is expected to be validated during serialization"), - ) { - Ok(repo) => repo, - Err(_) => { - return Err(CliError::IncorrectArgs { - msg: "Either repository url or path must be specified".to_string(), - }); - } + None => match cfg.repository.path { + Some(path) => match try_retrieve_repo_from_path(path) { + Ok(repo) => repo, + Err(err) => { + return Err(CliError::IncorrectArgs { + msg: "Either repository url or path must be specified".to_string(), + err: Some(err.into()), + }); } - } + }, None => { error!("Repository url and path are unspecified"); return Err(CliError::InvalidConfigPath { err: None }); @@ -130,11 +125,11 @@ pub fn run() -> Result<(), CliError> { fn check_args_validity(args: Args) -> Result<(), CliError> { match (&args.from_branch, &args.to_branch) { (None, None) => { - info!("No branches specified"); + trace!("No branches specified"); match &args.of_commit { Some(_) => Ok(()), None => { - error!("No commit specified. Nothing to analyze"); + error!("Neither commit nor branch specified. Nothing to analyze"); Err(CliError::InsufficientArgs) } } @@ -144,18 +139,27 @@ fn check_args_validity(args: Args) -> Result<(), CliError> { error!("from_branch specified, but to_branch is missing"); Err(CliError::IncorrectArgs { msg: "Specifying `from_branch` requires `to_branch` to be specified".to_string(), + err: None, }) } (Some(_), Some(_)) => Ok(()), } } -fn try_retrieve_repo_from_path(path: &str) -> Result> { - trace!( - "attempt to retireve repo path specified repository.\nPath:{}", - path - ); - todo!(); +fn try_retrieve_repo_from_path(path: Box) -> Result { + match open_repo(&path) { + Ok(repository) => { + info!("sucessfully retrieved repository from path"); + Ok(repository) + } + Err(err) => { + error!("failed to retrieve repository from path: {}", String::from((*path).to_string_lossy())); + Err(CliError::IncorrectArgs{ + msg: "Failed to retireve repository from path".to_string(), + err: Some(err.into()), + }) + } + } } fn try_retrieve_repo_from_url( @@ -190,14 +194,6 @@ fn try_retrieve_repo_from_url( } } -fn try_analyze_commit(_commit_id: &str) -> Result<(), Box> { - todo!() -} - -fn try_analyze_local_changes() -> Result<(), Box> { - todo!() -} - fn setup_logging(tracing_level: u8) { let tracing_level = match tracing_level { 0 => Level::TRACE, @@ -231,8 +227,8 @@ fn load_config(path: &Path) -> anyhow::Result { pub enum CliError { #[error("No branches and no commit specified. No local changes detected. Nothing to analyze.")] InsufficientArgs, - #[error("Incorrect CLI arguments.{}", msg)] - IncorrectArgs { msg: String }, + #[error("Incorrect CLI arguments.{}\nError:{:?}", msg, err)] + IncorrectArgs { msg: String, err: Option}, #[error("Config can not be retrieved")] InvalidConfigPath { err: Option }, #[error("Unknown error")] diff --git a/src/git.rs b/src/git.rs index d85c3a3..5428feb 100644 --- a/src/git.rs +++ b/src/git.rs @@ -20,6 +20,26 @@ pub enum GitError { path: String, err: git2::Error, }, + #[error("Failed to open repository from path: {}. Error: {}", path, err)] + OpenRepositoryFailure { path: String, err: git2::Error }, +} + +pub fn open_repo(path: &Path) -> Result { + info!("start opening repository"); + + match Repository::open(path) { + Ok(repository) => { + trace!("repository opened successfuly"); + Ok(repository) + } + Err(err) => { + error!("failed to open repository"); + Err(GitError::OpenRepositoryFailure { + path: String::from(path.to_string_lossy()), + err, + }) + } + } } pub fn clone_repo( From 9e6ab20aeebe4bf62af31954c0ba8896d29a52aa Mon Sep 17 00:00:00 2001 From: wzslr321 Date: Tue, 8 Oct 2024 23:30:21 +0200 Subject: [PATCH 4/9] feat: improve credentials passing and further error handling adjustments --- src/cli.rs | 74 ++++++++++++++++++++++++++++++++++----------- src/git.rs | 78 +++++++++++++++++++++++++++++++----------------- src/utils/mod.rs | 2 +- 3 files changed, 107 insertions(+), 47 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 4a3b8a2..3a1bef4 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -9,7 +9,7 @@ use uuid::Uuid; use crate::transform::init_registry; use crate::utils; -use crate::{config::Config, git::clone_repo, git::open_repo}; +use crate::{config::Config, git::clone_repo, git::extract_difference, git::open_repo}; #[derive(Parser, Debug, Clone)] #[command( @@ -69,6 +69,19 @@ struct Args { tracing_level: u8, } +pub enum Credentials<'a> { + UsernamePassword { + username: &'a str, + password: &'a str, + }, + SshKey { + username: String, + public_key_path: String, + private_key_path: String, + passphrase: Option, + }, +} + pub fn run() -> Result<(), CliError> { let args = Args::parse(); setup_logging(args.tracing_level); @@ -90,7 +103,7 @@ pub fn run() -> Result<(), CliError> { init_registry(cfg.custom_transform_scripts()); - let _repository = match cfg.repository.url { + let repository = match cfg.repository.url { Some(url) => match try_retrieve_repo_from_url( cfg.repository.access_token, "wzslr321", @@ -98,8 +111,8 @@ pub fn run() -> Result<(), CliError> { cfg.options.clone_into, ) { Ok(repo) => repo, - Err(_) => { - return Err(CliError::Unknown); + Err(e) => { + return Err(e); } }, None => match cfg.repository.path { @@ -119,6 +132,8 @@ pub fn run() -> Result<(), CliError> { }, }; + let _ = extract_difference(&repository); + Ok(()) } @@ -151,10 +166,13 @@ fn try_retrieve_repo_from_path(path: Box) -> Result Ok(repository) => { info!("sucessfully retrieved repository from path"); Ok(repository) - } - Err(err) => { - error!("failed to retrieve repository from path: {}", String::from((*path).to_string_lossy())); - Err(CliError::IncorrectArgs{ + } + Err(err) => { + error!( + "failed to retrieve repository from path: {}", + String::from((*path).to_string_lossy()) + ); + Err(CliError::IncorrectArgs { msg: "Failed to retireve repository from path".to_string(), err: Some(err.into()), }) @@ -167,29 +185,44 @@ fn try_retrieve_repo_from_url( username: &str, url: &Url, clone_into: Option>, -) -> Result> { +) -> Result { trace!("attempt to start from url-specified repository"); let clone_into_path = &clone_into.unwrap_or_else(|| { let path = Path::new(&format!("repository{}", Uuid::new_v4())).into(); trace!("set fallback clone_into path to {:?}", path); path }); + match utils::prepare_directory(&clone_into_path) { Ok(_) => { trace!("Starting to clone repository"); - let cloned_repo = match clone_repo(access_token, username, url, &clone_into_path) { + + let credentials = Credentials::UsernamePassword { + username, + password: &access_token.unwrap_or_else(|| "OnlyForTesting".to_string()), // expect("access_token must be specified, as it is the only supported authentication method for now"), + }; + let cloned_repo = match clone_repo(&credentials, url, &clone_into_path) { Ok(repo) => repo, Err(e) => { - error!("Failed to clone repository. error: {}", e); - return Err(e.into()); + error!("Failed to retreive repository from url.\nError: {}", e); + let err = match e { + crate::git::GitError::NoAccess { err } => CliError::InvalidArgs { + err: Some(err.into()), + }, + + _ => CliError::Unknown { + err: Some(e.into()), + }, + }; + return Err(err); } }; - info!("Repository cloned successfuly"); + info!("Repository retrieved successfuly from url"); Ok(cloned_repo) } - Err(e) => { + Err(err) => { error!("Failed to prepare directory for cloning"); - return Err(e.into()); + return Err(CliError::Unknown { err: Some(err) }); } } } @@ -228,9 +261,14 @@ pub enum CliError { #[error("No branches and no commit specified. No local changes detected. Nothing to analyze.")] InsufficientArgs, #[error("Incorrect CLI arguments.{}\nError:{:?}", msg, err)] - IncorrectArgs { msg: String, err: Option}, + IncorrectArgs { + msg: String, + err: Option, + }, + #[error("Invalid arguments. Error:{:?}", err)] + InvalidArgs { err: Option }, #[error("Config can not be retrieved")] InvalidConfigPath { err: Option }, - #[error("Unknown error")] - Unknown, + #[error("Unknown error: {:?}", err)] + Unknown { err: Option }, } diff --git a/src/git.rs b/src/git.rs index 5428feb..559c065 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1,14 +1,16 @@ -use std::{env, path::Path}; +use std::path::Path; use thiserror::Error; use git2::{Cred, RemoteCallbacks, Repository}; use tracing::{error, info, trace}; use url::Url; +use crate::cli::Credentials; + #[derive(Error, Debug)] pub enum GitError { - #[error("Failed to authorize git request, due to authentication failure. Error:{msg}")] - NoAccess { msg: String }, + #[error("Failed to authorize git request, due to authentication failure. Error:{err}")] + NoAccess { err: git2::Error }, #[error( "Failed to clone repository from url {} to given path: {}.\nError: {}", url, @@ -22,6 +24,14 @@ pub enum GitError { }, #[error("Failed to open repository from path: {}. Error: {}", path, err)] OpenRepositoryFailure { path: String, err: git2::Error }, + // #[error("Unknown error: {}", *err)] + // Unknown { err: Box }, +} + +pub struct FileDiff {} + +pub fn extract_difference(_repo: &Repository) -> Result, GitError> { + todo!() } pub fn open_repo(path: &Path) -> Result { @@ -42,37 +52,45 @@ pub fn open_repo(path: &Path) -> Result { } } +impl Credentials<'_> { + fn into(&self) -> Result { + let credentials = match self { + Credentials::UsernamePassword { username, password } => { + Cred::userpass_plaintext(&username, &password) + } + Credentials::SshKey { + username, + public_key_path, + private_key_path, + passphrase, + } => Cred::ssh_key( + &username, + Some(Path::new(public_key_path)), + Path::new(private_key_path), + passphrase.as_deref(), + ), + }; + + match credentials { + Ok(credentials) => Ok(credentials), + Err(err) => Err(err), + } + } +} + pub fn clone_repo( - access_token: Option, - username: &str, + credentials: &Credentials, url: &Url, clone_into: &Box, ) -> Result { info!("start cloning repository"); - trace!("try retrieve access token"); - let token = match access_token.to_owned() { - Some(token) => token, - None => match env::var("GITHUB_ACCESS_TOKEN") { - Ok(token) => token, - Err(_) => { - error!("failed to retrieve access token"); - return Err(GitError::NoAccess { - msg: "Access token unspecified".to_string(), - }); - } - }, - }; - trace!("Retrieved token successfully"); - let mut callbacks = RemoteCallbacks::new(); // TODO(wiktor.zajac) [https://github.com/wzslr321/impactifier/issues/9] // Support different credentials for git access // // Additionally, it can probably be extracted to a separate util - callbacks.credentials(|_url, _username_from_url, _allowed_types| { - Cred::userpass_plaintext(username, &token) - }); + callbacks.credentials(|_url, _username_from_url, _allowed_types| credentials.into()); trace!("Callback credentials set to userpass_plaintext"); let mut builder = git2::build::RepoBuilder::new(); @@ -93,11 +111,15 @@ pub fn clone_repo( } Err(e) => { error!("failed to clone repository"); - Err(GitError::CloneFailure { - url: url.to_string(), - path: String::from(clone_into.to_string_lossy()), - err: e, - }) + let err = match e.code() { + git2::ErrorCode::Auth => GitError::NoAccess { err: e }, + _ => GitError::CloneFailure { + url: url.to_string(), + path: String::from(clone_into.to_string_lossy()), + err: e, + }, + }; + Err(err) } } } diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 2263e79..7049829 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -1,7 +1,7 @@ use std::{fs, path::Path}; use tracing::{info, trace}; -pub fn prepare_directory(path: &Path) -> Result<(), Box> { +pub fn prepare_directory(path: &Path) -> Result<(), anyhow::Error> { if path.exists() { if path.read_dir()?.next().is_some() { info!("Directory is not empty, removing existing files..."); From b41f885c232f4817865c274e9daa8f04ce86d3b7 Mon Sep 17 00:00:00 2001 From: wzslr321 Date: Wed, 9 Oct 2024 22:11:01 +0200 Subject: [PATCH 5/9] feat: basic deltas extraction --- src/cli.rs | 26 +++++++++++----- src/git.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 94 insertions(+), 23 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 3a1bef4..88cb418 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -69,17 +69,12 @@ struct Args { tracing_level: u8, } +// TODO add more credentials variants pub enum Credentials<'a> { UsernamePassword { username: &'a str, password: &'a str, }, - SshKey { - username: String, - public_key_path: String, - private_key_path: String, - passphrase: Option, - }, } pub fn run() -> Result<(), CliError> { @@ -132,7 +127,22 @@ pub fn run() -> Result<(), CliError> { }, }; - let _ = extract_difference(&repository); + let credentials = Credentials::UsernamePassword { + username: "wzslr321", + password: "TEST", + }; + + crate::git::fetch_remote(&repository, "origin", &credentials).unwrap(); + + let diff = extract_difference( + &repository, + &crate::git::DiffOptions::Branches { + from: &args.from_branch.unwrap(), + to: &args.to_branch.unwrap_or_else(|| "main".to_string()), + }, + ); + + println!("{:?}", diff); Ok(()) } @@ -199,7 +209,7 @@ fn try_retrieve_repo_from_url( let credentials = Credentials::UsernamePassword { username, - password: &access_token.unwrap_or_else(|| "OnlyForTesting".to_string()), // expect("access_token must be specified, as it is the only supported authentication method for now"), + password: &access_token.unwrap_or_else(|| "OnlyForTesting".to_string()), // ehttps://www.twitch.tv/directory/followingxpect("access_token must be specified, as it is the only supported authentication method for now"), }; let cloned_repo = match clone_repo(&credentials, url, &clone_into_path) { Ok(repo) => repo, diff --git a/src/git.rs b/src/git.rs index 559c065..2b98704 100644 --- a/src/git.rs +++ b/src/git.rs @@ -2,6 +2,7 @@ use std::path::Path; use thiserror::Error; use git2::{Cred, RemoteCallbacks, Repository}; +use std::str; use tracing::{error, info, trace}; use url::Url; @@ -28,10 +29,81 @@ pub enum GitError { // Unknown { err: Box }, } -pub struct FileDiff {} +#[derive(Debug)] +pub struct Diff { + pub deltas: Vec, +} + +#[derive(Debug)] +pub struct FileDelta { + pub value: String, +} + +impl FileDelta { + fn from(value: String) -> Self { + Self { value } + } +} + +pub enum DiffOptions<'a> { + Branches { from: &'a str, to: &'a str }, +} + +pub fn extract_difference(repo: &Repository, options: &DiffOptions) -> anyhow::Result { + match options { + DiffOptions::Branches { from, to } => extract_difference_branches(repo, from, to), + } +} + +pub fn fetch_remote(repo: &Repository, remote_name: &str, credentials: &Credentials) -> anyhow::Result<()> { + // Find the remote + let mut remote = repo.find_remote(remote_name)?; + + // Set up callbacks for authentication (if needed) + let mut cb = RemoteCallbacks::new(); + cb.credentials(|_url, _username_from_url, _allowed_types| credentials.into()); + + // Configure fetch options with the callbacks + let mut fetch_options = git2::FetchOptions::new(); + fetch_options.remote_callbacks(cb); + + // Define the refspecs to fetch. Here, we fetch all branches. + let refspecs = ["+refs/heads/*:refs/remotes/origin/*"]; + + // Perform the fetch + remote.fetch(&refspecs, Some(&mut fetch_options), None)?; + + Ok(()) +} + +pub fn extract_difference_branches( + repo: &Repository, + from_branch: &str, + to_branch: &str, +) -> anyhow::Result { + let ref_from = repo.find_reference(&format!("refs/heads/{}", from_branch))?; + let ref_to = repo.find_reference(&format!("refs/remotes/origin/{}", to_branch))?; + + let commit_a = ref_from.peel_to_commit()?; + let commit_b = ref_to.peel_to_commit()?; + + let tree_a = commit_a.tree()?; + let tree_b = commit_b.tree()?; + + let diff = repo.diff_tree_to_tree(Some(&tree_a), Some(&tree_b), None)?; + let mut diff_output = Vec::new(); + diff.print(git2::DiffFormat::Patch, |_delta, _hunk, line| { + diff_output.extend_from_slice(line.content()); + true + })?; + + let diff_str = str::from_utf8(&diff_output) + .map_err(|e| git2::Error::from_str(&format!("UTF-8 conversion error: {}", e)))? + .to_string(); -pub fn extract_difference(_repo: &Repository) -> Result, GitError> { - todo!() + Ok(Diff { + deltas: vec![FileDelta::from(diff_str)], + }) } pub fn open_repo(path: &Path) -> Result { @@ -58,17 +130,6 @@ impl Credentials<'_> { Credentials::UsernamePassword { username, password } => { Cred::userpass_plaintext(&username, &password) } - Credentials::SshKey { - username, - public_key_path, - private_key_path, - passphrase, - } => Cred::ssh_key( - &username, - Some(Path::new(public_key_path)), - Path::new(private_key_path), - passphrase.as_deref(), - ), }; match credentials { @@ -114,7 +175,7 @@ pub fn clone_repo( let err = match e.code() { git2::ErrorCode::Auth => GitError::NoAccess { err: e }, _ => GitError::CloneFailure { - url: url.to_string(), + url: url.to_string(), path: String::from(clone_into.to_string_lossy()), err: e, }, From 93da68edf69ccf19bbb4f40f795cabf343af029b Mon Sep 17 00:00:00 2001 From: wzslr321 Date: Wed, 9 Oct 2024 22:56:38 +0200 Subject: [PATCH 6/9] feat: save diff to file and use it on CI to post as a comment --- .github/workflows/impactifier.yml | 94 ++++++++++++++++++++++++++++--- Cargo.lock | 5 +- Cargo.toml | 1 + src/cli.rs | 17 ++++-- src/git.rs | 5 +- 5 files changed, 105 insertions(+), 17 deletions(-) diff --git a/.github/workflows/impactifier.yml b/.github/workflows/impactifier.yml index db2b6d9..d7a5877 100644 --- a/.github/workflows/impactifier.yml +++ b/.github/workflows/impactifier.yml @@ -11,17 +11,18 @@ permissions: issues: write pull-requests: write - jobs: impactifier: runs-on: ubuntu-latest steps: + # 1. Checkout the repository with full history - name: Checkout Repository uses: actions/checkout@v4 with: - fetch-depth: 0 + fetch-depth: 0 # Fetch all history for all branches + # 2. Cache Cargo Registry - name: Cache Cargo Registry uses: actions/cache@v3 with: @@ -30,6 +31,7 @@ jobs: restore-keys: | ${{ runner.os }}-cargo-registry- + # 3. Cache Cargo Build - name: Cache Cargo Build uses: actions/cache@v3 with: @@ -38,24 +40,98 @@ jobs: restore-keys: | ${{ runner.os }}-cargo-build- + # 4. Build Impactifier - name: Build Impactifier run: | cargo build --release --manifest-path Cargo.toml + # 5. Run Impactifier and generate diff.json - name: Run Impactifier + id: run_impactifier env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - ./target/release/impactifier --config impactifier-config.yaml + ./target/release/impactifier --tracing-level=0 --from-branch=main --to-branch=refactor + + # 6. (Optional) Output diff.json for debugging + - name: Output diff.json (Debug) + if: ${{ github.event_name == 'pull_request' }} + run: | + cat diff.json + # 7. Post Comment on Pull Request with the diff - name: Post Comment on Pull Request if: github.event_name == 'pull_request' uses: actions/github-script@v6 with: script: | - github.rest.issues.createComment({ - issue_number: context.issue.number, - owner: context.repo.owner, - repo: context.repo.repo, - body: `## Impactifier Report` - }); + const fs = require('fs'); + const path = 'diff.json'; // Path to the diff JSON file + + // Check if the diff file exists + if (!fs.existsSync(path)) { + console.log('No diff.json file found.'); + return; + } + + // Read and parse the diff JSON + let diffData; + try { + const rawData = fs.readFileSync(path, 'utf8'); + diffData = JSON.parse(rawData); + } catch (error) { + console.error('Failed to read or parse diff.json:', error); + return; + } + + // Format the diff for the comment + let formattedDiff = ''; + if (diffData.deltas && Array.isArray(diffData.deltas)) { + diffData.deltas.forEach(delta => { + if (delta.value) { + // Escape backticks in the delta.value to prevent breaking the Markdown + const safeValue = delta.value.replace(/`/g, '\\`'); + formattedDiff += `${safeValue}\n`; + } + }); + } else { + formattedDiff = 'No differences found.'; + } + + // Handle large diffs by truncating (optional) + const maxLength = 60000; // GitHub comment limit + let truncatedDiff = formattedDiff; + if (formattedDiff.length > maxLength) { + truncatedDiff = formattedDiff.substring(0, maxLength) + '\n... (diff truncated)'; + } + + // Create a summary based on the number of deltas + let summary = ''; + if (diffData.deltas && diffData.deltas.length > 0) { + summary = `**Total Changes:** ${diffData.deltas.length} file(s) changed.\n\n`; + } else { + summary = 'No changes detected between the specified branches.\n\n'; + } + + // Create the comment body with summary and diff + const commentBody = `## Impactifier Report + + ${summary} + + \`\`\`diff + ${truncatedDiff} + \`\`\``; + + // Post the comment to the pull request + try { + await github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: commentBody + }); + console.log('Impactifier report posted successfully.'); + } catch (error) { + console.error('Failed to post Impactifier report:', error); + } + diff --git a/Cargo.lock b/Cargo.lock index 5855337..c39b024 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -365,6 +365,7 @@ dependencies = [ "rhai", "serde", "serde_derive", + "serde_json", "serde_yaml", "thiserror", "toml", @@ -774,9 +775,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.125" +version = "1.0.128" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "83c8e735a073ccf5be70aa8066aa984eaf2fa000db6c8d0100ae605b366d31ed" +checksum = "6ff5456707a1de34e7e37f2a6fd3d3f808c318259cbd01ab6377795054b483d8" dependencies = [ "itoa", "memchr", diff --git a/Cargo.toml b/Cargo.toml index c1f2f85..a2845e1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ lazy_static = "1.5.0" rhai = "1.19.0" serde = "1.0.208" serde_derive = "1.0.208" +serde_json = "1.0.128" serde_yaml = "0.9.34" thiserror = "1.0.64" toml = "0.8.19" diff --git a/src/cli.rs b/src/cli.rs index 88cb418..f9aa3af 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,3 +1,5 @@ +use std::fs::File; +use std::io::Write; use std::path::Path; use clap::Parser; @@ -6,6 +8,7 @@ use thiserror::Error; use tracing::{error, info, trace, Level}; use url::Url; use uuid::Uuid; +use serde_json::to_string_pretty; use crate::transform::init_registry; use crate::utils; @@ -67,6 +70,9 @@ struct Args { /// 4 = Error #[arg(long, default_value_t = 2)] tracing_level: u8, + + #[arg(long, default_value_t=String::from("origin"))] + origin: String, } // TODO add more credentials variants @@ -127,12 +133,12 @@ pub fn run() -> Result<(), CliError> { }, }; - let credentials = Credentials::UsernamePassword { + let mock_credentials = Credentials::UsernamePassword { username: "wzslr321", password: "TEST", }; - crate::git::fetch_remote(&repository, "origin", &credentials).unwrap(); + crate::git::fetch_remote(&repository, &args.origin, &mock_credentials).unwrap(); let diff = extract_difference( &repository, @@ -140,9 +146,12 @@ pub fn run() -> Result<(), CliError> { from: &args.from_branch.unwrap(), to: &args.to_branch.unwrap_or_else(|| "main".to_string()), }, - ); + ) + .unwrap(); + let serialized_diff = to_string_pretty(&diff).unwrap(); - println!("{:?}", diff); + let mut file = File::create("./diff.json").unwrap(); + file.write_all(serialized_diff.as_bytes()).unwrap(); Ok(()) } diff --git a/src/git.rs b/src/git.rs index 2b98704..499e20f 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1,4 +1,5 @@ use std::path::Path; +use serde::Serialize; use thiserror::Error; use git2::{Cred, RemoteCallbacks, Repository}; @@ -29,12 +30,12 @@ pub enum GitError { // Unknown { err: Box }, } -#[derive(Debug)] +#[derive(Debug, Serialize)] pub struct Diff { pub deltas: Vec, } -#[derive(Debug)] +#[derive(Debug, Serialize)] pub struct FileDelta { pub value: String, } From f1929f7921b2b3fcd86613c8f4c8a8c0367aad6c Mon Sep 17 00:00:00 2001 From: wzslr321 Date: Thu, 10 Oct 2024 19:44:39 +0200 Subject: [PATCH 7/9] feat: use anyhow::Result throughout the codebase; comment unused code --- src/cli.rs | 24 +++++++++++------------- src/config.rs | 47 +++++++++++++++++++++++------------------------ src/git.rs | 1 + src/main.rs | 1 + src/transform.rs | 1 + src/utils/mod.rs | 12 +++++++++++- 6 files changed, 48 insertions(+), 38 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index f9aa3af..81d9746 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -12,7 +12,9 @@ use serde_json::to_string_pretty; use crate::transform::init_registry; use crate::utils; -use crate::{config::Config, git::clone_repo, git::extract_difference, git::open_repo}; +use crate::git; +use crate::config; +use anyhow::Result; #[derive(Parser, Debug, Clone)] #[command( @@ -75,7 +77,7 @@ struct Args { origin: String, } -// TODO add more credentials variants +// TODO: add more credentials variants pub enum Credentials<'a> { UsernamePassword { username: &'a str, @@ -133,14 +135,10 @@ pub fn run() -> Result<(), CliError> { }, }; - let mock_credentials = Credentials::UsernamePassword { - username: "wzslr321", - password: "TEST", - }; - - crate::git::fetch_remote(&repository, &args.origin, &mock_credentials).unwrap(); + let credentials = utils::get_mock_credentials(); + git::fetch_remote(&repository, &args.origin, &credentials).unwrap(); - let diff = extract_difference( + let diff = git::extract_difference( &repository, &crate::git::DiffOptions::Branches { from: &args.from_branch.unwrap(), @@ -181,7 +179,7 @@ fn check_args_validity(args: Args) -> Result<(), CliError> { } fn try_retrieve_repo_from_path(path: Box) -> Result { - match open_repo(&path) { + match git::open_repo(&path) { Ok(repository) => { info!("sucessfully retrieved repository from path"); Ok(repository) @@ -220,7 +218,7 @@ fn try_retrieve_repo_from_url( username, password: &access_token.unwrap_or_else(|| "OnlyForTesting".to_string()), // ehttps://www.twitch.tv/directory/followingxpect("access_token must be specified, as it is the only supported authentication method for now"), }; - let cloned_repo = match clone_repo(&credentials, url, &clone_into_path) { + let cloned_repo = match git::clone_repo(&credentials, url, &clone_into_path) { Ok(repo) => repo, Err(e) => { error!("Failed to retreive repository from url.\nError: {}", e); @@ -261,9 +259,9 @@ fn setup_logging(tracing_level: u8) { .init(); } -fn load_config(path: &Path) -> anyhow::Result { +fn load_config(path: &Path) -> anyhow::Result { trace!("Starting loading config from {:?}", path); - match Config::load_from_file(path) { + match config::Config::load_from_file(path) { Ok(config) => { info!("Config loaded successfully"); Ok(config) diff --git a/src/config.rs b/src/config.rs index c94c62f..b0d9e57 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,10 +1,9 @@ -use anyhow::{Context, Result}; +use anyhow::Result; use serde::{Deserialize, Deserializer}; use tracing::error; use std::cmp; use std::fmt; use std::path::Path; -use std::path::PathBuf; use thiserror::Error; use tracing::debug; use url::Url; @@ -35,16 +34,16 @@ pub struct OptionsConfig { pub clone_into: Option>, } -#[derive(Debug, Deserialize)] -pub struct Trigger { - pub path: Box, - - #[serde(default)] - pub pattern: Option, - - #[serde(default)] - pub analyze_dependencies: bool, -} +// #[derive(Debug, Deserialize)] +// pub struct Trigger { +// pub path: Box, +// +// #[serde(default)] +// pub pattern: Option, +// +// #[serde(default)] +// pub analyze_dependencies: bool, +// } #[derive(Debug, Deserialize)] pub struct TransformStep { @@ -55,17 +54,17 @@ pub struct TransformStep { #[derive(Debug, Deserialize)] pub struct Transform { - pub name: String, + // pub name: String, #[serde(default)] pub steps: Vec, } -#[derive(Debug, Deserialize)] -pub struct Matcher { - pub path: PathBuf, - pub pattern: String, -} +// #[derive(Debug, Deserialize)] +// pub struct Matcher { +// pub path: PathBuf, +// pub pattern: String, +// } #[derive(Debug, Deserialize)] pub enum AlertLevel { @@ -76,17 +75,17 @@ pub enum AlertLevel { #[derive(Debug, Deserialize)] pub struct Action { - pub alert_level: AlertLevel, - pub message: String, + // pub alert_level: AlertLevel, + // pub message: String, } #[derive(Debug, Deserialize)] pub struct Rule { - pub name: String, - pub trigger: Trigger, + // pub name: String, + // pub trigger: Trigger, pub transform: Transform, - pub matcher: Matcher, - pub action: Action, + // pub matcher: Matcher, + // pub action: Action, } pub struct CustomStep { diff --git a/src/git.rs b/src/git.rs index 499e20f..4d58492 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1,6 +1,7 @@ use std::path::Path; use serde::Serialize; use thiserror::Error; +use anyhow::Result; use git2::{Cred, RemoteCallbacks, Repository}; use std::str; diff --git a/src/main.rs b/src/main.rs index d8d2e36..32ac406 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,6 +5,7 @@ mod utils; mod transform; use cli::CliError; +use anyhow::Result; fn main() -> Result<(), CliError> { cli::run() diff --git a/src/transform.rs b/src/transform.rs index c4f01cf..1f71aa5 100644 --- a/src/transform.rs +++ b/src/transform.rs @@ -1,4 +1,5 @@ use crate::config::CustomStep; +use anyhow::Result; use rhai::{Dynamic, Engine, Map, Scope}; use tracing::trace; use std::collections::HashMap; diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 7049829..36c25f0 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -1,7 +1,10 @@ use std::{fs, path::Path}; use tracing::{info, trace}; +use anyhow::Result; -pub fn prepare_directory(path: &Path) -> Result<(), anyhow::Error> { +use crate::cli::Credentials; + +pub fn prepare_directory(path: &Path) -> Result<()> { if path.exists() { if path.read_dir()?.next().is_some() { info!("Directory is not empty, removing existing files..."); @@ -15,3 +18,10 @@ pub fn prepare_directory(path: &Path) -> Result<(), anyhow::Error> { trace!("Successfully prepared directory for cloning"); Ok(()) } + +pub fn get_mock_credentials<'a>() -> &'a Credentials<'a> { + &Credentials::UsernamePassword { + username: "wzslr321", + password: "TEST", + } +} From f6767440f9c9a8b7bc11cfe4ee0e247fc68a4ff7 Mon Sep 17 00:00:00 2001 From: wzslr321 Date: Thu, 10 Oct 2024 21:54:08 +0200 Subject: [PATCH 8/9] feat: major cleanup --- src/cli.rs | 216 ++++++++++++++++++----------------------------- src/config.rs | 21 ----- src/git.rs | 16 ++-- src/transform.rs | 12 +-- 4 files changed, 100 insertions(+), 165 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 81d9746..8713831 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,19 +1,19 @@ +use anyhow::{anyhow, Context}; use std::fs::File; use std::io::Write; use std::path::Path; use clap::Parser; use git2::Repository; +use serde_json::to_string_pretty; use thiserror::Error; use tracing::{error, info, trace, Level}; use url::Url; -use uuid::Uuid; -use serde_json::to_string_pretty; +use crate::config::Config; +use crate::git; use crate::transform::init_registry; use crate::utils; -use crate::git; -use crate::config; use anyhow::Result; #[derive(Parser, Debug, Clone)] @@ -45,14 +45,14 @@ struct Args { config: String, /// From what branch changes should be compared. - /// - /// Defaults to the current branch. - #[arg(short, long)] + #[arg(long)] from_branch: Option, - /// To what branch changes should be compared - #[arg(long)] - to_branch: Option, + /// To what branch changes should be compared. + /// + /// Defaults to "main" + #[arg(long, default_value_t=String::from("main"))] + to_branch: String, /// Commit of which changes should be analyzed. Takes precedence over /// branch changes, if `from_branch` or `to_branch` is specified. @@ -89,63 +89,71 @@ pub fn run() -> Result<(), CliError> { let args = Args::parse(); setup_logging(args.tracing_level); - match check_args_validity(args.clone()) { - Ok(_) => { - trace!("args validation completed successfully. Continuing execution."); - } - Err(err) => { - error!("args are invalid. Exiting..."); - return Err(err); - } - } - let cfg = match load_config(Path::new(&args.config)) { Ok(config) => config, - Err(e) => return Err(e), + Err(e) => { + error!("initial config load failed. Exciting..."); + return Err(e); + } }; + trace!("Initial config load succeeded"); init_registry(cfg.custom_transform_scripts()); + trace!("Transform functions initialized successfully"); - let repository = match cfg.repository.url { - Some(url) => match try_retrieve_repo_from_url( - cfg.repository.access_token, - "wzslr321", - &url, - cfg.options.clone_into, - ) { - Ok(repo) => repo, - Err(e) => { - return Err(e); - } - }, - None => match cfg.repository.path { - Some(path) => match try_retrieve_repo_from_path(path) { - Ok(repo) => repo, - Err(err) => { - return Err(CliError::IncorrectArgs { - msg: "Either repository url or path must be specified".to_string(), - err: Some(err.into()), - }); - } - }, + // TODO: Retrieve properly from args + let mock_credentials = utils::get_mock_credentials(); + + let clone_into = match cfg.options.clone_into.as_deref() { + Some(path) => path, + None => Path::new("cloned_repository"), + }; + + let repository_retrieval_result = match cfg.repository.url { + Some(url) => try_retrieve_repo_from_url(mock_credentials, &url, clone_into), + None => match &cfg.repository.path { + Some(path) => try_retrieve_repo_from_path(path), None => { - error!("Repository url and path are unspecified"); - return Err(CliError::InvalidConfigPath { err: None }); + return Err(CliError::InvalidArgs { + err: Some(anyhow!("Either path or url must be specified")), + }); } }, }; - let credentials = utils::get_mock_credentials(); - git::fetch_remote(&repository, &args.origin, &credentials).unwrap(); + let repository = match repository_retrieval_result { + Ok(repository) => repository, + Err(err) => return Err(CliError::Unknown { err: Some(err) }), + }; + trace!("Successfully retrieved repository"); - let diff = git::extract_difference( + if let Err(fetch_err) = git::fetch_remote(&repository, &args.origin, &mock_credentials) { + error!("Failed to fetch remote"); + return Err(CliError::Unknown { + err: Some(fetch_err), + }); + } + trace!("Successfully fetched remote"); + + // TODO: Support other DiffOptions + // + // Current one is temporary, just for testing purposes + let diff = match git::extract_difference( &repository, - &crate::git::DiffOptions::Branches { + &git::DiffOptions::Branches { from: &args.from_branch.unwrap(), - to: &args.to_branch.unwrap_or_else(|| "main".to_string()), + to: &args.to_branch, }, - ) - .unwrap(); + ) { + Ok(diff) => diff, + Err(err) => { + error!("Failed to extract difference"); + return Err(CliError::Unknown { err: Some(err) }); + } + }; + trace!("Successfuly extracted difference"); + + // Temporary, for testing purposes let serialized_diff = to_string_pretty(&diff).unwrap(); let mut file = File::create("./diff.json").unwrap(); @@ -154,94 +162,43 @@ pub fn run() -> Result<(), CliError> { Ok(()) } -fn check_args_validity(args: Args) -> Result<(), CliError> { - match (&args.from_branch, &args.to_branch) { - (None, None) => { - trace!("No branches specified"); - match &args.of_commit { - Some(_) => Ok(()), - None => { - error!("Neither commit nor branch specified. Nothing to analyze"); - Err(CliError::InsufficientArgs) - } - } - } - (None, Some(_)) => Ok(()), - (Some(_), None) => { - error!("from_branch specified, but to_branch is missing"); - Err(CliError::IncorrectArgs { - msg: "Specifying `from_branch` requires `to_branch` to be specified".to_string(), - err: None, - }) - } - (Some(_), Some(_)) => Ok(()), - } -} - -fn try_retrieve_repo_from_path(path: Box) -> Result { +fn try_retrieve_repo_from_path(path: &Path) -> Result { match git::open_repo(&path) { Ok(repository) => { info!("sucessfully retrieved repository from path"); Ok(repository) } Err(err) => { - error!( - "failed to retrieve repository from path: {}", - String::from((*path).to_string_lossy()) - ); - Err(CliError::IncorrectArgs { - msg: "Failed to retireve repository from path".to_string(), - err: Some(err.into()), - }) + return Err(anyhow!( + "Failed to retrieve repository from path: {:?}.\nError:{}", + path, + err, + )); } } } fn try_retrieve_repo_from_url( - access_token: Option, - username: &str, + credentials: &Credentials, url: &Url, - clone_into: Option>, -) -> Result { + clone_into: &Path, +) -> Result { trace!("attempt to start from url-specified repository"); - let clone_into_path = &clone_into.unwrap_or_else(|| { - let path = Path::new(&format!("repository{}", Uuid::new_v4())).into(); - trace!("set fallback clone_into path to {:?}", path); - path - }); - - match utils::prepare_directory(&clone_into_path) { - Ok(_) => { - trace!("Starting to clone repository"); + utils::prepare_directory(&clone_into) + .with_context(|| "Failed to prepare directory for cloning")?; - let credentials = Credentials::UsernamePassword { - username, - password: &access_token.unwrap_or_else(|| "OnlyForTesting".to_string()), // ehttps://www.twitch.tv/directory/followingxpect("access_token must be specified, as it is the only supported authentication method for now"), - }; - let cloned_repo = match git::clone_repo(&credentials, url, &clone_into_path) { - Ok(repo) => repo, - Err(e) => { - error!("Failed to retreive repository from url.\nError: {}", e); - let err = match e { - crate::git::GitError::NoAccess { err } => CliError::InvalidArgs { - err: Some(err.into()), - }, - - _ => CliError::Unknown { - err: Some(e.into()), - }, - }; - return Err(err); - } - }; - info!("Repository retrieved successfuly from url"); - Ok(cloned_repo) - } + trace!("Starting to clone repository"); + let cloned_repo = match git::clone_repo(&credentials, url, &clone_into) { + Ok(repo) => repo, Err(err) => { - error!("Failed to prepare directory for cloning"); - return Err(CliError::Unknown { err: Some(err) }); + return Err(anyhow!( + "Failed to clone repository from url.\nError: {}", + err + )); } - } + }; + + Ok(cloned_repo) } fn setup_logging(tracing_level: u8) { @@ -259,9 +216,9 @@ fn setup_logging(tracing_level: u8) { .init(); } -fn load_config(path: &Path) -> anyhow::Result { +fn load_config(path: &Path) -> Result { trace!("Starting loading config from {:?}", path); - match config::Config::load_from_file(path) { + match Config::load_from_file(path) { Ok(config) => { info!("Config loaded successfully"); Ok(config) @@ -275,13 +232,6 @@ fn load_config(path: &Path) -> anyhow::Result { #[derive(Error, Debug)] pub enum CliError { - #[error("No branches and no commit specified. No local changes detected. Nothing to analyze.")] - InsufficientArgs, - #[error("Incorrect CLI arguments.{}\nError:{:?}", msg, err)] - IncorrectArgs { - msg: String, - err: Option, - }, #[error("Invalid arguments. Error:{:?}", err)] InvalidArgs { err: Option }, #[error("Config can not be retrieved")] diff --git a/src/config.rs b/src/config.rs index b0d9e57..6c1691a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -34,17 +34,6 @@ pub struct OptionsConfig { pub clone_into: Option>, } -// #[derive(Debug, Deserialize)] -// pub struct Trigger { -// pub path: Box, -// -// #[serde(default)] -// pub pattern: Option, -// -// #[serde(default)] -// pub analyze_dependencies: bool, -// } - #[derive(Debug, Deserialize)] pub struct TransformStep { pub name: String, @@ -60,12 +49,6 @@ pub struct Transform { pub steps: Vec, } -// #[derive(Debug, Deserialize)] -// pub struct Matcher { -// pub path: PathBuf, -// pub pattern: String, -// } - #[derive(Debug, Deserialize)] pub enum AlertLevel { Info, @@ -81,11 +64,7 @@ pub struct Action { #[derive(Debug, Deserialize)] pub struct Rule { - // pub name: String, - // pub trigger: Trigger, pub transform: Transform, - // pub matcher: Matcher, - // pub action: Action, } pub struct CustomStep { diff --git a/src/git.rs b/src/git.rs index 4d58492..b037006 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1,7 +1,7 @@ -use std::path::Path; +use anyhow::Result; use serde::Serialize; +use std::path::Path; use thiserror::Error; -use anyhow::Result; use git2::{Cred, RemoteCallbacks, Repository}; use std::str; @@ -51,13 +51,17 @@ pub enum DiffOptions<'a> { Branches { from: &'a str, to: &'a str }, } -pub fn extract_difference(repo: &Repository, options: &DiffOptions) -> anyhow::Result { +pub fn extract_difference(repo: &Repository, options: &DiffOptions) -> Result { match options { DiffOptions::Branches { from, to } => extract_difference_branches(repo, from, to), } } -pub fn fetch_remote(repo: &Repository, remote_name: &str, credentials: &Credentials) -> anyhow::Result<()> { +pub fn fetch_remote( + repo: &Repository, + remote_name: &str, + credentials: &Credentials, +) -> Result<()> { // Find the remote let mut remote = repo.find_remote(remote_name)?; @@ -82,7 +86,7 @@ pub fn extract_difference_branches( repo: &Repository, from_branch: &str, to_branch: &str, -) -> anyhow::Result { +) -> Result { let ref_from = repo.find_reference(&format!("refs/heads/{}", from_branch))?; let ref_to = repo.find_reference(&format!("refs/remotes/origin/{}", to_branch))?; @@ -144,7 +148,7 @@ impl Credentials<'_> { pub fn clone_repo( credentials: &Credentials, url: &Url, - clone_into: &Box, + clone_into: &Path, ) -> Result { info!("start cloning repository"); diff --git a/src/transform.rs b/src/transform.rs index 1f71aa5..bb044c7 100644 --- a/src/transform.rs +++ b/src/transform.rs @@ -1,10 +1,11 @@ use crate::config::CustomStep; +use anyhow::anyhow; use anyhow::Result; use rhai::{Dynamic, Engine, Map, Scope}; -use tracing::trace; use std::collections::HashMap; use std::path::PathBuf; use std::sync::Mutex; +use tracing::trace; #[derive(Debug, Clone)] pub struct Context { @@ -35,13 +36,15 @@ fn register_transform(name: &str, func: Box) { } pub fn init_registry(custom_steps: Option>) { - trace!("init_registry started"); + trace!("Starting to register transform scripts functions"); + register_transform("toUpperCase", Box::new(ToLowerCase)); register_transform("replace", Box::new(Replace)); - trace!("standard functions registered"); + trace!("Standard functions registered"); + if let Some(steps) = custom_steps { for step in steps { - trace!("initializing custom function {}", &step.name); + trace!("Initializing custom function {}", &step.name); register_transform( &step.name, Box::new(CustomFunction { @@ -50,7 +53,6 @@ pub fn init_registry(custom_steps: Option>) { ); } } - trace!("init_registry finished"); } pub struct ToLowerCase; From 0d078f5ed1199c07c81a4270cdbc8ce42f202b21 Mon Sep 17 00:00:00 2001 From: wzslr321 Date: Thu, 10 Oct 2024 22:02:13 +0200 Subject: [PATCH 9/9] chore: remove comments --- src/git.rs | 32 ++++++-------------------------- src/transform.rs | 1 - 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/src/git.rs b/src/git.rs index b037006..8148823 100644 --- a/src/git.rs +++ b/src/git.rs @@ -27,8 +27,6 @@ pub enum GitError { }, #[error("Failed to open repository from path: {}. Error: {}", path, err)] OpenRepositoryFailure { path: String, err: git2::Error }, - // #[error("Unknown error: {}", *err)] - // Unknown { err: Box }, } #[derive(Debug, Serialize)] @@ -57,26 +55,17 @@ pub fn extract_difference(repo: &Repository, options: &DiffOptions) -> Result Result<()> { - // Find the remote +pub fn fetch_remote(repo: &Repository, remote_name: &str, credentials: &Credentials) -> Result<()> { let mut remote = repo.find_remote(remote_name)?; - // Set up callbacks for authentication (if needed) - let mut cb = RemoteCallbacks::new(); - cb.credentials(|_url, _username_from_url, _allowed_types| credentials.into()); + let mut callback = RemoteCallbacks::new(); + callback.credentials(|_url, _username_from_url, _allowed_types| credentials.into()); - // Configure fetch options with the callbacks let mut fetch_options = git2::FetchOptions::new(); - fetch_options.remote_callbacks(cb); + fetch_options.remote_callbacks(callback); - // Define the refspecs to fetch. Here, we fetch all branches. let refspecs = ["+refs/heads/*:refs/remotes/origin/*"]; - // Perform the fetch remote.fetch(&refspecs, Some(&mut fetch_options), None)?; Ok(()) @@ -87,6 +76,7 @@ pub fn extract_difference_branches( from_branch: &str, to_branch: &str, ) -> Result { + // TODO: Those refs values most likely should not be hardcoded let ref_from = repo.find_reference(&format!("refs/heads/{}", from_branch))?; let ref_to = repo.find_reference(&format!("refs/remotes/origin/{}", to_branch))?; @@ -153,10 +143,6 @@ pub fn clone_repo( info!("start cloning repository"); let mut callbacks = RemoteCallbacks::new(); - // TODO(wiktor.zajac) [https://github.com/wzslr321/impactifier/issues/9] - // Support different credentials for git access - // - // Additionally, it can probably be extracted to a separate util callbacks.credentials(|_url, _username_from_url, _allowed_types| credentials.into()); trace!("Callback credentials set to userpass_plaintext"); @@ -167,15 +153,9 @@ pub fn clone_repo( fetch_options.depth(1); builder.fetch_options(fetch_options); - // TODO(wiktor.zajac) try to guard agains future changes, to update this trace automatically - // by using some implemented trait with display - trace!("FetchOptions set to depth=0"); match builder.clone(url.as_str(), &clone_into) { - Ok(repository) => { - info!("repository cloned successfully"); - Ok(repository) - } + Ok(repository) => Ok(repository), Err(e) => { error!("failed to clone repository"); let err = match e.code() { diff --git a/src/transform.rs b/src/transform.rs index bb044c7..6467ee4 100644 --- a/src/transform.rs +++ b/src/transform.rs @@ -1,5 +1,4 @@ use crate::config::CustomStep; -use anyhow::anyhow; use anyhow::Result; use rhai::{Dynamic, Engine, Map, Scope}; use std::collections::HashMap;