diff --git a/rust/hg-core/src/config/config.rs b/rust/hg-core/src/config/config.rs --- a/rust/hg-core/src/config/config.rs +++ b/rust/hg-core/src/config/config.rs @@ -137,11 +137,11 @@ fn add_trusted_dir(&mut self, path: &Path) -> Result<(), ConfigError> { if let Some(entries) = std::fs::read_dir(path) - .for_file(path) + .when_reading_file(path) .io_not_found_as_none()? { for entry in entries { - let file_path = entry.for_file(path)?.path(); + let file_path = entry.when_reading_file(path)?.path(); if file_path.extension() == Some(std::ffi::OsStr::new("rc")) { self.add_trusted_file(&file_path)? } @@ -151,8 +151,9 @@ } fn add_trusted_file(&mut self, path: &Path) -> Result<(), ConfigError> { - if let Some(data) = - std::fs::read(path).for_file(path).io_not_found_as_none()? + if let Some(data) = std::fs::read(path) + .when_reading_file(path) + .io_not_found_as_none()? { self.layers.extend(ConfigLayer::parse(path, &data)?) } diff --git a/rust/hg-core/src/config/layer.rs b/rust/hg-core/src/config/layer.rs --- a/rust/hg-core/src/config/layer.rs +++ b/rust/hg-core/src/config/layer.rs @@ -150,7 +150,8 @@ // `Path::join` with an absolute argument correctly ignores the // base path let filename = dir.join(&get_path_from_bytes(&filename_bytes)); - let data = std::fs::read(&filename).for_file(&filename)?; + let data = + std::fs::read(&filename).when_reading_file(&filename)?; layers.push(current_layer); layers.extend(Self::parse(&filename, &data)?); current_layer = Self::new(ConfigOrigin::File(src.to_owned())); diff --git a/rust/hg-core/src/errors.rs b/rust/hg-core/src/errors.rs --- a/rust/hg-core/src/errors.rs +++ b/rust/hg-core/src/errors.rs @@ -41,11 +41,15 @@ } /// Details about where an I/O error happened -#[derive(Debug, derive_more::From)] +#[derive(Debug)] pub enum IoErrorContext { - /// A filesystem operation for the given file - #[from] - File(std::path::PathBuf), + ReadingFile(std::path::PathBuf), + WritingFile(std::path::PathBuf), + RemovingFile(std::path::PathBuf), + RenamingFile { + from: std::path::PathBuf, + to: std::path::PathBuf, + }, /// `std::env::current_dir` CurrentDir, /// `std::env::current_exe` @@ -109,28 +113,55 @@ impl fmt::Display for IoErrorContext { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - IoErrorContext::File(path) => path.display().fmt(f), - IoErrorContext::CurrentDir => f.write_str("current directory"), - IoErrorContext::CurrentExe => f.write_str("current executable"), + IoErrorContext::ReadingFile(path) => { + write!(f, "when reading {}", path.display()) + } + IoErrorContext::WritingFile(path) => { + write!(f, "when writing {}", path.display()) + } + IoErrorContext::RemovingFile(path) => { + write!(f, "when removing {}", path.display()) + } + IoErrorContext::RenamingFile { from, to } => write!( + f, + "when renaming {} to {}", + from.display(), + to.display() + ), + IoErrorContext::CurrentDir => write!(f, "current directory"), + IoErrorContext::CurrentExe => write!(f, "current executable"), } } } pub trait IoResultExt { - /// Annotate a possible I/O error as related to a file at the given path. + /// Annotate a possible I/O error as related to a reading a file at the + /// given path. /// - /// This allows printing something like “File not found: example.txt” - /// instead of just “File not found”. + /// This allows printing something like “File not found when reading + /// example.txt” instead of just “File not found”. /// /// Converts a `Result` with `std::io::Error` into one with `HgError`. - fn for_file(self, path: &std::path::Path) -> Result; + fn when_reading_file(self, path: &std::path::Path) -> Result; + + fn with_context( + self, + context: impl FnOnce() -> IoErrorContext, + ) -> Result; } impl IoResultExt for std::io::Result { - fn for_file(self, path: &std::path::Path) -> Result { + fn when_reading_file(self, path: &std::path::Path) -> Result { + self.with_context(|| IoErrorContext::ReadingFile(path.to_owned())) + } + + fn with_context( + self, + context: impl FnOnce() -> IoErrorContext, + ) -> Result { self.map_err(|error| HgError::IoError { error, - context: IoErrorContext::File(path.to_owned()), + context: context(), }) } } diff --git a/rust/hg-core/src/lib.rs b/rust/hg-core/src/lib.rs --- a/rust/hg-core/src/lib.rs +++ b/rust/hg-core/src/lib.rs @@ -29,6 +29,7 @@ pub mod revlog; pub use revlog::*; pub mod config; +pub mod logging; pub mod operations; pub mod revset; pub mod utils; diff --git a/rust/hg-core/src/logging.rs b/rust/hg-core/src/logging.rs new file mode 100644 --- /dev/null +++ b/rust/hg-core/src/logging.rs @@ -0,0 +1,98 @@ +use crate::errors::{HgError, HgResultExt, IoErrorContext, IoResultExt}; +use crate::repo::Vfs; +use std::io::Write; + +/// An utility to append to a log file with the given name, and optionally +/// rotate it after it reaches a certain maximum size. +/// +/// Rotation works by renaming "example.log" to "example.log.1", after renaming +/// "example.log.1" to "example.log.2" etc up to the given maximum number of +/// files. +pub struct LogFile<'a> { + vfs: Vfs<'a>, + name: &'a str, + max_size: Option, + max_files: u32, +} + +impl<'a> LogFile<'a> { + pub fn new(vfs: Vfs<'a>, name: &'a str) -> Self { + Self { + vfs, + name, + max_size: None, + max_files: 0, + } + } + + /// Rotate before writing to a log file that was already larger than the + /// given size, in bytes. `None` disables rotation. + pub fn max_size(mut self, value: Option) -> Self { + self.max_size = value; + self + } + + /// Keep this many rotated files `{name}.1` up to `{name}.{max}`, in + /// addition to the original `{name}` file. + pub fn max_files(mut self, value: u32) -> Self { + self.max_files = value; + self + } + + /// Append the given `bytes` as-is to the log file, after rotating if + /// needed. + /// + /// No trailing newline is added. Make sure to include one in `bytes` if + /// desired. + pub fn write(&self, bytes: &[u8]) -> Result<(), HgError> { + let path = self.vfs.join(self.name); + let context = || IoErrorContext::WritingFile(path.clone()); + let open = || { + std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(&path) + .with_context(context) + }; + let mut file = open()?; + if let Some(max_size) = self.max_size { + if file.metadata().with_context(context)?.len() >= max_size { + // For example with `max_files == 5`, the first iteration of + // this loop has `i == 4` and renames `{name}.4` to `{name}.5`. The + // last iteration renames `{name}.1` to `{name}.2` + for i in (1..self.max_files).rev() { + self.vfs + .rename( + format!("{}.{}", self.name, i), + format!("{}.{}", self.name, i + 1), + ) + .io_not_found_as_none()?; + } + // Then rename `{name}` to `{name}.1`. This is the previously-opened `file`. + self.vfs + .rename(self.name, format!("{}.1", self.name)) + .io_not_found_as_none()?; + // Finally, create a new `{name}` file and replace our `file` handle. + file = open()?; + } + } + file.write_all(bytes).with_context(context)?; + file.sync_all().with_context(context) + } +} + +#[test] +fn test_rotation() { + let temp = tempfile::tempdir().unwrap(); + let vfs = Vfs { base: temp.path() }; + let logger = LogFile::new(vfs, "log").max_size(Some(2)).max_files(2); + logger.write(b"one\n").unwrap(); + logger.write(b"two\n").unwrap(); + logger.write(b"3\n").unwrap(); + logger.write(b"four\n").unwrap(); + logger.write(b"five\n").unwrap(); + assert_eq!(vfs.read("log").unwrap(), b"five\n"); + assert_eq!(vfs.read("log.1").unwrap(), b"3\nfour\n"); + assert_eq!(vfs.read("log.2").unwrap(), b"two\n"); + assert!(vfs.read("log.3").io_not_found_as_none().unwrap().is_none()); +} diff --git a/rust/hg-core/src/repo.rs b/rust/hg-core/src/repo.rs --- a/rust/hg-core/src/repo.rs +++ b/rust/hg-core/src/repo.rs @@ -1,5 +1,5 @@ use crate::config::{Config, ConfigError, ConfigParseError}; -use crate::errors::{HgError, IoResultExt}; +use crate::errors::{HgError, IoErrorContext, IoResultExt}; use crate::requirements; use crate::utils::current_dir; use crate::utils::files::get_path_from_bytes; @@ -38,8 +38,8 @@ /// Filesystem access abstraction for the contents of a given "base" diretory #[derive(Clone, Copy)] -pub(crate) struct Vfs<'a> { - base: &'a Path, +pub struct Vfs<'a> { + pub(crate) base: &'a Path, } impl Repo { @@ -196,12 +196,12 @@ /// For accessing repository files (in `.hg`), except for the store /// (`.hg/store`). - pub(crate) fn hg_vfs(&self) -> Vfs<'_> { + pub fn hg_vfs(&self) -> Vfs<'_> { Vfs { base: &self.dot_hg } } /// For accessing repository store files (in `.hg/store`) - pub(crate) fn store_vfs(&self) -> Vfs<'_> { + pub fn store_vfs(&self) -> Vfs<'_> { Vfs { base: &self.store } } @@ -209,7 +209,7 @@ // The undescore prefix silences the "never used" warning. Remove before // using. - pub(crate) fn _working_directory_vfs(&self) -> Vfs<'_> { + pub fn _working_directory_vfs(&self) -> Vfs<'_> { Vfs { base: &self.working_directory, } @@ -217,26 +217,38 @@ } impl Vfs<'_> { - pub(crate) fn join(&self, relative_path: impl AsRef) -> PathBuf { + pub fn join(&self, relative_path: impl AsRef) -> PathBuf { self.base.join(relative_path) } - pub(crate) fn read( + pub fn read( &self, relative_path: impl AsRef, ) -> Result, HgError> { let path = self.join(relative_path); - std::fs::read(&path).for_file(&path) + std::fs::read(&path).when_reading_file(&path) } - pub(crate) fn mmap_open( + pub fn mmap_open( &self, relative_path: impl AsRef, ) -> Result { let path = self.base.join(relative_path); - let file = std::fs::File::open(&path).for_file(&path)?; + let file = std::fs::File::open(&path).when_reading_file(&path)?; // TODO: what are the safety requirements here? - let mmap = unsafe { MmapOptions::new().map(&file) }.for_file(&path)?; + let mmap = unsafe { MmapOptions::new().map(&file) } + .when_reading_file(&path)?; Ok(mmap) } + + pub fn rename( + &self, + relative_from: impl AsRef, + relative_to: impl AsRef, + ) -> Result<(), HgError> { + let from = self.join(relative_from); + let to = self.join(relative_to); + std::fs::rename(&from, &to) + .with_context(|| IoErrorContext::RenamingFile { from, to }) + } }