Patchwork D10006: rust: Make `DirstateParents`’s fields typed `Node`s

login
register
mail settings
Submitter phabricator
Date Feb. 17, 2021, 12:59 p.m.
Message ID <differential-rev-PHID-DREV-elpgt4udn5u7pfykxrsk-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48323/
State Superseded
Headers show

Comments

phabricator - Feb. 17, 2021, 12:59 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Instead of plain byte arrays.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/hg-core/src/dirstate.rs
  rust/hg-core/src/dirstate/dirstate_map.rs
  rust/hg-core/src/dirstate/parsers.rs
  rust/hg-core/src/revlog/node.rs
  rust/hg-cpython/src/dirstate/dirstate_map.rs
  rust/hg-cpython/src/parsers.rs

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/rust/hg-cpython/src/parsers.rs b/rust/hg-cpython/src/parsers.rs
--- a/rust/hg-cpython/src/parsers.rs
+++ b/rust/hg-cpython/src/parsers.rs
@@ -53,10 +53,7 @@ 
                     PyBytes::new(py, copy_path.as_bytes()),
                 )?;
             }
-            Ok(
-                (PyBytes::new(py, &parents.p1), PyBytes::new(py, &parents.p2))
-                    .to_py_object(py),
-            )
+            Ok(dirstate_parents_to_pytuple(py, parents))
         }
         Err(e) => Err(PyErr::new::<exc::ValueError, _>(py, e.to_string())),
     }
@@ -155,3 +152,12 @@ 
 
     Ok(m)
 }
+
+pub(crate) fn dirstate_parents_to_pytuple(
+    py: Python,
+    parents: &DirstateParents,
+) -> PyTuple {
+    let p1 = PyBytes::new(py, parents.p1.as_bytes());
+    let p2 = PyBytes::new(py, parents.p2.as_bytes());
+    (p1, p2).to_py_object(py)
+}
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
@@ -24,12 +24,14 @@ 
         NonNormalEntries, NonNormalEntriesIterator,
     },
     dirstate::{dirs_multiset::Dirs, make_dirstate_tuple},
+    parsers::dirstate_parents_to_pytuple,
 };
 use hg::{
     errors::HgError,
+    revlog::Node,
     utils::hg_path::{HgPath, HgPathBuf},
     DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap,
-    DirstateMapError, DirstateParents, EntryState, StateMapIter, PARENT_SIZE,
+    DirstateMapError, DirstateParents, EntryState, StateMapIter,
 };
 
 // TODO
