From 2f2abd61ab48c303f2c21710c74265b989b06c7c Mon Sep 17 00:00:00 2001 From: Caelan Sayler Date: Thu, 5 Jun 2025 21:14:30 +0100 Subject: [PATCH] Replace file_rotate crate with more robust internal log rotation logic --- Cargo.lock | 12 -- Cargo.toml | 1 - src/bins/Cargo.toml | 1 - src/bins/src/stub.rs | 12 +- src/lib-csharp/Logging/FileVelopackLogger.cs | 2 +- src/lib-rust/Cargo.toml | 3 +- src/lib-rust/src/file_rotate.rs | 184 +++++++++++++++++++ src/lib-rust/src/lib.rs | 3 + src/lib-rust/src/logging.rs | 7 +- 9 files changed, 195 insertions(+), 30 deletions(-) create mode 100644 src/lib-rust/src/file_rotate.rs diff --git a/Cargo.lock b/Cargo.lock index 6b02ac51..85cc9a18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -705,16 +705,6 @@ dependencies = [ "simd-adler32", ] -[[package]] -name = "file-rotate" -version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e8e2fa049328a1f3295991407a88585805d126dfaadf74b9fe8c194c730aafc" -dependencies = [ - "chrono", - "flate2", -] - [[package]] name = "flate2" version = "1.1.1" @@ -2245,7 +2235,6 @@ dependencies = [ "async-std", "bitflags 2.9.1", "derivative", - "file-rotate", "glob", "lazy_static", "libc", @@ -2285,7 +2274,6 @@ dependencies = [ "derivative", "dialog", "enum-flags", - "file-rotate", "flate2", "fs_extra", "glob", diff --git a/Cargo.toml b/Cargo.toml index f0407e5f..03720427 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,7 +51,6 @@ clap = "4.5" chrono = "0.4" wait-timeout = "0.2" strum = { version = "0.27", features = ["derive"] } -file-rotate = "0.8" simple-stopwatch = "0.1" enum-flags = "0.4" remove_dir_all = "1.0" diff --git a/src/bins/Cargo.toml b/src/bins/Cargo.toml index 6bdbd77d..9cd731a0 100644 --- a/src/bins/Cargo.toml +++ b/src/bins/Cargo.toml @@ -58,7 +58,6 @@ bitflags.workspace = true regex.workspace = true normpath.workspace = true simple-stopwatch.workspace = true -file-rotate.workspace = true wait-timeout.workspace = true pretty-bytes-rust.workspace = true enum-flags.workspace = true diff --git a/src/bins/src/stub.rs b/src/bins/src/stub.rs index 81ef9d87..e7370391 100644 --- a/src/bins/src/stub.rs +++ b/src/bins/src/stub.rs @@ -5,21 +5,17 @@ extern crate log; use std::{ - env, os::windows::process::CommandExt, process::{Command as Process, ExitCode}, }; +use velopack::locator::LocationContext; +use velopack::logging; use windows::Win32::UI::WindowsAndMessaging::AllowSetForegroundWindow; fn main() -> ExitCode { let my_path = std::env::current_exe().unwrap(); - let default_log_file = { - let mut my_dir = env::current_exe().unwrap(); - my_dir.pop(); - my_dir.join("Velopack.log") - }; - - let _ = velopack::logging::init_logging("stub", Some(&default_log_file), false, false, None); + let default_log_file = logging::default_logfile_path(LocationContext::IAmUpdateExe); + let _ = logging::init_logging("stub", Some(&default_log_file), false, false, None); info!("--"); info!("Starting Velopack Stub (at {:?})", my_path); diff --git a/src/lib-csharp/Logging/FileVelopackLogger.cs b/src/lib-csharp/Logging/FileVelopackLogger.cs index 53c06137..19d21d60 100644 --- a/src/lib-csharp/Logging/FileVelopackLogger.cs +++ b/src/lib-csharp/Logging/FileVelopackLogger.cs @@ -16,7 +16,7 @@ namespace Velopack.Logging public FileVelopackLogger(string filePath, uint processId) { ProcessId = processId; - _fileStream = new FileStream(filePath, FileMode.Append, FileAccess.Write, FileShare.ReadWrite); + _fileStream = new FileStream(filePath, FileMode.Append, FileAccess.Write, FileShare.ReadWrite | FileShare.Delete); _writer = new StreamWriter(_fileStream); } diff --git a/src/lib-rust/Cargo.toml b/src/lib-rust/Cargo.toml index f522432e..d4d9e32e 100644 --- a/src/lib-rust/Cargo.toml +++ b/src/lib-rust/Cargo.toml @@ -17,7 +17,7 @@ rust-version.workspace = true default = [] async = ["async-std"] typescript = ["ts-rs"] -file-logging = ["log-panics", "simplelog", "file-rotate", "time"] +file-logging = ["log-panics", "simplelog", "time"] [package.metadata.docs.rs] features = ["async"] @@ -59,7 +59,6 @@ async-std = { workspace = true, optional = true } # file logging log-panics = { workspace = true, optional = true } simplelog = { workspace = true, optional = true } -file-rotate = { workspace = true, optional = true } time = { workspace = true, optional = true } [target.'cfg(windows)'.dependencies] diff --git a/src/lib-rust/src/file_rotate.rs b/src/lib-rust/src/file_rotate.rs new file mode 100644 index 00000000..86312ca0 --- /dev/null +++ b/src/lib-rust/src/file_rotate.rs @@ -0,0 +1,184 @@ +use std::fs::{self, File, OpenOptions}; +use std::io::Write; +use std::path::{Path, PathBuf}; + +pub struct FileRotate { + file: Option, + path: PathBuf, + max_size: u64, + current_size: u64, +} + +/// This is a very simple log file rotate implementation. It should implement the Write trait. +/// It should fail-safe, and not panic if it can't write logs for some reason. +/// It will rotate the "path" to "path.old" when it reaches 1mb, overwriting any previous "path.old" +impl FileRotate { + pub fn new>(path: P, max_size: u64) -> Self { + let path = path.as_ref(); + + let mut me = Self { file: None, path: path.to_path_buf(), max_size, current_size: 0 }; + + me.ensure_log_dir_exists(); + + if let Ok(metadata) = fs::metadata(&me.path) { + let size = metadata.len(); + if size > me.max_size { + me.rotate(); + } else { + me.current_size = size; + } + } + + me.open_file(); + me + } + + fn ensure_log_dir_exists(&self) { + if let Some(parent) = self.path.parent() { + if let Err(_e) = fs::create_dir_all(parent) { + // eprintln!("Failed to create log file parent directory: {e}"); + } + } + } + + fn open_file(&mut self) { + if let Some(_) = &self.file { + return; + } + + self.ensure_log_dir_exists(); + + let mut o = OpenOptions::new(); + o.read(true).create(true).append(true); + + match o.open(&self.path) { + Ok(file) => self.file = Some(file), + Err(_e) => {} //eprintln!("Failed to open log file: {e}"), + } + } + + fn rotate(&mut self) { + self.ensure_log_dir_exists(); + + let _ = self.file.take(); + + let rotation_path = append_extension(&self.path, "old"); + + if let Err(_e) = fs::rename(&self.path, &rotation_path) { + // eprintln!("Failed to rotate log file: {e}"); + } else { + self.current_size = 0; + } + + self.open_file(); + } +} + +impl Write for FileRotate { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + if self.file.is_none() { + self.open_file(); + } + + if let Some(file) = &mut self.file { + let written = file.write(buf)?; + self.current_size += written as u64; + if self.current_size > self.max_size { + self.flush()?; + self.rotate(); + } + Ok(written) + } else { + Err(std::io::Error::new(std::io::ErrorKind::Other, "File not open")) + } + } + + fn flush(&mut self) -> std::io::Result<()> { + if self.file.is_none() { + self.open_file(); + } + + if let Some(file) = &mut self.file { + file.flush() + } else { + Err(std::io::Error::new(std::io::ErrorKind::Other, "File not open")) + } + } +} + +fn append_extension(path: &Path, ext: &str) -> PathBuf { + let mut os_string = path.as_os_str().to_os_string(); + os_string.push(format!(".{}", ext)); + PathBuf::from(os_string) +} + +#[cfg(test)] +fn write_lines(path: &PathBuf, lines: u32) { + let mut writer = FileRotate::new(path, 200); + for idx in 1..=lines { + let _ = writeln!(writer, "Line {}: Hello World!", idx); + } +} + +#[test] +fn test_file_rotate() { + let tmpdir = tempfile::tempdir().unwrap(); + + let path = tmpdir.path().join("test.log"); + write_lines(&path, 28); + + let content = fs::read_to_string(&path).unwrap(); + + let expected = " +20: Hello World! +Line 21: Hello World! +Line 22: Hello World! +Line 23: Hello World! +Line 24: Hello World! +Line 25: Hello World! +Line 26: Hello World! +Line 27: Hello World! +Line 28: Hello World! +"; + + assert_eq!(content.trim(), expected.trim()); +} + +#[test] +fn test_file_rotate_case_insensitive() { + let tmpdir = tempfile::tempdir().unwrap(); + + let path = tmpdir.path().join("test.log"); + write_lines(&path, 7); + write_lines(&path, 7); + write_lines(&path, 7); + + let path2 = tmpdir.path().join("Test.log"); + write_lines(&path2, 7); + write_lines(&path2, 7); + write_lines(&path2, 7); + + let content = fs::read_to_string(&path).unwrap(); + let content_old = fs::read_to_string(&append_extension(&path, "old")).unwrap(); + + let expected = " +Line 6: Hello World! +Line 7: Hello World! +"; + + let expected_old = " +Line 3: Hello World! +Line 4: Hello World! +Line 5: Hello World! +Line 6: Hello World! +Line 7: Hello World! +Line 1: Hello World! +Line 2: Hello World! +Line 3: Hello World! +Line 4: Hello World! +Line 5: Hello World! +"; + + assert_eq!(content.trim(), expected.trim()); + assert_eq!(content_old.trim(), expected_old.trim()); +} diff --git a/src/lib-rust/src/lib.rs b/src/lib-rust/src/lib.rs index 5f2a9104..273e5ce6 100644 --- a/src/lib-rust/src/lib.rs +++ b/src/lib-rust/src/lib.rs @@ -82,6 +82,9 @@ mod app; mod manager; mod util; +// #[cfg(feature = "file-logging")] +mod file_rotate; + /// Utility functions for loading and working with Velopack bundles and manifests. pub mod bundle; diff --git a/src/lib-rust/src/logging.rs b/src/lib-rust/src/logging.rs index f18eb93a..07bcb84d 100644 --- a/src/lib-rust/src/logging.rs +++ b/src/lib-rust/src/logging.rs @@ -96,12 +96,9 @@ pub fn init_logging( if let Some(f) = file { let file_level = if verbose { LevelFilter::Trace } else { LevelFilter::Info }; - let writer = file_rotate::FileRotate::new( + let writer = super::file_rotate::FileRotate::new( f.clone(), - file_rotate::suffix::AppendCount::new(1), // keep 1 old log file - file_rotate::ContentLimit::Bytes(1 * 1024 * 1024), // 1MB max log file size - file_rotate::compression::Compression::None, - None, + 1 * 1024 * 1024, // 1MB max log file size ); loggers.push(WriteLogger::new(file_level, get_config(Some(process_name)), writer)); }