Patchwork D10963: dirstate: move the handling of special case within the dirstatemap

login
register
mail settings
Submitter phabricator
Date July 4, 2021, 9:55 p.m.
Message ID <differential-rev-PHID-DREV-dn4qt2xk5fdgv5g4sk32-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49280/
State Superseded
Headers show

Comments

phabricator - July 4, 2021, 9:55 p.m.
marmoute created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  The dirstatemap is as well, if not better, suited to decided how to encode the
  various case. So we move the associated code in the dirstatemap `addfile`
  method.
  
  This means the dirstate no longer need to know about the various magic value
  used in the dirstate V1 format.
  
  The pain of the messy API start to be quite palpable in Rust, we should clean
  this up once the current large refactoring is dealt with.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/dirstate.py
  mercurial/dirstatemap.py
  rust/hg-core/src/dirstate.rs
  rust/hg-core/src/dirstate/dirstate_map.rs
  rust/hg-core/src/dirstate_tree/dirstate_map.rs
  rust/hg-core/src/dirstate_tree/dispatch.rs
  rust/hg-cpython/src/dirstate/dirstate_map.rs
  rust/hg-cpython/src/dirstate/dispatch.rs

CHANGE DETAILS




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

Patch

diff --git a/rust/hg-cpython/src/dirstate/dispatch.rs b/rust/hg-cpython/src/dirstate/dispatch.rs
--- a/rust/hg-cpython/src/dirstate/dispatch.rs
+++ b/rust/hg-cpython/src/dirstate/dispatch.rs
@@ -26,8 +26,16 @@ 
         filename: &HgPath,
         old_state: EntryState,
         entry: DirstateEntry,
+        from_p2: bool,
+        possibly_dirty: bool,
     ) -> Result<(), DirstateError> {
-        self.get_mut().add_file(filename, old_state, entry)
+        self.get_mut().add_file(
+            filename,
+            old_state,
+            entry,
+            from_p2,
+            possibly_dirty,
+        )
     }
 
     fn remove_file(
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
@@ -28,6 +28,8 @@ 
 };
 use hg::{
     dirstate::parsers::Timestamp,
+    dirstate::MTIME_UNSET,
+    dirstate::SIZE_NON_NORMAL,
     dirstate_tree::dispatch::DirstateMapMethods,
     dirstate_tree::on_disk::DirstateV2ParseError,
     errors::HgError,
@@ -110,7 +112,9 @@ 
         state: PyObject,
         mode: PyObject,
         size: PyObject,
-        mtime: PyObject
+        mtime: PyObject,
+        from_p2: PyObject,
+        possibly_dirty: PyObject,
     ) -> PyResult<PyObject> {
         let f = f.extract::<PyBytes>(py)?;
         let filename = HgPath::new(f.data(py));
@@ -125,18 +129,32 @@ 
                 PyErr::new::<exc::ValueError, _>(py, e.to_string())
             })?;
         let mode = mode.extract(py)?;
-        let size = size.extract(py)?;
-        let mtime = mtime.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 {
             state: state,
             mode: mode,
             size: size,
             mtime: mtime,
         };
+        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,
             oldstate,
             entry,
+            from_p2,
+            possibly_dirty
         ).and(Ok(py.None())).or_else(|e: DirstateError| {
             Err(PyErr::new::<exc::ValueError, _>(py, e.to_string()))
         })
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
@@ -49,6 +49,8 @@ 
         filename: &HgPath,
         old_state: EntryState,
         entry: DirstateEntry,
+        from_p2: bool,
+        possibly_dirty: bool,
     ) -> Result<(), DirstateError>;
 
     /// Mark a file as "removed" (as in `hg rm`).
@@ -287,8 +289,10 @@ 
         filename: &HgPath,
         old_state: EntryState,
         entry: DirstateEntry,
+        from_p2: bool,
+        possibly_dirty: bool,
     ) -> Result<(), DirstateError> {
-        self.add_file(filename, old_state, entry)
+        self.add_file(filename, old_state, entry, from_p2, possibly_dirty)
     }
 
     fn remove_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
@@ -11,8 +11,10 @@ 
 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;
@@ -721,7 +723,27 @@ 
         filename: &HgPath,
         old_state: EntryState,
         entry: DirstateEntry,
+        from_p2: bool,
+        possibly_dirty: bool,
     ) -> Result<(), DirstateError> {
+        let mut entry = entry;
+        if entry.state == EntryState::Added {
+            assert!(!possibly_dirty);
+            assert!(!from_p2);
+            entry.size = SIZE_NON_NORMAL;
+            entry.mtime = MTIME_UNSET;
+        } else if from_p2 {
+            assert!(!possibly_dirty);
+            entry.size = SIZE_FROM_OTHER_PARENT;
+            entry.mtime = MTIME_UNSET;
+        } else if possibly_dirty {
+            entry.size = SIZE_NON_NORMAL;
+            entry.mtime = MTIME_UNSET;
+        } else {
+            entry.size = entry.size & V1_RANGEMASK;
+            entry.mtime = entry.mtime & V1_RANGEMASK;
+        }
+
         Ok(self.add_or_remove_file(filename, old_state, entry)?)
     }
 
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
@@ -8,8 +8,10 @@ 
 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,
@@ -68,7 +70,28 @@ 
         filename: &HgPath,
         old_state: EntryState,
         entry: DirstateEntry,
