D7503: rust-dirs: address failing tests for `dirs` impl with a temporary fix

Alphare (Raphaël Gomès) phabricator at mercurial-scm.org
Fri Nov 22 09:47:18 UTC 2019


Alphare created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  https://phab.mercurial-scm.org/D7252 (5d40317d42b7083b49467502549e25f144888cb3 <https://phab.mercurial-scm.org/rHG5d40317d42b7083b49467502549e25f144888cb3>)
  introduced a regression in Rust tests.
  
  This is a temporary fix that replicates the behavior of the C and Python impl,
  pending the resolution of the discussion (in the phabricator link) about how
  we actually want to solve this problem.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7503

AFFECTED FILES
  rust/hg-core/src/dirstate/dirs_multiset.rs
  rust/hg-core/src/dirstate/dirstate_map.rs
  rust/hg-core/src/lib.rs
  rust/hg-cpython/src/dirstate/dirs_multiset.rs
  rust/hg-cpython/src/dirstate/dirstate_map.rs

CHANGE DETAILS

diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
@@ -25,8 +25,8 @@
 use hg::{
     utils::hg_path::{HgPath, HgPathBuf},
     DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap,
-    DirstateParents, DirstateParseError, EntryState, StateMapIter,
-    PARENT_SIZE,
+    DirstateMapError, DirstateParents, DirstateParseError, EntryState,
+    StateMapIter, PARENT_SIZE,
 };
 
 // TODO
@@ -97,8 +97,9 @@
                 size: size.extract(py)?,
                 mtime: mtime.extract(py)?,
             },
-        );
-        Ok(py.None())
+        ).and(Ok(py.None())).or_else(|e: DirstateMapError| {
+            Err(PyErr::new::<exc::ValueError, _>(py, e.to_string()))
+        })
     }
 
     def removefile(
diff --git a/rust/hg-cpython/src/dirstate/dirs_multiset.rs b/rust/hg-cpython/src/dirstate/dirs_multiset.rs
--- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs
+++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs
@@ -68,8 +68,19 @@
     def addpath(&self, path: PyObject) -> PyResult<PyObject> {
         self.inner_shared(py).borrow_mut()?.add_path(
             HgPath::new(path.extract::<PyBytes>(py)?.data(py)),
-        );
-        Ok(py.None())
+        ).and(Ok(py.None())).or_else(|e| {
+            match e {
+                DirstateMapError::EmptyPath => {
+                    Ok(py.None())
+                },
+                e => {
+                    Err(PyErr::new::<exc::ValueError, _>(
+                        py,
+                        e.to_string(),
+                    ))
+                }
+            }
+        })
     }
 
     def delpath(&self, path: PyObject) -> PyResult<PyObject> {
@@ -79,15 +90,15 @@
             .and(Ok(py.None()))
             .or_else(|e| {
                 match e {
-                    DirstateMapError::PathNotFound(_p) => {
+                    DirstateMapError::EmptyPath => {
+                        Ok(py.None())
+                    },
+                    e => {
                         Err(PyErr::new::<exc::ValueError, _>(
                             py,
-                            "expected a value, found none".to_string(),
+                            e.to_string(),
                         ))
                     }
-                    DirstateMapError::EmptyPath => {
-                        Ok(py.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
@@ -101,6 +101,20 @@
 pub enum DirstateMapError {
     PathNotFound(HgPathBuf),
     EmptyPath,
+    ConsecutiveSlashes,
+}
+
+impl ToString for DirstateMapError {
+    fn to_string(&self) -> String {
+        use crate::DirstateMapError::*;
+        match self {
+            PathNotFound(_) => "expected a value, found none".to_string(),
+            EmptyPath => "Overflow in dirstate.".to_string(),
+            ConsecutiveSlashes => {
+                "found invalid consecutive slashes in path".to_string()
+            }
+        }
+    }
 }
 
 pub enum DirstateError {
diff --git a/rust/hg-core/src/dirstate/dirstate_map.rs b/rust/hg-core/src/dirstate/dirstate_map.rs
--- a/rust/hg-core/src/dirstate/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs
@@ -83,16 +83,16 @@
         filename: &HgPath,
         old_state: EntryState,
         entry: DirstateEntry,
-    ) {
+    ) -> Result<(), DirstateMapError> {
         if old_state == EntryState::Unknown || old_state == EntryState::Removed
         {
             if let Some(ref mut dirs) = self.dirs {
-                dirs.add_path(filename)
+                dirs.add_path(filename)?;
             }
         }
         if old_state == EntryState::Unknown {
             if let Some(ref mut all_dirs) = self.all_dirs {
-                all_dirs.add_path(filename)
+                all_dirs.add_path(filename)?;
             }
         }
         self.state_map.insert(filename.to_owned(), entry.to_owned());
@@ -104,6 +104,7 @@
         if entry.size == SIZE_FROM_OTHER_PARENT {
             self.other_parent_set.insert(filename.to_owned());
         }
+        Ok(())
     }
 
     /// Mark a file as removed in the dirstate.
diff --git a/rust/hg-core/src/dirstate/dirs_multiset.rs b/rust/hg-core/src/dirstate/dirs_multiset.rs
--- a/rust/hg-core/src/dirstate/dirs_multiset.rs
+++ b/rust/hg-core/src/dirstate/dirs_multiset.rs
@@ -65,14 +65,20 @@
     /// Increases the count of deepest directory contained in the path.
     ///
     /// If the directory is not yet in the map, adds its parents.
-    pub fn add_path(&mut self, path: &HgPath) {
+    pub fn add_path(&mut self, path: &HgPath) -> Result<(), DirstateMapError> {
         for subpath in files::find_dirs(path) {
+            if subpath.as_bytes().last() == Some(&b'/') {
+                // TODO Remove this once PathAuditor is certified
+                // as the only entrypoint for path data
+                return Err(DirstateMapError::ConsecutiveSlashes);
+            }
             if let Some(val) = self.inner.get_mut(subpath) {
                 *val += 1;
                 break;
             }
             self.inner.insert(subpath.to_owned(), 1);
         }
+        Ok(())
     }
 
     /// Decreases the count of deepest directory contained in the path.



To: Alphare, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel


More information about the Mercurial-devel mailing list