[Commented On] D11315: dirstatemap: replace `removefile` by an explicit `entry.set_untracked()`

[baymax (Baymax, Your Personal Patch-care Companion)]

baymax added a comment.
baymax updated this revision to Diff 30041.


  ✅ refresh by Heptapod after a successful CI run (🐙 💚)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D11315?vs=29964&id=30041

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D11315/new/

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

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,



To: marmoute, #hg-reviewers, Alphare
Cc: mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210826/1754d3cf/attachment-0002.html>