Skip to content

Commit

Permalink
Add default features for time and compression
Browse files Browse the repository at this point in the history
This makes the `chrono` and `flate2` dependencies optional, adding a `time` and `compression` feature for each. These features are enabled by default, but can be disabled by using `default-features = false` in the Cargo.toml dependency definition, allowing library users to opt out of these dependencies if they don't need them. This should reduce compile times and binary size. In my testing on a release build I saved 1.3 seconds of compilation time (although this is very CPU dependent) and 66 KiB of binary size by dropping these features from a project that depends on file-rotate.

`cfg` attributes have been added to tests to support testing with features disabled, and I've added additional test runs in the rust.yml GitHub Action to cover these cases. If you want me to add additional cargo check runs for these cases let me know and I'll do that as well. The example and doc tests only work with the default features enabled, but I think that's fine.

I have not bumped the library version in Cargo.toml as I believe that should be left to the maintainer. However, note that a major version bump is needed (e.g. 0.7.6 -> 0.8.0) as this change is breaking for anyone already using `default-features = false` (even though there are currently no features someone may still be doing this for some reason). I've seen other projects get hit by this, and affected library users typically call for a cargo yank.
  • Loading branch information
zkxs committed Jun 29, 2024
1 parent fc60342 commit cfd9d7b
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 67 deletions.
8 changes: 7 additions & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,11 @@ jobs:
- os: windows-latest
steps:
- uses: actions/checkout@v3
- name: Test
- name: Test (default features)
run: cargo test
- name: Test (no default features)
run: cargo test --tests --no-default-features
- name: Test (compression feature only)
run: cargo test --tests --no-default-features --features compression
- name: Test (time feature only)
run: cargo test --tests --no-default-features --features time
9 changes: 7 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@ repository = "https://github.com/kstrafe/file-rotate"
keywords= ["log", "rotate", "logrotate"]
license = "MIT"

[features]
default = ["time", "compression"]
time = ["dep:chrono"]
compression = ["dep:flate2"]

[dependencies]
chrono = { version = "0.4.20", default-features = false, features = ["clock"] }
flate2 = "1.0"
chrono = { version = "0.4.20", default-features = false, features = ["clock"], optional = true }
flate2 = { version = "1.0", optional = true }

[dev-dependencies]
filetime = "0.2"
Expand Down
36 changes: 21 additions & 15 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,9 @@
unused_qualifications
)]

#[cfg(feature = "time")]
use chrono::prelude::*;
#[cfg(feature = "compression")]
use compression::*;
use std::io::{BufRead, BufReader};
use std::{
Expand All @@ -306,6 +308,7 @@ use suffix::*;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;

#[cfg(feature = "compression")]
pub mod compression;
pub mod suffix;
#[cfg(test)]
Expand All @@ -315,6 +318,7 @@ mod tests;

/// At which frequency to rotate the file.
#[derive(Clone, Copy, Debug)]
#[cfg(feature = "time")]
pub enum TimeFrequency {
/// Rotate every hour.
Hourly,
Expand All @@ -336,6 +340,7 @@ pub enum ContentLimit {
/// Cut the log file at line breaks.
Lines(usize),
/// Cut the log at time interval.
#[cfg(feature = "time")]
Time(TimeFrequency),
/// Cut the log file after surpassing size in bytes (but having written a complete buffer from a write call.)
BytesSurpassed(usize),
Expand Down Expand Up @@ -385,15 +390,14 @@ impl<Repr: Representation> PartialOrd for SuffixInfo<Repr> {
pub struct FileRotate<S: SuffixScheme> {
basepath: PathBuf,
file: Option<File>,
modified: Option<DateTime<Local>>,
#[cfg(feature = "time")] modified: Option<DateTime<Local>>,
content_limit: ContentLimit,
count: usize,
compression: Compression,
#[cfg(feature = "compression")] compression: Compression,
suffix_scheme: S,
/// The bool is whether or not there's a .gz suffix to the filename
suffixes: BTreeSet<SuffixInfo<S::Repr>>,
#[cfg(unix)]
mode: Option<u32>,
#[cfg(unix)] mode: Option<u32>,
}

impl<S: SuffixScheme> FileRotate<S> {
Expand All @@ -411,7 +415,7 @@ impl<S: SuffixScheme> FileRotate<S> {
path: P,
suffix_scheme: S,
content_limit: ContentLimit,
compression: Compression,
#[cfg(feature = "compression")] compression: Compression,
#[cfg(unix)] mode: Option<u32>,
) -> Self {
match content_limit {
Expand All @@ -421,7 +425,7 @@ impl<S: SuffixScheme> FileRotate<S> {
ContentLimit::Lines(lines) => {
assert!(lines > 0);
}
ContentLimit::Time(_) => {}
#[cfg(feature = "time")] ContentLimit::Time(_) => {}
ContentLimit::BytesSurpassed(bytes) => {
assert!(bytes > 0);
}
Expand All @@ -433,15 +437,14 @@ impl<S: SuffixScheme> FileRotate<S> {

let mut s = Self {
file: None,
modified: None,
#[cfg(feature = "time")] modified: None,
basepath,
content_limit,
count: 0,
compression,
#[cfg(feature = "compression")] compression,
suffixes: BTreeSet::new(),
suffix_scheme,
#[cfg(unix)]
mode,
#[cfg(unix)] mode,
};
s.ensure_log_directory_exists();
s.scan_suffixes();
Expand Down Expand Up @@ -473,6 +476,7 @@ impl<S: SuffixScheme> FileRotate<S> {
ContentLimit::Lines(_) => {
self.count = BufReader::new(file).lines().count();
}
#[cfg(feature = "time")]
ContentLimit::Time(_) => {
self.modified = mtime(file);
}
Expand Down Expand Up @@ -607,6 +611,7 @@ impl<S: SuffixScheme> FileRotate<S> {
}

// Compression
#[cfg(feature = "compression")]
if let Compression::OnRotate(max_file_n) = self.compression {
let n = (self.suffixes.len() as i32 - max_file_n as i32).max(0) as usize;
// The oldest N files should be compressed
Expand Down Expand Up @@ -652,6 +657,7 @@ impl<S: SuffixScheme> Write for FileRotate<S> {
file.write_all(buf)?;
}
}
#[cfg(feature = "time")]
ContentLimit::Time(time) => {
let local: DateTime<Local> = now();

Expand Down Expand Up @@ -741,7 +747,7 @@ impl<S: SuffixScheme> Write for FileRotate<S> {
}

/// Get modification time, in non test case.
#[cfg(not(test))]
#[cfg(all(not(test), feature = "time"))]
fn mtime(file: &File) -> Option<DateTime<Local>> {
if let Ok(time) = file.metadata().and_then(|metadata| metadata.modified()) {
return Some(time.into());
Expand All @@ -751,19 +757,19 @@ fn mtime(file: &File) -> Option<DateTime<Local>> {
}

/// Get modification time, in test case.
#[cfg(test)]
#[cfg(all(test, feature = "time"))]
fn mtime(_: &File) -> Option<DateTime<Local>> {
Some(now())
}

/// Get system time, in non test case.
#[cfg(not(test))]
#[cfg(all(not(test), feature = "time"))]
fn now() -> DateTime<Local> {
Local::now()
}

/// Get mocked system time, in test case.
#[cfg(test)]
#[cfg(all(test, feature = "time"))]
pub mod mock_time {
use super::*;
use std::cell::RefCell;
Expand All @@ -781,5 +787,5 @@ pub mod mock_time {
}
}

#[cfg(test)]
#[cfg(all(test, feature = "time"))]
pub use mock_time::now;
17 changes: 15 additions & 2 deletions src/suffix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
//! This behaviour is fully extensible through the [SuffixScheme] trait, and two behaviours are
//! provided: [AppendCount] and [AppendTimestamp]
//!
#[cfg(feature = "time")]
use super::now;
use crate::SuffixInfo;
#[cfg(feature = "time")]
use chrono::{format::ParseErrorKind, offset::Local, Duration, NaiveDateTime};
#[cfg(feature = "time")]
use std::cmp::Ordering;
use std::{
cmp::Ordering,
collections::BTreeSet,
io,
path::{Path, PathBuf},
Expand Down Expand Up @@ -141,6 +144,7 @@ impl SuffixScheme for AppendCount {
}

/// Add timestamp from:
#[cfg(feature = "time")]
pub enum DateFrom {
/// Date yesterday, to represent the timestamps within the log file.
DateYesterday,
Expand All @@ -156,6 +160,7 @@ pub enum DateFrom {
/// Current limitations:
/// - Neither `format` nor the base filename can include the character `"."`.
/// - The `format` should ensure that the lexical and chronological orderings are the same
#[cfg(feature = "time")]
pub struct AppendTimestamp {
/// The format of the timestamp suffix
pub format: &'static str,
Expand All @@ -165,6 +170,7 @@ pub struct AppendTimestamp {
pub date_from: DateFrom,
}

#[cfg(feature = "time")]
impl AppendTimestamp {
/// With format `"%Y%m%dT%H%M%S"`
pub fn default(file_limit: FileLimit) -> Self {
Expand All @@ -186,13 +192,16 @@ impl AppendTimestamp {

/// Structured representation of the suffixes of AppendTimestamp.
#[derive(Debug, Clone, PartialEq, Eq)]
#[cfg(feature = "time")]
pub struct TimestampSuffix {
/// The timestamp
pub timestamp: String,
/// Optional number suffix if two timestamp suffixes are the same
pub number: Option<usize>,
}
#[cfg(feature = "time")]
impl Representation for TimestampSuffix {}
#[cfg(feature = "time")]
impl Ord for TimestampSuffix {
fn cmp(&self, other: &Self) -> Ordering {
// Most recent = smallest (opposite as the timestamp Ord)
Expand All @@ -203,11 +212,13 @@ impl Ord for TimestampSuffix {
}
}
}
#[cfg(feature = "time")]
impl PartialOrd for TimestampSuffix {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}
#[cfg(feature = "time")]
impl std::fmt::Display for TimestampSuffix {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
match self.number {
Expand All @@ -217,6 +228,7 @@ impl std::fmt::Display for TimestampSuffix {
}
}

#[cfg(feature = "time")]
impl SuffixScheme for AppendTimestamp {
type Repr = TimestampSuffix;

Expand Down Expand Up @@ -304,12 +316,13 @@ pub enum FileLimit {
/// Delete the oldest files if number of files is too high
MaxFiles(usize),
/// Delete files whose age exceeds the `Duration` - age is determined by the suffix of the file
#[cfg(feature = "time")]
Age(Duration),
/// Never delete files
Unlimited,
}

#[cfg(test)]
#[cfg(all(test, feature = "time"))]
mod test {
use super::*;
use std::fs::File;
Expand Down
Loading

0 comments on commit cfd9d7b

Please sign in to comment.