-
Notifications
You must be signed in to change notification settings - Fork 394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update log
crate and create shim method(s) between git2
's tracing and log
#1066
Changes from 4 commits
b4daf13
d316b0e
4db09e4
9e7bae0
396c5d2
5521816
8d87dd5
eba4cbd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
use std::sync::atomic::{AtomicUsize, Ordering}; | ||
|
||
use libc::c_char; | ||
use log::RecordBuilder; | ||
|
||
use crate::{panic, raw, util::Binding}; | ||
|
||
|
@@ -57,14 +58,51 @@ 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: | ||
/// - [TraceLevel::None] goes to [None] | ||
/// - [TraceLevel::Fatal] goes to [log::Level::Error]. | ||
pub const fn as_log_level(self) -> Option<log::Level> { | ||
use log::Level; | ||
|
||
match self { | ||
Self::None => None, | ||
Self::Fatal | Self::Error => Some(Level::Error), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has the same symptom. Different users may have different thoughts on mapping Fatal and Error to different levels. We try not to decide this kind of application logic for users. |
||
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) | ||
/// Callback type used to pass tracing events to the subscriber. | ||
/// see `trace_set` to register a subscriber. | ||
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); | ||
|
||
|
@@ -75,6 +113,27 @@ pub fn trace_set(level: TraceLevel, cb: TracingCb) -> bool { | |
return true; | ||
} | ||
|
||
/// Passes [trace_set] a shim function to pass tracing info to the [log] crate. | ||
pub fn trace_shim_log_crate() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
// 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 { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the use case for this function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is theoretically useful for users writing a trace callback that needs to compare the log level given by
git2
with theLevelFilter
they're using with thelog
crate, however you're right that it's not enough utility to make thelog
dependency public, so I'll go ahead and remove it.