@@ -285,10 +287,7 @@ 
     def parents(&self, st: PyObject) -> PyResult<PyTuple> {
         self.inner(py).borrow_mut()
             .parents(st.extract::<PyBytes>(py)?.data(py))
-            .and_then(|d| {
-                Ok((PyBytes::new(py, &d.p1), PyBytes::new(py, &d.p2))
-                    .to_py_object(py))
-            })
+            .map(|parents| dirstate_parents_to_pytuple(py, parents))
             .or_else(|_| {
                 Err(PyErr::new::<exc::OSError, _>(
                     py,
@@ -311,9 +310,8 @@ 
             .read(st.extract::<PyBytes>(py)?.data(py))
         {
             Ok(Some(parents)) => Ok(Some(
-                (PyBytes::new(py, &parents.p1), PyBytes::new(py, &parents.p2))
-                    .to_py_object(py)
-                    .into_object(),
+                dirstate_parents_to_pytuple(py, parents)
+                    .into_object()
             )),
             Ok(None) => Ok(Some(py.None())),
             Err(_) => Err(PyErr::new::<exc::OSError, _>(
@@ -601,7 +599,7 @@ 
     Option<(PyBytes, PyObject)>
 );
 
-fn extract_node_id(py: Python, obj: &PyObject) -> PyResult<[u8; PARENT_SIZE]> {
+fn extract_node_id(py: Python, obj: &PyObject) -> PyResult<Node> {
     let bytes = obj.extract::<PyBytes>(py)?;
     match bytes.data(py).try_into() {
         Ok(s) => Ok(s),
diff --git a/rust/hg-core/src/revlog/node.rs b/rust/hg-core/src/revlog/node.rs
--- a/rust/hg-core/src/revlog/node.rs
+++ b/rust/hg-core/src/revlog/node.rs
@@ -85,6 +85,13 @@ 
     }
 }
 
+impl From<&'_ NodeData> for Node {
+    #[inline]
+    fn from(data: &'_ NodeData) -> Self {
+        Self { data: *data }
+    }
+}
+
 impl fmt::LowerHex for Node {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         for &byte in &self.data {
diff --git a/rust/hg-core/src/dirstate/parsers.rs b/rust/hg-core/src/dirstate/parsers.rs
--- a/rust/hg-core/src/dirstate/parsers.rs
+++ b/rust/hg-core/src/dirstate/parsers.rs
@@ -89,8 +89,8 @@ 
 
     let mut packed = Vec::with_capacity(expected_size);
 
-    packed.extend(&parents.p1);
-    packed.extend(&parents.p2);
+    packed.extend(parents.p1.as_bytes());
+    packed.extend(parents.p2.as_bytes());
 
     for (filename, entry) in state_map.iter_mut() {
         let new_filename = filename.to_owned();
@@ -223,8 +223,8 @@ 
         let mut state_map = StateMap::default();
         let copymap = FastHashMap::default();
         let parents = DirstateParents {
-            p1: *b"12345678910111213141",
-            p2: *b"00000000000000000000",
+            p1: b"12345678910111213141".into(),
+            p2: b"00000000000000000000".into(),
         };
         let now = Duration::new(15000000, 0);
         let expected = b"1234567891011121314100000000000000000000".to_vec();
@@ -254,8 +254,8 @@ 
 
         let copymap = FastHashMap::default();
         let parents = DirstateParents {
-            p1: *b"12345678910111213141",
-            p2: *b"00000000000000000000",
+            p1: b"12345678910111213141".into(),
+            p2: b"00000000000000000000".into(),
         };
         let now = Duration::new(15000000, 0);
         let expected = [
@@ -294,8 +294,8 @@ 
             HgPathBuf::from_bytes(b"copyname"),
         );
         let parents = DirstateParents {
-            p1: *b"12345678910111213141",
-            p2: *b"00000000000000000000",
+            p1: b"12345678910111213141".into(),
+            p2: b"00000000000000000000".into(),
         };
         let now = Duration::new(15000000, 0);
         let expected = [
@@ -334,8 +334,8 @@ 
             HgPathBuf::from_bytes(b"copyname"),
         );
         let parents = DirstateParents {
-            p1: *b"12345678910111213141",
-            p2: *b"00000000000000000000",
+            p1: b"12345678910111213141".into(),
+            p2: b"00000000000000000000".into(),
         };
         let now = Duration::new(15000000, 0);
         let result =
@@ -412,8 +412,8 @@ 
             HgPathBuf::from_bytes(b"copyname2"),
         );
         let parents = DirstateParents {
-            p1: *b"12345678910111213141",
-            p2: *b"00000000000000000000",
+            p1: b"12345678910111213141".into(),
+            p2: b"00000000000000000000".into(),
         };
         let now = Duration::new(15000000, 0);
         let result =
@@ -458,8 +458,8 @@ 
             HgPathBuf::from_bytes(b"copyname"),
         );
         let parents = DirstateParents {
-            p1: *b"12345678910111213141",
-            p2: *b"00000000000000000000",
+            p1: b"12345678910111213141".into(),
+            p2: b"00000000000000000000".into(),
         };
         let now = Duration::new(15000000, 0);
         let result =
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
@@ -6,7 +6,7 @@ 
 // GNU General Public License version 2 or any later version.
 
 use crate::errors::HgError;
-use crate::revlog::node::NULL_NODE_ID;
+use crate::revlog::node::NULL_NODE;
 use crate::{
     dirstate::{parsers::PARENT_SIZE, EntryState, SIZE_FROM_OTHER_PARENT},
     pack_dirstate, parse_dirstate,
@@ -73,8 +73,8 @@ 
         self.non_normal_set = None;
         self.other_parent_set = None;
         self.set_parents(&DirstateParents {
-            p1: NULL_NODE_ID,
-            p2: NULL_NODE_ID,
+            p1: NULL_NODE,
+            p2: NULL_NODE,
         })
     }
 
@@ -367,8 +367,8 @@ 
             };
         } else if file_contents.is_empty() {
             parents = DirstateParents {
-                p1: NULL_NODE_ID,
-                p2: NULL_NODE_ID,
+                p1: NULL_NODE,
+                p2: NULL_NODE,
             };
         } else {
             return Err(
diff --git a/rust/hg-core/src/dirstate.rs b/rust/hg-core/src/dirstate.rs
--- a/rust/hg-core/src/dirstate.rs
+++ b/rust/hg-core/src/dirstate.rs
@@ -6,6 +6,7 @@ 
 // GNU General Public License version 2 or any later version.
 
 use crate::errors::HgError;
+use crate::revlog::Node;
 use crate::{utils::hg_path::HgPathBuf, FastHashMap};
 use bytes_cast::{unaligned, BytesCast};
 use std::collections::hash_map;
@@ -21,8 +22,8 @@ 
 #[derive(Debug, PartialEq, Clone, BytesCast)]
 #[repr(C)]
 pub struct DirstateParents {
-    pub p1: [u8; 20],
-    pub p2: [u8; 20],
+    pub p1: Node,
+    pub p2: Node,
 }
 
 /// The C implementation uses all signed types. This will be an issue