From b4daf13297af8146306a8662f73d25adde6780e6 Mon Sep 17 00:00:00 2001 From: Venus Xeon-Blonde Date: Wed, 10 Jul 2024 14:00:54 -0400 Subject: [PATCH 1/7] Update log crate and add shim methods between git2's tracing and log --- Cargo.toml | 2 +- src/lib.rs | 2 +- src/tracing.rs | 159 +++++++++++++++++++++++++++++++++---------------- 3 files changed, 111 insertions(+), 52 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9e6fd81f56..d7447ca40d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ edition = "2018" url = "2.0" bitflags = "2.1.0" libc = "0.2" -log = "0.4.8" +log = "0.4.22" libgit2-sys = { path = "libgit2-sys", version = "0.17.0" } [target."cfg(all(unix, not(target_os = \"macos\")))".dependencies] diff --git a/src/lib.rs b/src/lib.rs index e52efb3734..84e2fad71e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -136,7 +136,7 @@ pub use crate::status::{StatusEntry, StatusIter, StatusOptions, StatusShow, Stat pub use crate::submodule::{Submodule, SubmoduleUpdateOptions}; pub use crate::tag::Tag; pub use crate::time::{IndexTime, Time}; -pub use crate::tracing::{trace_set, TraceLevel}; +pub use crate::tracing::{trace_set, trace_shim_log_crate, TraceLevel}; pub use crate::transaction::Transaction; pub use crate::tree::{Tree, TreeEntry, TreeIter, TreeWalkMode, TreeWalkResult}; pub use crate::treebuilder::TreeBuilder; diff --git a/src/tracing.rs b/src/tracing.rs index 5acae8a850..0407db8af8 100644 --- a/src/tracing.rs +++ b/src/tracing.rs @@ -1,6 +1,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use libc::c_char; +use log::RecordBuilder; use crate::{panic, raw, util::Binding}; @@ -8,53 +9,90 @@ use crate::{panic, raw, util::Binding}; /// callers will be provided tracing at the given level and all lower levels. #[derive(Copy, Clone, Debug)] pub enum TraceLevel { - /// No tracing will be performed. - None, + /// No tracing will be performed. + None, - /// Severe errors that may impact the program's execution - Fatal, + /// Severe errors that may impact the program's execution + Fatal, - /// Errors that do not impact the program's execution - Error, + /// Errors that do not impact the program's execution + Error, - /// Warnings that suggest abnormal data - Warn, + /// Warnings that suggest abnormal data + Warn, - /// Informational messages about program execution - Info, + /// Informational messages about program execution + Info, - /// Detailed data that allows for debugging - Debug, + /// Detailed data that allows for debugging + Debug, - /// Exceptionally detailed debugging data - Trace, + /// Exceptionally detailed debugging data + Trace, } impl Binding for TraceLevel { - type Raw = raw::git_trace_level_t; - unsafe fn from_raw(raw: raw::git_trace_level_t) -> Self { - match raw { - raw::GIT_TRACE_NONE => Self::None, - raw::GIT_TRACE_FATAL => Self::Fatal, - raw::GIT_TRACE_ERROR => Self::Error, - raw::GIT_TRACE_WARN => Self::Warn, - raw::GIT_TRACE_INFO => Self::Info, - raw::GIT_TRACE_DEBUG => Self::Debug, - raw::GIT_TRACE_TRACE => Self::Trace, - _ => panic!("Unknown git trace level"), - } - } - fn raw(&self) -> raw::git_trace_level_t { - match *self { - Self::None => raw::GIT_TRACE_NONE, - Self::Fatal => raw::GIT_TRACE_FATAL, - Self::Error => raw::GIT_TRACE_ERROR, - Self::Warn => raw::GIT_TRACE_WARN, - Self::Info => raw::GIT_TRACE_INFO, - Self::Debug => raw::GIT_TRACE_DEBUG, - Self::Trace => raw::GIT_TRACE_TRACE, - } - } + type Raw = raw::git_trace_level_t; + unsafe fn from_raw(raw: raw::git_trace_level_t) -> Self { + match raw { + raw::GIT_TRACE_NONE => Self::None, + raw::GIT_TRACE_FATAL => Self::Fatal, + raw::GIT_TRACE_ERROR => Self::Error, + raw::GIT_TRACE_WARN => Self::Warn, + raw::GIT_TRACE_INFO => Self::Info, + raw::GIT_TRACE_DEBUG => Self::Debug, + raw::GIT_TRACE_TRACE => Self::Trace, + _ => panic!("Unknown git trace level"), + } + } + fn raw(&self) -> raw::git_trace_level_t { + match *self { + Self::None => raw::GIT_TRACE_NONE, + Self::Fatal => raw::GIT_TRACE_FATAL, + Self::Error => raw::GIT_TRACE_ERROR, + Self::Warn => raw::GIT_TRACE_WARN, + Self::Info => raw::GIT_TRACE_INFO, + Self::Debug => raw::GIT_TRACE_DEBUG, + Self::Trace => raw::GIT_TRACE_TRACE, + } + } +} + +impl TraceLevel { + /// Convert the [TraceLevel] to a [log::LevelFilter]. + /// + /// [TraceLevel::Fatal] becomes [log::LevelFilter::Error] but otherwise all the conversions + /// are trivial. + pub const fn as_log_level_filter(self) -> log::LevelFilter { + use log::LevelFilter; + + match self { + Self::None => LevelFilter::Off, + Self::Fatal | Self::Error => LevelFilter::Error, + Self::Warn => LevelFilter::Warn, + Self::Info => LevelFilter::Info, + Self::Debug => LevelFilter::Debug, + Self::Trace => LevelFilter::Trace, + } + } + + /// Attempt to convert this [TraceLevel] to a [log::LevelFilter]. + /// + /// This is done trivially with two exceptions: + /// - [TraceLevel::None] goes to [None] + /// - [TraceLevel::Fatal] goes to [log::Level::Error]. + pub const fn as_log_level(self) -> Option { + use log::Level; + + match self { + Self::None => None, + Self::Fatal | Self::Error => Some(Level::Error), + Self::Warn => Some(Level::Warn), + Self::Info => Some(Level::Info), + Self::Debug => Some(Level::Debug), + Self::Trace => Some(Level::Trace), + } + } } //TODO: pass raw &[u8] and leave conversion to consumer (breaking API) @@ -64,22 +102,43 @@ pub type TracingCb = fn(TraceLevel, &str); static CALLBACK: AtomicUsize = AtomicUsize::new(0); -/// +/// Set the tracing callback. pub fn trace_set(level: TraceLevel, cb: TracingCb) -> bool { - CALLBACK.store(cb as usize, Ordering::SeqCst); + CALLBACK.store(cb as usize, Ordering::SeqCst); - unsafe { - raw::git_trace_set(level.raw(), Some(tracing_cb_c)); - } + unsafe { + raw::git_trace_set(level.raw(), Some(tracing_cb_c)); + } + + return true; +} - return true; +/// Passes [trace_set] a shim function to pass tracing info to the [log] crate. +pub fn trace_shim_log_crate() { + // Use `trace` to get all tracing events -- let the user configure filtering + // through the `log` crate. + trace_set(TraceLevel::Trace, |level, msg| { + // Convert the trace level to a log level. + let log_level = level + .as_log_level() + .expect("libgit2 should not produce tracing events with level=None"); + + // Build a record to pass to the logger. + let mut record_builder = RecordBuilder::new(); + + // Set the target and level. + record_builder.target("libgit2").level(log_level); + + // Log the trace event to the global logger. + log::logger().log(&record_builder.args(format_args!("{}", msg)).build()); + }); } extern "C" fn tracing_cb_c(level: raw::git_trace_level_t, msg: *const c_char) { - let cb = CALLBACK.load(Ordering::SeqCst); - panic::wrap(|| unsafe { - let cb: TracingCb = std::mem::transmute(cb); - let msg = std::ffi::CStr::from_ptr(msg).to_string_lossy(); - cb(Binding::from_raw(level), msg.as_ref()); - }); + let cb = CALLBACK.load(Ordering::SeqCst); + panic::wrap(|| unsafe { + let cb: TracingCb = std::mem::transmute(cb); + let msg = std::ffi::CStr::from_ptr(msg).to_string_lossy(); + cb(Binding::from_raw(level), msg.as_ref()); + }); } From d316b0e048ddff3c652f6e7b8ad47ad92ada1bc0 Mon Sep 17 00:00:00 2001 From: Venus Xeon-Blonde Date: Wed, 10 Jul 2024 14:08:27 -0400 Subject: [PATCH 2/7] Replace tabs with spaces --- src/tracing.rs | 216 ++++++++++++++++++++++++------------------------- 1 file changed, 108 insertions(+), 108 deletions(-) diff --git a/src/tracing.rs b/src/tracing.rs index 0407db8af8..78a273434f 100644 --- a/src/tracing.rs +++ b/src/tracing.rs @@ -9,90 +9,90 @@ use crate::{panic, raw, util::Binding}; /// callers will be provided tracing at the given level and all lower levels. #[derive(Copy, Clone, Debug)] pub enum TraceLevel { - /// No tracing will be performed. - None, - - /// Severe errors that may impact the program's execution - Fatal, - - /// Errors that do not impact the program's execution - Error, - - /// Warnings that suggest abnormal data - Warn, - - /// Informational messages about program execution - Info, - - /// Detailed data that allows for debugging - Debug, - - /// Exceptionally detailed debugging data - Trace, + /// No tracing will be performed. + None, + + /// Severe errors that may impact the program's execution + Fatal, + + /// Errors that do not impact the program's execution + Error, + + /// Warnings that suggest abnormal data + Warn, + + /// Informational messages about program execution + Info, + + /// Detailed data that allows for debugging + Debug, + + /// Exceptionally detailed debugging data + Trace, } impl Binding for TraceLevel { - type Raw = raw::git_trace_level_t; - unsafe fn from_raw(raw: raw::git_trace_level_t) -> Self { - match raw { - raw::GIT_TRACE_NONE => Self::None, - raw::GIT_TRACE_FATAL => Self::Fatal, - raw::GIT_TRACE_ERROR => Self::Error, - raw::GIT_TRACE_WARN => Self::Warn, - raw::GIT_TRACE_INFO => Self::Info, - raw::GIT_TRACE_DEBUG => Self::Debug, - raw::GIT_TRACE_TRACE => Self::Trace, - _ => panic!("Unknown git trace level"), - } - } - fn raw(&self) -> raw::git_trace_level_t { - match *self { - Self::None => raw::GIT_TRACE_NONE, - Self::Fatal => raw::GIT_TRACE_FATAL, - Self::Error => raw::GIT_TRACE_ERROR, - Self::Warn => raw::GIT_TRACE_WARN, - Self::Info => raw::GIT_TRACE_INFO, - Self::Debug => raw::GIT_TRACE_DEBUG, - Self::Trace => raw::GIT_TRACE_TRACE, - } - } + type Raw = raw::git_trace_level_t; + unsafe fn from_raw(raw: raw::git_trace_level_t) -> Self { + match raw { + raw::GIT_TRACE_NONE => Self::None, + raw::GIT_TRACE_FATAL => Self::Fatal, + raw::GIT_TRACE_ERROR => Self::Error, + raw::GIT_TRACE_WARN => Self::Warn, + raw::GIT_TRACE_INFO => Self::Info, + raw::GIT_TRACE_DEBUG => Self::Debug, + raw::GIT_TRACE_TRACE => Self::Trace, + _ => panic!("Unknown git trace level"), + } + } + fn raw(&self) -> raw::git_trace_level_t { + match *self { + Self::None => raw::GIT_TRACE_NONE, + Self::Fatal => raw::GIT_TRACE_FATAL, + Self::Error => raw::GIT_TRACE_ERROR, + Self::Warn => raw::GIT_TRACE_WARN, + Self::Info => raw::GIT_TRACE_INFO, + Self::Debug => raw::GIT_TRACE_DEBUG, + Self::Trace => raw::GIT_TRACE_TRACE, + } + } } impl TraceLevel { - /// Convert the [TraceLevel] to a [log::LevelFilter]. - /// - /// [TraceLevel::Fatal] becomes [log::LevelFilter::Error] but otherwise all the conversions - /// are trivial. - pub const fn as_log_level_filter(self) -> log::LevelFilter { - use log::LevelFilter; - - match self { - Self::None => LevelFilter::Off, - Self::Fatal | Self::Error => LevelFilter::Error, - Self::Warn => LevelFilter::Warn, - Self::Info => LevelFilter::Info, - Self::Debug => LevelFilter::Debug, - Self::Trace => LevelFilter::Trace, - } - } - - /// Attempt to convert this [TraceLevel] to a [log::LevelFilter]. - /// - /// This is done trivially with two exceptions: - /// - [TraceLevel::None] goes to [None] - /// - [TraceLevel::Fatal] goes to [log::Level::Error]. - pub const fn as_log_level(self) -> Option { - use log::Level; - - match self { - Self::None => None, - Self::Fatal | Self::Error => Some(Level::Error), - Self::Warn => Some(Level::Warn), - Self::Info => Some(Level::Info), - Self::Debug => Some(Level::Debug), - Self::Trace => Some(Level::Trace), - } - } + /// Convert the [TraceLevel] to a [log::LevelFilter]. + /// + /// [TraceLevel::Fatal] becomes [log::LevelFilter::Error] but otherwise all the conversions + /// are trivial. + pub const fn as_log_level_filter(self) -> log::LevelFilter { + use log::LevelFilter; + + match self { + Self::None => LevelFilter::Off, + Self::Fatal | Self::Error => LevelFilter::Error, + Self::Warn => LevelFilter::Warn, + Self::Info => LevelFilter::Info, + Self::Debug => LevelFilter::Debug, + Self::Trace => LevelFilter::Trace, + } + } + + /// Attempt to convert this [TraceLevel] to a [log::LevelFilter]. + /// + /// This is done trivially with two exceptions: + /// - [TraceLevel::None] goes to [None] + /// - [TraceLevel::Fatal] goes to [log::Level::Error]. + pub const fn as_log_level(self) -> Option { + use log::Level; + + match self { + Self::None => None, + Self::Fatal | Self::Error => Some(Level::Error), + Self::Warn => Some(Level::Warn), + Self::Info => Some(Level::Info), + Self::Debug => Some(Level::Debug), + Self::Trace => Some(Level::Trace), + } + } } //TODO: pass raw &[u8] and leave conversion to consumer (breaking API) @@ -104,41 +104,41 @@ static CALLBACK: AtomicUsize = AtomicUsize::new(0); /// Set the tracing callback. pub fn trace_set(level: TraceLevel, cb: TracingCb) -> bool { - CALLBACK.store(cb as usize, Ordering::SeqCst); - - unsafe { - raw::git_trace_set(level.raw(), Some(tracing_cb_c)); - } - - return true; + CALLBACK.store(cb as usize, Ordering::SeqCst); + + unsafe { + raw::git_trace_set(level.raw(), Some(tracing_cb_c)); + } + + return true; } /// Passes [trace_set] a shim function to pass tracing info to the [log] crate. pub fn trace_shim_log_crate() { - // Use `trace` to get all tracing events -- let the user configure filtering - // through the `log` crate. - trace_set(TraceLevel::Trace, |level, msg| { - // Convert the trace level to a log level. - let log_level = level - .as_log_level() - .expect("libgit2 should not produce tracing events with level=None"); - - // Build a record to pass to the logger. - let mut record_builder = RecordBuilder::new(); - - // Set the target and level. - record_builder.target("libgit2").level(log_level); - - // Log the trace event to the global logger. - log::logger().log(&record_builder.args(format_args!("{}", msg)).build()); - }); + // Use `trace` to get all tracing events -- let the user configure filtering + // through the `log` crate. + trace_set(TraceLevel::Trace, |level, msg| { + // Convert the trace level to a log level. + let log_level = level + .as_log_level() + .expect("libgit2 should not produce tracing events with level=None"); + + // Build a record to pass to the logger. + let mut record_builder = RecordBuilder::new(); + + // Set the target and level. + record_builder.target("libgit2").level(log_level); + + // Log the trace event to the global logger. + log::logger().log(&record_builder.args(format_args!("{}", msg)).build()); + }); } extern "C" fn tracing_cb_c(level: raw::git_trace_level_t, msg: *const c_char) { - let cb = CALLBACK.load(Ordering::SeqCst); - panic::wrap(|| unsafe { - let cb: TracingCb = std::mem::transmute(cb); - let msg = std::ffi::CStr::from_ptr(msg).to_string_lossy(); - cb(Binding::from_raw(level), msg.as_ref()); - }); + let cb = CALLBACK.load(Ordering::SeqCst); + panic::wrap(|| unsafe { + let cb: TracingCb = std::mem::transmute(cb); + let msg = std::ffi::CStr::from_ptr(msg).to_string_lossy(); + cb(Binding::from_raw(level), msg.as_ref()); + }); } From 4db09e4171911c628dd9b0391f3d7b63e245e521 Mon Sep 17 00:00:00 2001 From: Venus Xeon-Blonde Date: Wed, 10 Jul 2024 14:10:07 -0400 Subject: [PATCH 3/7] Remove trailing whitespace --- src/tracing.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/tracing.rs b/src/tracing.rs index 78a273434f..fc217358cf 100644 --- a/src/tracing.rs +++ b/src/tracing.rs @@ -11,22 +11,22 @@ use crate::{panic, raw, util::Binding}; pub enum TraceLevel { /// No tracing will be performed. None, - + /// Severe errors that may impact the program's execution Fatal, - + /// Errors that do not impact the program's execution Error, - + /// Warnings that suggest abnormal data Warn, - + /// Informational messages about program execution Info, - + /// Detailed data that allows for debugging Debug, - + /// Exceptionally detailed debugging data Trace, } @@ -65,7 +65,7 @@ impl TraceLevel { /// are trivial. pub const fn as_log_level_filter(self) -> log::LevelFilter { use log::LevelFilter; - + match self { Self::None => LevelFilter::Off, Self::Fatal | Self::Error => LevelFilter::Error, @@ -75,7 +75,7 @@ impl TraceLevel { Self::Trace => LevelFilter::Trace, } } - + /// Attempt to convert this [TraceLevel] to a [log::LevelFilter]. /// /// This is done trivially with two exceptions: @@ -83,7 +83,7 @@ impl TraceLevel { /// - [TraceLevel::Fatal] goes to [log::Level::Error]. pub const fn as_log_level(self) -> Option { use log::Level; - + match self { Self::None => None, Self::Fatal | Self::Error => Some(Level::Error), @@ -105,11 +105,11 @@ static CALLBACK: AtomicUsize = AtomicUsize::new(0); /// Set the tracing callback. pub fn trace_set(level: TraceLevel, cb: TracingCb) -> bool { CALLBACK.store(cb as usize, Ordering::SeqCst); - + unsafe { raw::git_trace_set(level.raw(), Some(tracing_cb_c)); } - + return true; } @@ -122,13 +122,13 @@ pub fn trace_shim_log_crate() { let log_level = level .as_log_level() .expect("libgit2 should not produce tracing events with level=None"); - + // Build a record to pass to the logger. let mut record_builder = RecordBuilder::new(); - + // Set the target and level. record_builder.target("libgit2").level(log_level); - + // Log the trace event to the global logger. log::logger().log(&record_builder.args(format_args!("{}", msg)).build()); }); From 9e7bae0b31145e4800719d63b4ddc72ade3d2bca Mon Sep 17 00:00:00 2001 From: Venus Xeon-Blonde Date: Wed, 10 Jul 2024 14:18:14 -0400 Subject: [PATCH 4/7] fix formatting --- src/tracing.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tracing.rs b/src/tracing.rs index fc217358cf..8896176dde 100644 --- a/src/tracing.rs +++ b/src/tracing.rs @@ -120,8 +120,8 @@ pub fn trace_shim_log_crate() { trace_set(TraceLevel::Trace, |level, msg| { // Convert the trace level to a log level. let log_level = level - .as_log_level() - .expect("libgit2 should not produce tracing events with level=None"); + .as_log_level() + .expect("libgit2 should not produce tracing events with level=None"); // Build a record to pass to the logger. let mut record_builder = RecordBuilder::new(); From 396c5d2350ef0c877f1d7393b213143dd55132ae Mon Sep 17 00:00:00 2001 From: Venus Xeon-Blonde Date: Mon, 22 Jul 2024 13:44:01 -0400 Subject: [PATCH 5/7] Remove function converting `TraceLevel` to `LogLevelFilter`. --- src/tracing.rs | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/tracing.rs b/src/tracing.rs index 8896176dde..aab1885aaa 100644 --- a/src/tracing.rs +++ b/src/tracing.rs @@ -59,23 +59,6 @@ impl Binding for TraceLevel { } impl TraceLevel { - /// Convert the [TraceLevel] to a [log::LevelFilter]. - /// - /// [TraceLevel::Fatal] becomes [log::LevelFilter::Error] but otherwise all the conversions - /// are trivial. - pub const fn as_log_level_filter(self) -> log::LevelFilter { - use log::LevelFilter; - - match self { - Self::None => LevelFilter::Off, - Self::Fatal | Self::Error => LevelFilter::Error, - Self::Warn => LevelFilter::Warn, - Self::Info => LevelFilter::Info, - Self::Debug => LevelFilter::Debug, - Self::Trace => LevelFilter::Trace, - } - } - /// Attempt to convert this [TraceLevel] to a [log::LevelFilter]. /// /// This is done trivially with two exceptions: From 5521816072305ad346ef5f385aeeb20ca9c4e9e0 Mon Sep 17 00:00:00 2001 From: Venus Xeon-Blonde Date: Mon, 22 Jul 2024 14:20:32 -0400 Subject: [PATCH 6/7] Do not use `log` types in public functions. --- src/tracing.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tracing.rs b/src/tracing.rs index aab1885aaa..2963450f72 100644 --- a/src/tracing.rs +++ b/src/tracing.rs @@ -60,11 +60,13 @@ impl Binding for TraceLevel { impl TraceLevel { /// Attempt to convert this [TraceLevel] to a [log::LevelFilter]. + /// + /// This function is not public to avoid having a public dependency on [`log`]. /// /// This is done trivially with two exceptions: /// - [TraceLevel::None] goes to [None] /// - [TraceLevel::Fatal] goes to [log::Level::Error]. - pub const fn as_log_level(self) -> Option { + const fn as_log_level(self) -> Option { use log::Level; match self { From 8d87dd5bba009ed48cba96cd86b593b9bb02c937 Mon Sep 17 00:00:00 2001 From: Venus Xeon-Blonde Date: Mon, 22 Jul 2024 15:44:26 -0400 Subject: [PATCH 7/7] cargo fmt --- src/tracing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tracing.rs b/src/tracing.rs index 2963450f72..9baeaa83d9 100644 --- a/src/tracing.rs +++ b/src/tracing.rs @@ -60,7 +60,7 @@ impl Binding for TraceLevel { impl TraceLevel { /// Attempt to convert this [TraceLevel] to a [log::LevelFilter]. - /// + /// /// This function is not public to avoid having a public dependency on [`log`]. /// /// This is done trivially with two exceptions: