annotate rust/hg-core/src/logging.rs @ 49000:dd6b67d5c256 stable

rust: fix unsound `OwningDirstateMap` As per the previous patch, `OwningDirstateMap` is unsound. Self-referential structs are difficult to implement correctly in Rust since the compiler is free to move structs around as much as it wants to. They are also very rarely needed in practice, so the state-of-the-art on how they should be done within the Rust rules is still a bit new. The crate `ouroboros` is an attempt at providing a safe way (in the Rust sense) of declaring self-referential structs. It is getting a lot attention and was improved very quickly when soundness issues were found in the past: rather than relying on our own (limited) review circle, we might as well use the de-facto common crate to fix this problem. This will give us a much better chance of finding issues should any new ones be discovered as well as the benefit of fewer `unsafe` APIs of our own. I was starting to think about how I would present a safe API to the old struct but soon realized that the callback-based approach was already done in `ouroboros`, along with a lot more care towards refusing incorrect structs. In short: we don't return a mutable reference to the `DirstateMap` anymore, we expect users of its API to pass a `FnOnce` that takes the map as an argument. This allows our `OwningDirstateMap` to control the input and output lifetimes of the code that modifies it to prevent such issues. Changing to `ouroboros` meant changing every API with it, but it is relatively low churn in the end. It correctly identified the example buggy modification of `copy_map_insert` outlined in the previous patch as violating the borrow rules. Differential Revision: https://phab.mercurial-scm.org/D12429
author Rapha?l Gom?s <rgomes@octobus.net>
date Tue, 05 Apr 2022 10:55:28 +0200
parents 9cd35c8c6044
children db7dbe6f7bb2
Ignore whitespace changes - Everywhere: Within whitespace: At end of lines:
rev   line source
46599
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
1 use crate::errors::{HgError, HgResultExt, IoErrorContext, IoResultExt};
47952
9cd35c8c6044 rust: Move VFS code to its own module
Simon Sapin <simon.sapin@octobus.net>
parents: 46599
diff changeset
2 use crate::vfs::Vfs;
46599
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
3 use std::io::Write;
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
4
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
5 /// An utility to append to a log file with the given name, and optionally
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
6 /// rotate it after it reaches a certain maximum size.
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
7 ///
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
8 /// Rotation works by renaming "example.log" to "example.log.1", after renaming
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
9 /// "example.log.1" to "example.log.2" etc up to the given maximum number of
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
10 /// files.
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
11 pub struct LogFile<'a> {
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
12 vfs: Vfs<'a>,
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
13 name: &'a str,
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
14 max_size: Option<u64>,
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
15 max_files: u32,
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
16 }
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
17
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
18 impl<'a> LogFile<'a> {
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
19 pub fn new(vfs: Vfs<'a>, name: &'a str) -> Self {
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
20 Self {
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
21 vfs,
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
22 name,
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
23 max_size: None,
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
24 max_files: 0,
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
25 }
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
26 }
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
27
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
28 /// Rotate before writing to a log file that was already larger than the
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
29 /// given size, in bytes. `None` disables rotation.
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
30 pub fn max_size(mut self, value: Option<u64>) -> Self {
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
31 self.max_size = value;
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
32 self
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
33 }
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
34
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
35 /// Keep this many rotated files `{name}.1` up to `{name}.{max}`, in
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
36 /// addition to the original `{name}` file.
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
37 pub fn max_files(mut self, value: u32) -> Self {
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
38 self.max_files = value;
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
39 self
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
40 }
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
41
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
42 /// Append the given `bytes` as-is to the log file, after rotating if
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
43 /// needed.
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
44 ///
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
45 /// No trailing newline is added. Make sure to include one in `bytes` if
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
46 /// desired.
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
47 pub fn write(&self, bytes: &[u8]) -> Result<(), HgError> {
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
48 let path = self.vfs.join(self.name);
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
49 let context = || IoErrorContext::WritingFile(path.clone());
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
50 let open = || {
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
51 std::fs::OpenOptions::new()
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
52 .create(true)
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
53 .append(true)
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
54 .open(&path)
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
55 .with_context(context)
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
56 };
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
57 let mut file = open()?;
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
58 if let Some(max_size) = self.max_size {
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
59 if file.metadata().with_context(context)?.len() >= max_size {
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
60 // For example with `max_files == 5`, the first iteration of
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
61 // this loop has `i == 4` and renames `{name}.4` to `{name}.5`.
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
62 // The last iteration renames `{name}.1` to
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
63 // `{name}.2`
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
64 for i in (1..self.max_files).rev() {
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
65 self.vfs
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
66 .rename(
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
67 format!("{}.{}", self.name, i),
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
68 format!("{}.{}", self.name, i + 1),
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
69 )
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
70 .io_not_found_as_none()?;
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
71 }
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
72 // Then rename `{name}` to `{name}.1`. This is the
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
73 // previously-opened `file`.
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
74 self.vfs
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
75 .rename(self.name, format!("{}.1", self.name))
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
76 .io_not_found_as_none()?;
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
77 // Finally, create a new `{name}` file and replace our `file`
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
78 // handle.
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
79 file = open()?;
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
80 }
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
81 }
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
82 file.write_all(bytes).with_context(context)?;
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
83 file.sync_all().with_context(context)
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
84 }
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
85 }
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
86
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
87 #[test]
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
88 fn test_rotation() {
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
89 let temp = tempfile::tempdir().unwrap();
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
90 let vfs = Vfs { base: temp.path() };
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
91 let logger = LogFile::new(vfs, "log").max_size(Some(3)).max_files(2);
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
92 logger.write(b"one\n").unwrap();
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
93 logger.write(b"two\n").unwrap();
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
94 logger.write(b"3\n").unwrap();
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
95 logger.write(b"four\n").unwrap();
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
96 logger.write(b"five\n").unwrap();
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
97 assert_eq!(vfs.read("log").unwrap(), b"five\n");
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
98 assert_eq!(vfs.read("log.1").unwrap(), b"3\nfour\n");
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
99 assert_eq!(vfs.read("log.2").unwrap(), b"two\n");
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
100 assert!(vfs.read("log.3").io_not_found_as_none().unwrap().is_none());
1f55cd5b292f rust: Add a log file rotation utility
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
101 }