Patchwork D11315: dirstatemap: replace `removefile` by an explicit `entry.set_untracked()`

login
register
mail settings
Submitter phabricator
Date Aug. 21, 2021, 9:57 a.m.
Message ID <differential-rev-PHID-DREV-syq7yzcsdes6hrdopukj-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49635/
State Superseded
Headers show

Comments

phabricator - Aug. 21, 2021, 9:57 a.m.
marmoute created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  All the other caller goes through `reset_state`, so we can safely have an
  explicit method on `DirstateItem` object.
  This means that all the logic to preserve the previous state (from p2, merged,
  etc) is now properly encapsulated within the DirstateItem. This pave the way to
  using different storage for these information.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/cext/parsers.c
  mercurial/dirstate.py
  mercurial/dirstatemap.py
  mercurial/pure/parsers.py

CHANGE DETAILS




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

Patch

diff --git a/mercurial/pure/parsers.py b/mercurial/pure/parsers.py
--- a/mercurial/pure/parsers.py
+++ b/mercurial/pure/parsers.py
@@ -89,6 +89,22 @@ 
         """
         self._mtime = AMBIGUOUS_TIME
 
+    def set_untracked(self):
+        """mark a file as untracked in the working copy
+
+        This will ultimately be called by command like `hg remove`.
+        """
+        # backup the previous state (useful for merge)
+        size = 0
+        if self.merged:  # merge
+            size = NONNORMAL
+        elif self.from_p2:
+            size = FROM_P2
+        self._state = b'r'
+        self._mode = 0
+        self._size = size
+        self._mtime = 0
+
     def __getitem__(self, idx):
         if idx == 0 or idx == -4:
             msg = b"do not use item[x], use item.state"
diff --git a/mercurial/dirstatemap.py b/mercurial/dirstatemap.py
--- a/mercurial/dirstatemap.py
+++ b/mercurial/dirstatemap.py
@@ -303,32 +303,15 @@ 
         else:
             assert False, 'unreachable'
 
-    def removefile(self, f, in_merge=False):
-        """
-        Mark a file as removed in the dirstate.
-
-        The `size` parameter is used to store sentinel values that indicate
-        the file's previous state.  In the future, we should refactor this
-        to be more explicit about what that state is.
-        """
-        entry = self.get(f)
-        size = 0
-        if in_merge:
-            # XXX we should not be able to have 'm' state and 'FROM_P2' if not
-            # during a merge. So I (marmoute) am not sure we need the
-            # conditionnal at all. Adding double checking this with assert
-            # would be nice.
-            if entry is not None:
-                # backup the previous state
-                if entry.merged:  # merge
-                    size = NONNORMAL
-                elif entry.from_p2:
-                    size = FROM_P2
-                    self.otherparentset.add(f)
-        if entry is not None and not (entry.merged or entry.from_p2):
+    def set_untracked(self, f):
+        """Mark a file as no longer tracked in the dirstate map"""
+        entry = self[f]
+        self._dirs_decr(f, old_entry=entry, remove_variant=True)
+        if entry.from_p2:
+            self.otherparentset.add(f)
+        elif not entry.merged:
             self.copymap.pop(f, None)
-        self._dirs_decr(f, old_entry=entry, remove_variant=True)
-        self._map[f] = DirstateItem(b'r', 0, size, 0)
+        entry.set_untracked()
         self.nonnormalset.add(f)
 
     def dropfile(self, f):
@@ -664,6 +647,14 @@ 
             else:
                 assert False, 'unreachable'
 
+        def set_untracked(self, f):
+            """Mark a file as no longer tracked in the dirstate map"""
+            # in merge is only trigger more logic, so it "fine" to pass it.
+            #
+            # the inner rust dirstate map code need to be adjusted once the API
+            # for dirstate/dirstatemap/DirstateItem is a bit more settled
+            self._rustmap.removefile(f, in_merge=True)
+
         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
@@ -502,7 +502,7 @@ 
         else:
             self._dirty = True
             self._updatedfiles.add(filename)
-            self._map.removefile(filename, in_merge=self.in_merge)
+            self._map.set_untracked(filename)
             return True
 
     @requires_no_parents_change
diff --git a/mercurial/cext/parsers.c b/mercurial/cext/parsers.c
--- a/mercurial/cext/parsers.c
+++ b/mercurial/cext/parsers.c
@@ -223,6 +223,21 @@ 
 	Py_RETURN_NONE;
 }
 
+static PyObject *dirstate_item_set_untracked(dirstateItemObject *self)
+{
+	if (self->state == 'm') {
+		self->size = dirstate_v1_nonnormal;
+	} else if (self->state == 'n' && self->size == dirstate_v1_from_p2) {
+		self->size = dirstate_v1_from_p2;
+	} else {
+		self->size = 0;
+	}
+	self->state = 'r';
+	self->mode = 0;
+	self->mtime = 0;
+	Py_RETURN_NONE;
+}
+
 static PyMethodDef dirstate_item_methods[] = {
     {"v1_state", (PyCFunction)dirstate_item_v1_state, METH_NOARGS,
      "return a \"state\" suitable for v1 serialization"},
@@ -238,6 +253,8 @@ 
      "build a new DirstateItem object from V1 data"},
     {"set_possibly_dirty", (PyCFunction)dirstate_item_set_possibly_dirty,
      METH_NOARGS, "mark a file as \"possibly dirty\""},
+    {"set_untracked", (PyCFunction)dirstate_item_set_untracked, METH_NOARGS,
+     "mark a file as \"untracked\""},
     {"dm_nonnormal", (PyCFunction)dm_nonnormal, METH_NOARGS,
      "True is the entry is non-normal in the dirstatemap sense"},
     {"dm_otherparent", (PyCFunction)dm_otherparent, METH_NOARGS,