Patchwork D8702: commitctx: return "touched" status from _filecommit

login
register
mail settings
Submitter phabricator
Date July 8, 2020, 8:38 a.m.
Message ID <differential-rev-PHID-DREV-wxbbhhpsei5zzy2i36pk-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/46662/
State Superseded
Headers show

Comments

phabricator - July 8, 2020, 8:38 a.m.
marmoute created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Instead of mutating a list passed in argument, we simply return the information
  from the `_filecommit` function. This make for a cleaner API and allow for
  richer information to be returned. That richer information will be used in the
  next commit to avoid duplicated computation.
  
  This is part of a larger refactoring/cleanup of the commitctx code to clarify
  and augment the logic gathering metadata useful for copy tracing. The current
  code is a tad too long and entangled to make such update easy. We start with
  easy and small cleanup.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/localrepo.py
  tests/test-annotate.t
  tests/test-fastannotate-hg.t

CHANGE DETAILS




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

Patch

diff --git a/tests/test-fastannotate-hg.t b/tests/test-fastannotate-hg.t
--- a/tests/test-fastannotate-hg.t
+++ b/tests/test-fastannotate-hg.t
@@ -485,7 +485,7 @@ 
   > def reposetup(ui, repo):
   >     class legacyrepo(repo.__class__):
   >         def _filecommit(self, fctx, manifest1, manifest2,
-  >                         linkrev, tr, changelist, includecopymeta):
+  >                         linkrev, tr, includecopymeta):
   >             fname = fctx.path()
   >             text = fctx.data()
   >             flog = self.file(fname)
@@ -496,9 +496,8 @@ 
   >             if copy and copy[0] != fname:
   >                 raise error.Abort('copying is not supported')
   >             if fparent2 != node.nullid:
-  >                 changelist.append(fname)
   >                 return flog.add(text, meta, tr, linkrev,
-  >                                 fparent1, fparent2)
+  >                                 fparent1, fparent2), 'modified'
   >             raise error.Abort('only merging is supported')
   >     repo.__class__ = legacyrepo
   > EOF
diff --git a/tests/test-annotate.t b/tests/test-annotate.t
--- a/tests/test-annotate.t
+++ b/tests/test-annotate.t
@@ -483,7 +483,7 @@ 
   > def reposetup(ui, repo):
   >     class legacyrepo(repo.__class__):
   >         def _filecommit(self, fctx, manifest1, manifest2,
-  >                         linkrev, tr, changelist, includecopymeta):
+  >                         linkrev, tr, includecopymeta):
   >             fname = fctx.path()
   >             text = fctx.data()
   >             flog = self.file(fname)
@@ -494,9 +494,8 @@ 
   >             if copy and copy != fname:
   >                 raise error.Abort('copying is not supported')
   >             if fparent2 != node.nullid:
-  >                 changelist.append(fname)
   >                 return flog.add(text, meta, tr, linkrev,
-  >                                 fparent1, fparent2)
+  >                                 fparent1, fparent2), 'modified'
   >             raise error.Abort('only merging is supported')
   >     repo.__class__ = legacyrepo
   > EOF
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -2775,14 +2775,7 @@ 
         return self._currentlock(self._wlockref)
 
     def _filecommit(
-        self,
-        fctx,
-        manifest1,
-        manifest2,
-        linkrev,
-        tr,
-        changelist,
-        includecopymeta,
+        self, fctx, manifest1, manifest2, linkrev, tr, includecopymeta,
     ):
         """
         commit an individual file as part of a larger transaction
@@ -2794,19 +2787,23 @@ 
             manifest2:  manifest of changeset second parent
             linkrev:    revision number of the changeset being created
             tr:         current transation
-            changelist: list of file being changed (modified inplace)
             individual: boolean, set to False to skip storing the copy data
                         (only used by the Google specific feature of using
                         changeset extra as copy source of truth).
 
-        output:
-
-            The resulting filenode
+        output: (filenode, touched)
+
+            filenode: the filenode that should be used by this changeset
+            touched:  one of: None, 'added' or 'modified'
         """
 
         fname = fctx.path()
         fparent1 = manifest1.get(fname, nullid)
         fparent2 = manifest2.get(fname, nullid)
+        touched = None
+        if fparent1 == fparent2 == nullid:
+            touched = 'added'
+
         if isinstance(fctx, context.filectx):
             # This block fast path most comparison, making the assumption that
             # is a bare filectx is used, no merge happened on this file., and
@@ -2823,12 +2820,14 @@ 
                     fparent2 != nullid
                     and manifest2.flags(fname) != fctx.flags()
                 ):
-                    changelist.append(fname)
-                return node
+                    touched = 'modified'
+                return node, touched
 
         flog = self.file(fname)
         meta = {}
         cfname = fctx.copysource()
+        fnode = None
+
         if cfname and cfname != fname:
             # Mark the new revision of this file as a copy of another
             # file.  This copy data will effectively act as a parent
@@ -2905,13 +2904,16 @@ 
         # is the file changed?
         text = fctx.data()
         if fparent2 != nullid or meta or flog.cmp(fparent1, text):
-            changelist.append(fname)
-            return flog.add(text, meta, tr, linkrev, fparent1, fparent2)
+            if touched is None:  # do not overwrite added
+                touched = 'modified'
+            fnode = flog.add(text, meta, tr, linkrev, fparent1, fparent2)
         # are just the flags changed during merge?
         elif fname in manifest1 and manifest1.flags(fname) != fctx.flags():
-            changelist.append(fname)
-
-        return fparent1
+            touched = 'modified'
+            fnode = fparent1
+        else:
+            fnode = fparent1
+        return fnode, touched
 
     def checkcommitpatterns(self, wctx, match, status, fail):
         """check for commit arguments that aren't committable"""
@@ -3144,15 +3146,11 @@ 
                             removed.append(f)
                         else:
                             added.append(f)
-                            m[f] = self._filecommit(
-                                fctx,
-                                m1,
-                                m2,
-                                linkrev,
-                                trp,
-                                changed,
-                                writefilecopymeta,
+                            m[f], is_touched = self._filecommit(
+                                fctx, m1, m2, linkrev, trp, writefilecopymeta,
                             )
+                            if is_touched:
+                                changed.append(f)
                             m.setflag(f, fctx.flags())
                     except OSError:
                         self.ui.warn(