dirstate: Pass the final DirstateItem to _rustmap.addfile()

Now that the Python DirstateItem class wraps a Rust DirstateEntry value,
use that value directly instead of converting through v1 data + 5 booleans.

Also remove propogating the return value. None of the callers look at it,
and it is always None.

Differential Revision: https://phab.mercurial-scm.org/D11494

mercurial/dirstatemap.py

@@ -561,19 +561,26 b' if rustmod is not None:'
             from_p2=False,
             possibly_dirty=False,
         ):
-            ret = self._rustmap.addfile(
-                f,
-                mode,
-                size,
-                mtime,
-                added,
-                merged,
-                from_p2,
-                possibly_dirty,
-            )
+            if added:
+                assert not possibly_dirty
+                assert not from_p2
+                item = DirstateItem.new_added()
+            elif merged:
+                assert not possibly_dirty
+                assert not from_p2
+                item = DirstateItem.new_merged()
+            elif from_p2:
+                assert not possibly_dirty
+                item = DirstateItem.new_from_p2()
+            elif possibly_dirty:
+                item = DirstateItem.new_possibly_dirty()
+            else:
+                size = size & rangemask
+                mtime = mtime & rangemask
+                item = DirstateItem.new_normal(mode, size, mtime)
+            self._rustmap.addfile(f, item)
             if added:
                 self.copymap.pop(f, None)
-            return ret
 
         def reset_state(
             self,

rust/hg-core/src/dirstate/dirstate_map.rs

@@ -8,10 +8,8 b''
 use crate::dirstate::parsers::Timestamp;
 use crate::{
     dirstate::EntryState,
-    dirstate::MTIME_UNSET,
     dirstate::SIZE_FROM_OTHER_PARENT,
     dirstate::SIZE_NON_NORMAL,
-    dirstate::V1_RANGEMASK,
     pack_dirstate, parse_dirstate,
     utils::hg_path::{HgPath, HgPathBuf},
     CopyMap, DirsMultiset, DirstateEntry, DirstateError, DirstateParents,
@@ -73,44 +71,7 b' impl DirstateMap {'
         &mut self,
         filename: &HgPath,
         entry: DirstateEntry,
-        // XXX once the dust settle this should probably become an enum
-        added: bool,
-        merged: bool,
-        from_p2: bool,
-        possibly_dirty: bool,
     ) -> Result<(), DirstateError> {
-        let state;
-        let size;
-        let mtime;
-        if added {
-            assert!(!possibly_dirty);
-            assert!(!from_p2);
-            state = EntryState::Added;
-            size = SIZE_NON_NORMAL;
-            mtime = MTIME_UNSET;
-        } else if merged {
-            assert!(!possibly_dirty);
-            assert!(!from_p2);
-            state = EntryState::Merged;
-            size = SIZE_FROM_OTHER_PARENT;
-            mtime = MTIME_UNSET;
-        } else if from_p2 {
-            assert!(!possibly_dirty);
-            state = EntryState::Normal;
-            size = SIZE_FROM_OTHER_PARENT;
-            mtime = MTIME_UNSET;
-        } else if possibly_dirty {
-            state = EntryState::Normal;
-            size = SIZE_NON_NORMAL;
-            mtime = MTIME_UNSET;
-        } else {
-            state = EntryState::Normal;
-            size = entry.size() & V1_RANGEMASK;
-            mtime = entry.mtime() & V1_RANGEMASK;
-        }
-        let mode = entry.mode();
-        let entry = DirstateEntry::from_v1_data(state, mode, size, mtime);
-
         let old_state = self.get(filename).map(|e| e.state());
         if old_state.is_none() || old_state == Some(EntryState::Removed) {
             if let Some(ref mut dirs) = self.dirs {
@@ -417,10 +378,6 b' mod tests {'
         map.add_file(
             HgPath::new(b"meh"),
             DirstateEntry::from_v1_data(EntryState::Normal, 1337, 1337, 1337),
-            false,
-            false,
-            false,
-            false,
         )
         .unwrap();
 

rust/hg-core/src/dirstate/entry.rs

@@ -83,12 +83,7 b' impl DirstateEntry {'
                         mtime: 0,
                     }
                 } else {
-                    Self {
-                        flags: Flags::WDIR_TRACKED | Flags::P1_TRACKED,
-                        mode,
-                        size,
-                        mtime,
-                    }
+                    Self::new_normal(mode, size, mtime)
                 }
             }
             EntryState::Added => Self::new_added(),
@@ -111,7 +106,7 b' impl DirstateEntry {'
         }
     }
 
