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 _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
