From cfd9d7bf95a621a395a71bef94c7215c0125e600 Mon Sep 17 00:00:00 2001 From: Michael Ripley Date: Sat, 29 Jun 2024 13:34:53 -0500 Subject: [PATCH 1/2] Add default features for time and compression 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. --- .github/workflows/rust.yml | 8 +++- Cargo.toml | 9 +++- src/lib.rs | 36 +++++++++------- src/suffix.rs | 17 +++++++- src/tests.rs | 85 +++++++++++++++++--------------------- 5 files changed, 88 insertions(+), 67 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 4882eb2..381a87e 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -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 diff --git a/Cargo.toml b/Cargo.toml index ec4fb12..b73e659 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" diff --git a/src/lib.rs b/src/lib.rs index 898ee34..f43abb3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -291,7 +291,9 @@ unused_qualifications )] +#[cfg(feature = "time")] use chrono::prelude::*; +#[cfg(feature = "compression")] use compression::*; use std::io::{BufRead, BufReader}; use std::{ @@ -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)] @@ -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, @@ -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), @@ -385,15 +390,14 @@ impl PartialOrd for SuffixInfo { pub struct FileRotate { basepath: PathBuf, file: Option, - modified: Option>, + #[cfg(feature = "time")] modified: Option>, 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>, - #[cfg(unix)] - mode: Option, + #[cfg(unix)] mode: Option, } impl FileRotate { @@ -411,7 +415,7 @@ impl FileRotate { path: P, suffix_scheme: S, content_limit: ContentLimit, - compression: Compression, + #[cfg(feature = "compression")] compression: Compression, #[cfg(unix)] mode: Option, ) -> Self { match content_limit { @@ -421,7 +425,7 @@ impl FileRotate { ContentLimit::Lines(lines) => { assert!(lines > 0); } - ContentLimit::Time(_) => {} + #[cfg(feature = "time")] ContentLimit::Time(_) => {} ContentLimit::BytesSurpassed(bytes) => { assert!(bytes > 0); } @@ -433,15 +437,14 @@ impl FileRotate { 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(); @@ -473,6 +476,7 @@ impl FileRotate { ContentLimit::Lines(_) => { self.count = BufReader::new(file).lines().count(); } + #[cfg(feature = "time")] ContentLimit::Time(_) => { self.modified = mtime(file); } @@ -607,6 +611,7 @@ impl FileRotate { } // 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 @@ -652,6 +657,7 @@ impl Write for FileRotate { file.write_all(buf)?; } } + #[cfg(feature = "time")] ContentLimit::Time(time) => { let local: DateTime = now(); @@ -741,7 +747,7 @@ impl Write for FileRotate { } /// Get modification time, in non test case. -#[cfg(not(test))] +#[cfg(all(not(test), feature = "time"))] fn mtime(file: &File) -> Option> { if let Ok(time) = file.metadata().and_then(|metadata| metadata.modified()) { return Some(time.into()); @@ -751,19 +757,19 @@ fn mtime(file: &File) -> Option> { } /// Get modification time, in test case. -#[cfg(test)] +#[cfg(all(test, feature = "time"))] fn mtime(_: &File) -> Option> { Some(now()) } /// Get system time, in non test case. -#[cfg(not(test))] +#[cfg(all(not(test), feature = "time"))] fn now() -> DateTime { 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; @@ -781,5 +787,5 @@ pub mod mock_time { } } -#[cfg(test)] +#[cfg(all(test, feature = "time"))] pub use mock_time::now; diff --git a/src/suffix.rs b/src/suffix.rs index b7279f3..d579507 100644 --- a/src/suffix.rs +++ b/src/suffix.rs @@ -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}, @@ -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, @@ -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, @@ -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 { @@ -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, } +#[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) @@ -203,11 +212,13 @@ impl Ord for TimestampSuffix { } } } +#[cfg(feature = "time")] impl PartialOrd for TimestampSuffix { fn partial_cmp(&self, other: &Self) -> Option { 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 { @@ -217,6 +228,7 @@ impl std::fmt::Display for TimestampSuffix { } } +#[cfg(feature = "time")] impl SuffixScheme for AppendTimestamp { type Repr = TimestampSuffix; @@ -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; diff --git a/src/tests.rs b/src/tests.rs index 4e545d7..cf9b069 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -19,6 +19,7 @@ fn list(dir: &Path) { } #[test] +#[cfg(feature = "time")] fn timestamp_max_files_rotation() { let tmp_dir = TempDir::new().unwrap(); let log_path = tmp_dir.path().join("log"); @@ -27,9 +28,8 @@ fn timestamp_max_files_rotation() { &log_path, AppendTimestamp::default(FileLimit::MaxFiles(4)), ContentLimit::Lines(2), - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); // Write 9 lines @@ -113,9 +113,8 @@ fn count_max_files_rotation() { &*log_path.to_string_lossy(), AppendCount::new(4), ContentLimit::Lines(2), - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); // Write 9 lines @@ -155,9 +154,8 @@ fn rotate_to_deleted_directory() { &*log_path.to_string_lossy(), AppendCount::new(4), ContentLimit::Lines(1), - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); write!(log, "a\nb\n").unwrap(); @@ -184,11 +182,10 @@ fn write_complete_record_until_bytes_surpassed() { let mut log = FileRotate::new( &log_path, - AppendTimestamp::default(FileLimit::MaxFiles(100)), + AppendCount::new(100), ContentLimit::BytesSurpassed(1), - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); write!(log, "0123456789").unwrap(); @@ -204,6 +201,7 @@ fn write_complete_record_until_bytes_surpassed() { } #[test] +#[cfg(feature = "compression")] fn compression_on_rotation() { let tmp_dir = TempDir::new().unwrap(); let parent = tmp_dir.path(); @@ -213,8 +211,7 @@ fn compression_on_rotation() { AppendCount::new(3), ContentLimit::Lines(1), Compression::OnRotate(1), // Keep one file uncompressed - #[cfg(unix)] - None, + #[cfg(unix)] None, ); writeln!(log, "A").unwrap(); @@ -257,9 +254,8 @@ fn no_truncate() { &*log_path.to_string_lossy(), AppendCount::new(3), ContentLimit::Lines(10000), - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ) }; writeln!(file_rotate(), "A").unwrap(); @@ -284,9 +280,8 @@ fn byte_count_recalculation() { &*log_path.to_string_lossy(), AppendCount::new(3), ContentLimit::Bytes(2), - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); write!(file_rotate, "bc").unwrap(); @@ -313,9 +308,8 @@ fn line_count_recalculation() { &*log_path.to_string_lossy(), AppendCount::new(3), ContentLimit::Lines(2), - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); // A single line existed before the new logger ('a') @@ -381,9 +375,8 @@ fn manual_rotation() { &*log_path.to_string_lossy(), AppendCount::new(3), ContentLimit::None, - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); writeln!(log, "A").unwrap(); log.rotate().unwrap(); @@ -407,11 +400,10 @@ fn arbitrary_lines(count: usize) { let count = count.max(1); let mut log = FileRotate::new( &log_path, - AppendTimestamp::default(FileLimit::MaxFiles(100)), + AppendCount::new(100), ContentLimit::Lines(count), - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); for _ in 0..count - 1 { @@ -433,11 +425,10 @@ fn arbitrary_bytes(count: usize) { let count = count.max(1); let mut log = FileRotate::new( &log_path, - AppendTimestamp::default(FileLimit::MaxFiles(100)), + AppendCount::new(100), ContentLimit::Bytes(count), - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); for _ in 0..count { @@ -451,6 +442,7 @@ fn arbitrary_bytes(count: usize) { } #[test] +#[cfg(feature = "time")] fn rotate_by_time_frequency() { // Test time frequency by hours. test_time_frequency( @@ -504,6 +496,7 @@ fn rotate_by_time_frequency() { } #[test] +#[cfg(feature = "time")] fn test_file_limit() { let tmp_dir = TempDir::new().unwrap(); let dir = tmp_dir.path(); @@ -520,9 +513,8 @@ fn test_file_limit() { log_path, AppendTimestamp::with_format("%Y-%m-%d", FileLimit::MaxFiles(1), DateFrom::DateYesterday), ContentLimit::Time(TimeFrequency::Daily), - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); mock_time::set_mock_time(first); @@ -549,9 +541,8 @@ fn test_panic() { &log_path, AppendCount::new(2), ContentLimit::None, - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); write!(log, "nineteen characters").unwrap(); @@ -562,9 +553,8 @@ fn test_panic() { &log_path, AppendCount::new(2), ContentLimit::Bytes(8), - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); write!(log, "0123").unwrap(); @@ -577,12 +567,14 @@ fn test_panic() { assert_eq!("0123", fs::read_to_string(&log_path).unwrap()); } +#[cfg(feature = "time")] fn get_fake_date_time(date_time: &str) -> DateTime { let date_obj = NaiveDateTime::parse_from_str(date_time, "%Y-%m-%dT%H:%M:%S"); Local.from_local_datetime(&date_obj.unwrap()).unwrap() } +#[cfg(feature = "time")] fn test_time_frequency( old_time: &str, second_old_time: &str, @@ -604,9 +596,8 @@ fn test_time_frequency( &log_path, AppendTimestamp::with_format("%Y-%m-%d_%H-%M-%S", FileLimit::MaxFiles(7), date_from), ContentLimit::Time(frequency), - Compression::None, - #[cfg(unix)] - None, + #[cfg(feature = "compression")] Compression::None, + #[cfg(unix)] None, ); writeln!(log, "a").unwrap(); From f70c1577c7040f2265fa013fa1bf700e4c85f082 Mon Sep 17 00:00:00 2001 From: Michael Ripley Date: Thu, 4 Jul 2024 11:47:10 -0500 Subject: [PATCH 2/2] Fix unix tests --- src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests.rs b/src/tests.rs index cf9b069..d8f0ef5 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -344,7 +344,7 @@ fn unix_file_permissions() { &*log_path.to_string_lossy(), AppendCount::new(3), ContentLimit::Lines(2), - Compression::None, + #[cfg(feature = "compression")] Compression::None, Some(*permission), );