+        // XXX once the dust settle this should probably become an enum
+        from_p2: bool,
+        possibly_dirty: bool,
     ) -> Result<(), DirstateError> {
+        let mut entry = entry;
+        if entry.state == EntryState::Added {
+            assert!(!possibly_dirty);
+            assert!(!from_p2);
+            entry.size = SIZE_NON_NORMAL;
+            entry.mtime = MTIME_UNSET;
+        } else if from_p2 {
+            assert!(!possibly_dirty);
+            entry.size = SIZE_FROM_OTHER_PARENT;
+            entry.mtime = MTIME_UNSET;
+        } else if possibly_dirty {
+            entry.size = SIZE_NON_NORMAL;
+            entry.mtime = MTIME_UNSET;
+        } else {
+            entry.size = entry.size & V1_RANGEMASK;
+            entry.mtime = entry.mtime & V1_RANGEMASK;
+        }
+
         if old_state == EntryState::Unknown || old_state == EntryState::Removed
         {
             if let Some(ref mut dirs) = self.dirs {
@@ -381,6 +404,8 @@ 
                 mtime: 1337,
                 size: 1337,
             },
+            false,
+            false,
         )
         .unwrap();
 
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
@@ -77,6 +77,8 @@ 
     length: unaligned::I32Be,
 }
 
+pub const V1_RANGEMASK: i32 = 0x7FFFFFFF;
+
 pub const MTIME_UNSET: i32 = -1;
 
 /// A `DirstateEntry` with a size of `-2` means that it was merged from the
diff --git a/mercurial/dirstatemap.py b/mercurial/dirstatemap.py
--- a/mercurial/dirstatemap.py
+++ b/mercurial/dirstatemap.py
@@ -35,6 +35,8 @@ 
 # a special value used internally for `time` if the time is ambigeous
 AMBIGUOUS_TIME = -1
 
+rangemask = 0x7FFFFFFF
+
 
 class dirstatemap(object):
     """Map encapsulating the dirstate's contents.
@@ -142,8 +144,37 @@ 
         """Loads the underlying data, if it's not already loaded"""
         self._map
 
-    def addfile(self, f, oldstate, state, mode, size, mtime):
+    def addfile(
+        self,
+        f,
+        oldstate,
+        state,
+        mode,
+        size=None,
+        mtime=None,
+        from_p2=False,
+        possibly_dirty=False,
+    ):
         """Add a tracked file to the dirstate."""
+        if state == b'a':
+            assert not possibly_dirty
+            assert not from_p2
+            size = NONNORMAL
+            mtime = AMBIGUOUS_TIME
+        elif from_p2:
+            assert not possibly_dirty
+            size = FROM_P2
+            mtime = AMBIGUOUS_TIME
+        elif possibly_dirty:
+            size = NONNORMAL
+            mtime = AMBIGUOUS_TIME
+        else:
+            assert size != FROM_P2
+            assert size != NONNORMAL
+            size = size & rangemask
+            mtime = mtime & rangemask
+        assert size is not None
+        assert mtime is not None
         if oldstate in b"?r" and "_dirs" in self.__dict__:
             self._dirs.addpath(f)
         if oldstate == b"?" and "_alldirs" in self.__dict__:
@@ -425,8 +456,27 @@ 
                 False,
             )
 
-        def addfile(self, *args, **kwargs):
-            return self._rustmap.addfile(*args, **kwargs)
+        def addfile(
+            self,
+            f,
+            oldstate,
+            state,
+            mode,
+            size=None,
+            mtime=None,
+            from_p2=False,
+            possibly_dirty=False,
+        ):
+            return self._rustmap.addfile(
+                f,
+                oldstate,
+                state,
+                mode,
+                size,
+                mtime,
+                from_p2,
+                possibly_dirty,
+            )
 
         def removefile(self, *args, **kwargs):
             return self._rustmap.removefile(*args, **kwargs)
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -43,11 +43,10 @@ 
 
 propertycache = util.propertycache
 filecache = scmutil.filecache
-_rangemask = 0x7FFFFFFF
+_rangemask = dirstatemap.rangemask
 
 dirstatetuple = parsers.dirstatetuple
 
-
 # a special value used internally for `size` if the file come from the other parent
 FROM_P2 = dirstatemap.FROM_P2
 
@@ -455,8 +454,8 @@ 
         f,
         state,
         mode,
-        size=NONNORMAL,
-        mtime=AMBIGUOUS_TIME,
+        size=None,
+        mtime=None,
         from_p2=False,
         possibly_dirty=False,
     ):
@@ -476,25 +475,18 @@ 
                     msg = _(b'file %r in dirstate clashes with %r')
                     msg %= (pycompat.bytestr(d), pycompat.bytestr(f))
                     raise error.Abort(msg)
-        if state == b'a':
-            assert not possibly_dirty
-            assert not from_p2
-            size = NONNORMAL
-            mtime = AMBIGUOUS_TIME
-        elif from_p2:
-            assert not possibly_dirty
-            size = FROM_P2
-            mtime = AMBIGUOUS_TIME
-        elif possibly_dirty:
-            mtime = AMBIGUOUS_TIME
-        else:
-            assert size != FROM_P2
-            assert size != NONNORMAL
-            size = size & _rangemask
-            mtime = mtime & _rangemask
         self._dirty = True
         self._updatedfiles.add(f)
-        self._map.addfile(f, oldstate, state, mode, size, mtime)
+        self._map.addfile(
+            f,
+            oldstate,
+            state=state,
+            mode=mode,
+            size=size,
+            mtime=mtime,
+            from_p2=from_p2,
+            possibly_dirty=possibly_dirty,
+        )
 
     def normal(self, f, parentfiledata=None):
         """Mark a file normal and clean.