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

login
register
mail settings
Submitter phabricator
Date Nov. 22, 2019, 5:44 p.m.
Message ID <303a71cc72d062891e74533baa56ba30@localhost.localdomain>
Download mbox | patch
Permalink /patch/43446/
State Not Applicable
Headers show

Comments

phabricator - Nov. 22, 2019, 5:44 p.m.
Closed by commit rHG20a3bf5e71d6: rust-dirs: address failing tests for `dirs` impl with a temporary fix (authored by Alphare).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs Review".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7503?vs=18310&id=18318

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7503/new/

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




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

Patch

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.