changeset 46638:1f55cd5b292f

rust: Add a log file rotation utility 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. Differential Revision: https://phab.mercurial-scm.org/D10010
author Simon Sapin <simon.sapin@octobus.net>
date Thu, 11 Feb 2021 15:51:11 +0100
parents bc08c2331f99
children 36f3a64846c8
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
diffstat 6 files changed, 177 insertions(+), 30 deletions(-) [+]
line wrap: on
line diff
--- a/rust/hg-core/src/config/config.rs	Tue Feb 16 15:22:20 2021 +0100
+++ b/rust/hg-core/src/config/config.rs	Thu Feb 11 15:51:11 2021 +0100
@@ -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)?)
         }
--- a/rust/hg-core/src/config/layer.rs	Tue Feb 16 15:22:20 2021 +0100
+++ b/rust/hg-core/src/config/layer.rs	Thu Feb 11 15:51:11 2021 +0100
@@ -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()));
--- a/rust/hg-core/src/errors.rs	Tue Feb 16 15:22:20 2021 +0100
+++ b/rust/hg-core/src/errors.rs	Thu Feb 11 15:51:11 2021 +0100
@@ -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(),
         })
     }
 }
--- a/rust/hg-core/src/lib.rs	Tue Feb 16 15:22:20 2021 +0100
+++ b/rust/hg-core/src/lib.rs	Thu Feb 11 15:51:11 2021 +0100
@@ -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;
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/rust/hg-core/src/logging.rs	Thu Feb 11 15:51:11 2021 +0100
@@ -0,0 +1,101 @@
+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 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(3)).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());
+}
--- a/rust/hg-core/src/repo.rs	Tue Feb 16 15:22:20 2021 +0100
+++ b/rust/hg-core/src/repo.rs	Thu Feb 11 15:51:11 2021 +0100
@@ -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 })
+    }
 }