Skip to content

Commit

Permalink
Improve logging in dynamic libraries
Browse files Browse the repository at this point in the history
  • Loading branch information
thorio committed Jul 1, 2024
1 parent f786ebe commit e8bd860
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 27 deletions.
3 changes: 1 addition & 2 deletions gravel-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ pub use frontend::{
FrontendExitStatusNe, FrontendMessage, FrontendMessageNe, QueryResult,
};
pub use hit::{ArcDynHit, BoxDynHitActionContext, Hit, HitActionContext, ScoredHit, SimpleHit};
use logging::BoxDynLogTarget;
pub use plugin::{PluginDefinition, PluginMetadata};
pub use provider::{BoxDynProvider, Provider, ProviderDef, ProviderResult};

Expand All @@ -47,7 +46,7 @@ pub const MIN_SCORE: u32 = u32::MIN;
#[sabi(kind(Prefix(prefix_ref = PluginLibRef)))]
pub struct PluginLib {
#[sabi(last_prefix_field)]
pub plugin: extern "C" fn(log_target: BoxDynLogTarget) -> PluginDefinition,
pub plugin: extern "C" fn(log_target: logging::BoxDynLogTarget) -> PluginDefinition,
}

// TODO: write a test to check for compatibility with older plugins
Expand Down
25 changes: 6 additions & 19 deletions gravel-ffi/src/logging.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
// This warns that abi_stable::marker_type::NonOwningPhantom
// is not FFI-safe, which I can only assume is a false positive
#![allow(improper_ctypes_definitions)]

use abi_stable::{sabi_trait, std_types::RBox};
use log::{Log, Metadata, Record};
use log_types::{RLevelFilter, RMetadata, RRecord};
Expand Down Expand Up @@ -67,13 +63,11 @@ impl Log for ForwardLogger {
}
}

// TODO: check if this is actually allocation free

/// Contains FFI-safe wrappers and associated conversions for `log` types.
mod log_types {
use abi_stable::std_types::{ROption, RStr};
use abi_stable::std_types::{ROption, RStr, RString};
use abi_stable::traits::{IntoReprC, IntoReprRust};
use abi_stable::{DynTrait, RRef, StableAbi};
use abi_stable::StableAbi;
use log::{Level, LevelFilter, Log, Metadata, Record, RecordBuilder};
use std::fmt::Arguments;

Expand Down Expand Up @@ -176,21 +170,14 @@ mod log_types {
}
}

// we only need Display and specifically don't want Sync/Send
#[repr(C)]
#[derive(StableAbi)]
#[sabi(impl_InterfaceType(Display))]
struct DisplayInterface;

/// FFI-safe wrapper around `std::fmt::Arguments`
type DynArguments<'a> = DynTrait<'a, RRef<'a, ()>, DisplayInterface>;

/// FFI-safe representation of `log::Record`
#[repr(C)]
#[derive(StableAbi)]
pub struct RRecord<'a> {
// you can do this without allocations using DynTrait,
// but I found it's a few hundred nanoseconds slower overall
args: RString,
metadata: RMetadata<'a>,
args: DynArguments<'a>,
module_path: ROption<RStr<'a>>,
file: ROption<RStr<'a>>,
line: ROption<u32>,
Expand All @@ -199,8 +186,8 @@ mod log_types {
impl<'a> From<&'a Record<'a>> for RRecord<'a> {
fn from(value: &'a Record<'a>) -> Self {
Self {
args: value.args().to_string().into_c(),
metadata: value.metadata().into(),
args: DynArguments::from_borrowing_ptr(value.args()),
module_path: value.module_path().map(IntoReprC::into_c).into_c(),
file: value.file().map(IntoReprC::into_c).into_c(),
line: value.line().into_c(),
Expand Down
4 changes: 2 additions & 2 deletions gravel/src/init/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use anyhow::Result;
use chrono::Local;
use fern::{Dispatch, FormatCallback};
use file_rotate::{compression::Compression, suffix::AppendCount, ContentLimit, FileRotate};
use gravel_core::paths::log_path;
use gravel_core::paths;
use log::{LevelFilter, Log, Record};
use std::fmt::Arguments;
use std::io::Write;
Expand All @@ -16,7 +16,7 @@ pub fn logging(args: LogArgs) -> Result<()> {
dispatch = chain_stderr(dispatch);
}

let log_path = &args.log_file.unwrap_or_else(log_path);
let log_path = &args.log_file.unwrap_or_else(paths::log_path);
if log_path.to_str() != Some("off") {
dispatch = chain_file(dispatch, log_path);
}
Expand Down
6 changes: 2 additions & 4 deletions gravel/src/init/plugins.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use abi_stable::{
library::{lib_header_from_path, LibHeader, LibraryError, RootModule},
sabi_types::VersionNumber,
};
use abi_stable::library::{lib_header_from_path, LibHeader, LibraryError, RootModule};
use abi_stable::sabi_types::VersionNumber;
use glob::{glob, Paths};
use gravel_core::{paths, plugin::PluginRegistry};
use gravel_ffi::{logging::StaticLogTarget, PluginLibRef};
Expand Down

0 comments on commit e8bd860

Please sign in to comment.