-    fn new_from_p2() -> Self {
+    pub fn new_from_p2() -> Self {
         Self {
             // might be missing P1_TRACKED
             flags: Flags::WDIR_TRACKED | Flags::P2_TRACKED | Flags::CLEAN_P2,
@@ -121,7 +116,7 b' impl DirstateEntry {'
         }
     }
 
-    fn new_possibly_dirty() -> Self {
+    pub fn new_possibly_dirty() -> Self {
         Self {
             flags: Flags::WDIR_TRACKED
                 | Flags::P1_TRACKED
@@ -132,7 +127,7 b' impl DirstateEntry {'
         }
     }
 
-    fn new_added() -> Self {
+    pub fn new_added() -> Self {
         Self {
             flags: Flags::WDIR_TRACKED,
             mode: 0,
@@ -141,7 +136,7 b' impl DirstateEntry {'
         }
     }
 
-    fn new_merged() -> Self {
+    pub fn new_merged() -> Self {
         Self {
             flags: Flags::WDIR_TRACKED
                 | Flags::P1_TRACKED // might not be true because of rename ?
@@ -153,6 +148,15 b' impl DirstateEntry {'
         }
     }
 
+    pub fn new_normal(mode: i32, size: i32, mtime: i32) -> Self {
+        Self {
+            flags: Flags::WDIR_TRACKED | Flags::P1_TRACKED,
+            mode,
+            size,
+            mtime,
+        }
+    }
+
     /// Creates a new entry in "removed" state.
     ///
     /// `size` is expected to be zero, `SIZE_NON_NORMAL`, or
@@ -161,14 +165,6 b' impl DirstateEntry {'
         Self::from_v1_data(EntryState::Removed, 0, size, 0)
     }
 
-    /// TODO: refactor `DirstateMap::add_file` to not take a `DirstateEntry`
-    /// parameter and remove this constructor
-    pub fn new_for_add_file(mode: i32, size: i32, mtime: i32) -> Self {
-        // XXX Arbitrary default value since the value is determined later
-        let state = EntryState::Normal;
-        Self::from_v1_data(state, mode, size, mtime)
-    }
-
     pub fn tracked(&self) -> bool {
         self.flags.contains(Flags::WDIR_TRACKED)
     }

rust/hg-core/src/dirstate_tree/dirstate_map.rs

@@ -11,10 +11,8 b' use crate::dirstate::parsers::pack_entry'
 use crate::dirstate::parsers::packed_entry_size;
 use crate::dirstate::parsers::parse_dirstate_entries;
 use crate::dirstate::parsers::Timestamp;
-use crate::dirstate::MTIME_UNSET;
 use crate::dirstate::SIZE_FROM_OTHER_PARENT;
 use crate::dirstate::SIZE_NON_NORMAL;
-use crate::dirstate::V1_RANGEMASK;
 use crate::matchers::Matcher;
 use crate::utils::hg_path::{HgPath, HgPathBuf};
 use crate::CopyMapIter;
@@ -771,45 +769,8 b" impl<'on_disk> super::dispatch::Dirstate"
         &mut self,
         filename: &HgPath,
         entry: DirstateEntry,
-        added: bool,
-        merged: bool,
-        from_p2: bool,
-        possibly_dirty: bool,
     ) -> Result<(), DirstateError> {
-        let state;
-        let size;
-        let mtime;
-        if added {
-            assert!(!possibly_dirty);
-            assert!(!from_p2);
-            state = EntryState::Added;
-            size = SIZE_NON_NORMAL;
-            mtime = MTIME_UNSET;
-        } else if merged {
-            assert!(!possibly_dirty);
-            assert!(!from_p2);
-            state = EntryState::Merged;
-            size = SIZE_FROM_OTHER_PARENT;
-            mtime = MTIME_UNSET;
-        } else if from_p2 {
-            assert!(!possibly_dirty);
-            state = EntryState::Normal;
-            size = SIZE_FROM_OTHER_PARENT;
-            mtime = MTIME_UNSET;
-        } else if possibly_dirty {
-            state = EntryState::Normal;
-            size = SIZE_NON_NORMAL;
-            mtime = MTIME_UNSET;
-        } else {
-            state = EntryState::Normal;
-            size = entry.size() & V1_RANGEMASK;
-            mtime = entry.mtime() & V1_RANGEMASK;
-        }
-        let mode = entry.mode();
-        let entry = DirstateEntry::from_v1_data(state, mode, size, mtime);
-
         let old_state = self.get(filename)?.map(|e| e.state());
-
         Ok(self.add_or_remove_file(filename, old_state, entry)?)
     }
 

rust/hg-core/src/dirstate_tree/dispatch.rs

@@ -50,10 +50,6 b' pub trait DirstateMapMethods {'
         &mut self,
         filename: &HgPath,
         entry: DirstateEntry,
-        added: bool,
-        merged: bool,
-        from_p2: bool,
-        possibly_dirty: bool,
     ) -> Result<(), DirstateError>;
 
     /// Mark a file as "removed" (as in `hg rm`).
@@ -326,12 +322,8 b' impl DirstateMapMethods for DirstateMap '
         &mut self,
         filename: &HgPath,
         entry: DirstateEntry,
-        added: bool,
-        merged: bool,
-        from_p2: bool,
-        possibly_dirty: bool,
     ) -> Result<(), DirstateError> {
-        self.add_file(filename, entry, added, merged, from_p2, possibly_dirty)
+        self.add_file(filename, entry)
     }
 
     fn remove_file(

rust/hg-core/src/dirstate_tree/owning_dispatch.rs

@@ -32,19 +32,8 b' impl DirstateMapMethods for OwningDirsta'
         &mut self,
         filename: &HgPath,
         entry: DirstateEntry,
-        added: bool,
-        merged: bool,
-        from_p2: bool,
-        possibly_dirty: bool,
     ) -> Result<(), DirstateError> {
-        self.get_mut().add_file(
-            filename,
-            entry,
-            added,
-            merged,
-            from_p2,
-            possibly_dirty,
-        )
+        self.get_mut().add_file(filename, entry)
     }
 
     fn remove_file(

rust/hg-cpython/src/dirstate/dirstate_map.rs

@@ -27,8 +27,6 b' use crate::{'
 };
 use hg::{
     dirstate::parsers::Timestamp,
-    dirstate::MTIME_UNSET,
-    dirstate::SIZE_NON_NORMAL,
     dirstate_tree::dirstate_map::DirstateMap as TreeDirstateMap,
     dirstate_tree::dispatch::DirstateMapMethods,
     dirstate_tree::on_disk::DirstateV2ParseError,
@@ -145,50 +143,16 b' py_class!(pub class DirstateMap |py| {'
 
     def addfile(
         &self,
-        f: PyObject,
-        mode: PyObject,
-        size: PyObject,
-        mtime: PyObject,
-        added: PyObject,
-        merged: PyObject,
-        from_p2: PyObject,
-        possibly_dirty: PyObject,
-    ) -> PyResult<PyObject> {
-        let f = f.extract::<PyBytes>(py)?;
+        f: PyBytes,
+        item: DirstateItem,
+    ) -> PyResult<PyNone> {
         let filename = HgPath::new(f.data(py));
-        let mode = if mode.is_none(py) {
-            // fallback default value
-            0
-        } else {
-            mode.extract(py)?
-        };
-        let size = if size.is_none(py) {
-            // fallback default value
-            SIZE_NON_NORMAL
-        } else {
-            size.extract(py)?
-        };
-        let mtime = if mtime.is_none(py) {
-            // fallback default value
-            MTIME_UNSET
-        } else {
-            mtime.extract(py)?
-        };
-        let entry = DirstateEntry::new_for_add_file(mode, size, mtime);
-        let added = added.extract::<PyBool>(py)?.is_true();
-        let merged = merged.extract::<PyBool>(py)?.is_true();
-        let from_p2 = from_p2.extract::<PyBool>(py)?.is_true();
-        let possibly_dirty = possibly_dirty.extract::<PyBool>(py)?.is_true();
-        self.inner(py).borrow_mut().add_file(
-            filename,
-            entry,
-            added,
-            merged,
-            from_p2,
-            possibly_dirty
-        ).and(Ok(py.None())).or_else(|e: DirstateError| {
-            Err(PyErr::new::<exc::ValueError, _>(py, e.to_string()))
-        })
+        let entry = item.get_entry(py);
+        self.inner(py)
+            .borrow_mut()
+            .add_file(filename, entry)
+            .map_err(|e |dirstate_error(py, e))?;
+        Ok(PyNone)
     }
 
     def removefile(

rust/hg-cpython/src/dirstate/item.rs

@@ -146,6 +146,36 b' py_class!(pub class DirstateItem |py| {'
         DirstateItem::create_instance(py, Cell::new(entry))
     }
 
+    @classmethod
+    def new_added(_cls) -> PyResult<Self> {
+        let entry = DirstateEntry::new_added();
+        DirstateItem::create_instance(py, Cell::new(entry))
+    }
+
+    @classmethod
+    def new_merged(_cls) -> PyResult<Self> {
+        let entry = DirstateEntry::new_merged();
+        DirstateItem::create_instance(py, Cell::new(entry))
+    }
+
+    @classmethod
+    def new_from_p2(_cls) -> PyResult<Self> {
+        let entry = DirstateEntry::new_from_p2();
+        DirstateItem::create_instance(py, Cell::new(entry))
+    }
+
+    @classmethod
+    def new_possibly_dirty(_cls) -> PyResult<Self> {
+        let entry = DirstateEntry::new_possibly_dirty();
+        DirstateItem::create_instance(py, Cell::new(entry))
+    }
+
+    @classmethod
+    def new_normal(_cls, mode: i32, size: i32, mtime: i32) -> PyResult<Self> {
+        let entry = DirstateEntry::new_normal(mode, size, mtime);
+        DirstateItem::create_instance(py, Cell::new(entry))
+    }
+
     def set_clean(
         &self,
         mode: i32,