Patchwork D11489: dirstate: Propagate dirstate-v2 parse errors from set_dirstate_item

login
register
mail settings
Submitter phabricator
Date Sept. 22, 2021, 8:56 p.m.
Message ID <differential-rev-PHID-DREV-ia4liglpqgwyh4w3xn4x-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49804/
State Superseded
Headers show

Comments

phabricator - Sept. 22, 2021, 8:56 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  … so that Python sees a proper ValueError instead of only
  `SystemError: Rust panic`

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/hg-core/src/dirstate_tree/dirstate_map.rs
  rust/hg-core/src/dirstate_tree/dispatch.rs
  rust/hg-core/src/dirstate_tree/owning_dispatch.rs
  rust/hg-cpython/src/dirstate/dirstate_map.rs

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, 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
@@ -136,7 +136,10 @@ 
     ) -> PyResult<PyObject> {
         let f = path.extract::<PyBytes>(py)?;
         let filename = HgPath::new(f.data(py));
-        self.inner(py).borrow_mut().set_entry(filename, item.get_entry(py));
+        self.inner(py)
+            .borrow_mut()
+            .set_entry(filename, item.get_entry(py))
+            .map_err(|e| v2_error(py, e))?;
         Ok(py.None())
     }
 
diff --git a/rust/hg-core/src/dirstate_tree/owning_dispatch.rs b/rust/hg-core/src/dirstate_tree/owning_dispatch.rs
--- a/rust/hg-core/src/dirstate_tree/owning_dispatch.rs
+++ b/rust/hg-core/src/dirstate_tree/owning_dispatch.rs
@@ -20,7 +20,11 @@ 
         self.get_mut().clear()
     }
 
-    fn set_entry(&mut self, filename: &HgPath, entry: DirstateEntry) {
+    fn set_entry(
+        &mut self,
+        filename: &HgPath,
+        entry: DirstateEntry,
+    ) -> Result<(), DirstateV2ParseError> {
         self.get_mut().set_entry(filename, entry)
     }
 
diff --git a/rust/hg-core/src/dirstate_tree/dispatch.rs b/rust/hg-core/src/dirstate_tree/dispatch.rs
--- a/rust/hg-core/src/dirstate_tree/dispatch.rs
+++ b/rust/hg-core/src/dirstate_tree/dispatch.rs
@@ -39,7 +39,11 @@ 
 
     /// Add the given filename to the map if it is not already there, and
     /// associate the given entry with it.
-    fn set_entry(&mut self, filename: &HgPath, entry: DirstateEntry);
+    fn set_entry(
+        &mut self,
+        filename: &HgPath,
+        entry: DirstateEntry,
+    ) -> Result<(), DirstateV2ParseError>;
 
     /// Add or change the information associated to a given file.
     ///
@@ -321,8 +325,13 @@ 
     ///
     /// XXX Is temporary during a refactor of V1 dirstate and will disappear
     /// shortly.
-    fn set_entry(&mut self, filename: &HgPath, entry: DirstateEntry) {
-        self.set_entry(&filename, entry)
+    fn set_entry(
+        &mut self,
+        filename: &HgPath,
+        entry: DirstateEntry,
+    ) -> Result<(), DirstateV2ParseError> {
+        self.set_entry(&filename, entry);
+        Ok(())
     }
 
     fn add_file(
diff --git a/rust/hg-core/src/dirstate_tree/dirstate_map.rs b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
--- a/rust/hg-core/src/dirstate_tree/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
@@ -758,10 +758,13 @@ 
         self.nodes_with_copy_source_count = 0;
     }
 
-    fn set_entry(&mut self, filename: &HgPath, entry: DirstateEntry) {
-        let node =
-            self.get_or_insert(&filename).expect("no parse error in v1");
-        node.data = NodeData::Entry(entry);
+    fn set_entry(
+        &mut self,
+        filename: &HgPath,
+        entry: DirstateEntry,
+    ) -> Result<(), DirstateV2ParseError> {
+        self.get_or_insert(&filename)?.data = NodeData::Entry(entry);
+        Ok(())
     }
 
     fn add_file(