D10010: rust: Add a log file rotation utility
SimonSapin
phabricator at mercurial-scm.org
Wed Feb 17 12:59:51 UTC 2021
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.
REVISION SUMMARY
This is ported to Rust from `mercurial/loggingutil.py`.
The "builder" pattern is used to make it visible at call sites what the two
numeric parameters mean. In Python they might simply by keyword arguments.
REPOSITORY
rHG Mercurial
BRANCH
default
REVISION DETAIL
https://phab.mercurial-scm.org/D10010
AFFECTED FILES
rust/hg-core/src/config/config.rs
rust/hg-core/src/config/layer.rs
rust/hg-core/src/errors.rs
rust/hg-core/src/lib.rs
rust/hg-core/src/logging.rs
rust/hg-core/src/repo.rs
CHANGE DETAILS
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<Path>) -> PathBuf {
+ pub fn join(&self, relative_path: impl AsRef<Path>) -> PathBuf {
self.base.join(relative_path)
}
- pub(crate) fn read(
+ pub fn read(
&self,
relative_path: impl AsRef<Path>,
) -> Result<Vec<u8>, 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<Path>,
) -> Result<Mmap, HgError> {
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<Path>,
+ relative_to: impl AsRef<Path>,
+ ) -> 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 })
+ }
}
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,95 @@
+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<u64>,
+ 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<u64>) -> 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 i in (1..self.max_files).rev() {
+ self.vfs
+ .rename(
+ format!("{}.{}", self.name, i),
+ format!("{}.{}", self.name, i + 1),
+ )
+ .io_not_found_as_none()?;
+ }
+ self.vfs
+ .rename(self.name, format!("{}.1", self.name))
+ .io_not_found_as_none()?;
+ // The previously-opened `file` was renamed to "name.1".
+ // Now create a new empty file at "name".
+ 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/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/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<T> {
- /// 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<T, HgError>;
+ fn when_reading_file(self, path: &std::path::Path) -> Result<T, HgError>;
+
+ fn with_context(
+ self,
+ context: impl FnOnce() -> IoErrorContext,
+ ) -> Result<T, HgError>;
}
impl<T> IoResultExt<T> for std::io::Result<T> {
- fn for_file(self, path: &std::path::Path) -> Result<T, HgError> {
+ fn when_reading_file(self, path: &std::path::Path) -> Result<T, HgError> {
+ self.with_context(|| IoErrorContext::ReadingFile(path.to_owned()))
+ }
+
+ fn with_context(
+ self,
+ context: impl FnOnce() -> IoErrorContext,
+ ) -> Result<T, HgError> {
self.map_err(|error| HgError::IoError {
error,
- context: IoErrorContext::File(path.to_owned()),
+ context: context(),
})
}
}
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/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)?)
}
To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
More information about the Mercurial-devel
mailing list