changeset 52298:645d247d4c75

rust-vfs: rename `open` to `open_write` and `open_read` to `open` `open` being read *and* write is surprising because it differs from the Rust stdlib where `std::fs::File::open` is read-only by default. More importantly, writing is more dangerous than reading, so let's make it more explicit.
author Raphaël Gomès <rgomes@octobus.net>
date Tue, 29 Oct 2024 12:03:55 +0100
parents 644c696b6c18
children 4d0c0c255425
files rust/hg-core/src/revlog/file_io.rs rust/hg-core/src/revlog/inner_revlog.rs rust/hg-core/src/revlog/mod.rs rust/hg-core/src/vfs.rs rust/hg-cpython/src/vfs.rs
diffstat 5 files changed, 34 insertions(+), 32 deletions(-) [+]
line wrap: on
line diff
--- a/rust/hg-core/src/revlog/file_io.rs	Tue Oct 29 11:41:27 2024 +0100
+++ b/rust/hg-core/src/revlog/file_io.rs	Tue Oct 29 12:03:55 2024 +0100
@@ -172,9 +172,9 @@
         let file = if create {
             vfs.create(filename.as_ref(), false)?
         } else if write {
-            vfs.open(filename.as_ref())?
+            vfs.open_write(filename.as_ref())?
         } else {
-            vfs.open_read(filename.as_ref())?
+            vfs.open(filename.as_ref())?
         };
         Ok(Self {
             vfs,
@@ -194,7 +194,7 @@
         let mut file = if create {
             vfs.create(filename.as_ref(), false)?
         } else {
-            vfs.open(filename.as_ref())?
+            vfs.open_write(filename.as_ref())?
         };
         let size = vfs.file_size(&file)?;
         let offset = file
--- a/rust/hg-core/src/revlog/inner_revlog.rs	Tue Oct 29 11:41:27 2024 +0100
+++ b/rust/hg-core/src/revlog/inner_revlog.rs	Tue Oct 29 12:03:55 2024 +0100
@@ -837,7 +837,7 @@
             self.end(Revision((self.len() - 1) as BaseRevision))
         };
         let data_handle = if !self.is_inline() {
-            let data_handle = match self.vfs.open(&self.data_file) {
+            let data_handle = match self.vfs.open_write(&self.data_file) {
                 Ok(mut f) => {
                     if let Some(end) = data_end {
                         f.seek(SeekFrom::Start(end as u64))
@@ -892,10 +892,10 @@
             if self.data_config.check_ambig {
                 self.vfs.open_check_ambig(&self.index_file)
             } else {
-                self.vfs.open(&self.index_file)
+                self.vfs.open_write(&self.index_file)
             }
         } else {
-            self.vfs.open(&self.index_file)
+            self.vfs.open_write(&self.index_file)
         };
         match res {
             Ok(mut handle) => {
@@ -1198,7 +1198,8 @@
         }
         self.vfs.copy(&self.index_file, &pending_index_file)?;
         if let Some(delayed_buffer) = self.delayed_buffer.take() {
-            let mut index_file_handle = self.vfs.open(&pending_index_file)?;
+            let mut index_file_handle =
+                self.vfs.open_write(&pending_index_file)?;
             index_file_handle
                 .seek(SeekFrom::End(0))
                 .when_writing_file(&pending_index_file)?;
@@ -1238,7 +1239,8 @@
                 ));
             }
             (Some(delay), None) => {
-                let mut index_file_handle = self.vfs.open(&self.index_file)?;
+                let mut index_file_handle =
+                    self.vfs.open_write(&self.index_file)?;
                 index_file_handle
                     .seek(SeekFrom::End(0))
                     .when_writing_file(&self.index_file)?;
--- a/rust/hg-core/src/revlog/mod.rs	Tue Oct 29 11:41:27 2024 +0100
+++ b/rust/hg-core/src/revlog/mod.rs	Tue Oct 29 12:03:55 2024 +0100
@@ -494,7 +494,7 @@
     index_path: &Path,
     options: RevlogOpenOptions,
 ) -> Result<Index, HgError> {
-    let buf: IndexData = match store_vfs.open_read(index_path) {
+    let buf: IndexData = match store_vfs.open(index_path) {
         Ok(mut file) => {
             let mut buf = if let Some(threshold) =
                 options.data_config.mmap_index_threshold
--- a/rust/hg-core/src/vfs.rs	Tue Oct 29 11:41:27 2024 +0100
+++ b/rust/hg-core/src/vfs.rs	Tue Oct 29 12:03:55 2024 +0100
@@ -518,13 +518,12 @@
 /// Abstracts over the VFS to allow for different implementations of the
 /// filesystem layer (like passing one from Python).
 pub trait Vfs: Sync + Send + DynClone {
-    // TODO make `open` readonly and make `open_read` an `open_write`
+    /// Open a [`VfsFile::Normal`] for reading the file at `filename`,
+    /// relative to this VFS's root.
+    fn open(&self, filename: &Path) -> Result<VfsFile, HgError>;
     /// Open a [`VfsFile::Normal`] for writing and reading the file at
     /// `filename`, relative to this VFS's root.
-    fn open(&self, filename: &Path) -> Result<VfsFile, HgError>;
-    /// Open a [`VfsFile::Normal`] for reading the file at `filename`,
-    /// relative to this VFS's root.
-    fn open_read(&self, filename: &Path) -> Result<VfsFile, HgError>;
+    fn open_write(&self, filename: &Path) -> Result<VfsFile, HgError>;
     /// Open a [`VfsFile::Normal`] for reading and writing the file at
     /// `filename`, relative to this VFS's root. This file will be checked
     /// for an ambiguous mtime on [`drop`]. See [`is_filetime_ambiguous`].
@@ -580,6 +579,15 @@
 /// users of `hg-core` start doing more on their own, like writing to files.
 impl Vfs for VfsImpl {
     fn open(&self, filename: &Path) -> Result<VfsFile, HgError> {
+        // TODO auditpath
+        let path = self.base.join(filename);
+        Ok(VfsFile::normal(
+            std::fs::File::open(&path).when_reading_file(&path)?,
+            filename.to_owned(),
+        ))
+    }
+
+    fn open_write(&self, filename: &Path) -> Result<VfsFile, HgError> {
         if self.readonly {
             return Err(HgError::abort(
                 "write access in a readonly vfs",
@@ -603,15 +611,6 @@
         ))
     }
 
-    fn open_read(&self, filename: &Path) -> Result<VfsFile, HgError> {
-        // TODO auditpath
-        let path = self.base.join(filename);
-        Ok(VfsFile::normal(
-            std::fs::File::open(&path).when_reading_file(&path)?,
-            filename.to_owned(),
-        ))
-    }
-
     fn open_check_ambig(&self, filename: &Path) -> Result<VfsFile, HgError> {
         if self.readonly {
             return Err(HgError::abort(
@@ -843,15 +842,15 @@
 impl Vfs for FnCacheVfs {
     fn open(&self, filename: &Path) -> Result<VfsFile, HgError> {
         let encoded = path_encode(&get_bytes_from_path(filename));
-        let encoded_path = get_path_from_bytes(&encoded);
-        self.maybe_add_to_fncache(filename, encoded_path)?;
-        self.inner.open(encoded_path)
+        let filename = get_path_from_bytes(&encoded);
+        self.inner.open(filename)
     }
 
-    fn open_read(&self, filename: &Path) -> Result<VfsFile, HgError> {
+    fn open_write(&self, filename: &Path) -> Result<VfsFile, HgError> {
         let encoded = path_encode(&get_bytes_from_path(filename));
-        let filename = get_path_from_bytes(&encoded);
-        self.inner.open_read(filename)
+        let encoded_path = get_path_from_bytes(&encoded);
+        self.maybe_add_to_fncache(filename, encoded_path)?;
+        self.inner.open_write(encoded_path)
     }
 
     fn open_check_ambig(&self, filename: &Path) -> Result<VfsFile, HgError> {
--- a/rust/hg-cpython/src/vfs.rs	Tue Oct 29 11:41:27 2024 +0100
+++ b/rust/hg-cpython/src/vfs.rs	Tue Oct 29 12:03:55 2024 +0100
@@ -142,11 +142,12 @@
 
 impl Vfs for PyVfs {
     fn open(&self, filename: &Path) -> Result<VfsFile, HgError> {
-        self.inner_open(filename, false, false, false, true)
+        self.inner_open(filename, false, false, false, false)
             .map(|(f, _)| VfsFile::normal(f, filename.to_owned()))
     }
-    fn open_read(&self, filename: &Path) -> Result<VfsFile, HgError> {
-        self.inner_open(filename, false, false, false, false)
+
+    fn open_write(&self, filename: &Path) -> Result<VfsFile, HgError> {
+        self.inner_open(filename, false, false, false, true)
             .map(|(f, _)| VfsFile::normal(f, filename.to_owned()))
     }