commitctx: extract _filecommit too

This function is exclusively used in `commitctx`. So we should extract it too
for consistency and to reduce the `localrepo` bloat.

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.

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

mercurial/commit.py

@@ -11,10 +11,13 import weakref
 from .i18n import _
 from .node import (
     hex,
+    nullid,
     nullrev,
 )
 
 from . import (
+    context,
+    mergestate,
     metadata,
     phases,
     scmutil,
@@ -98,8 +101,8 def commitctx(repo, ctx, error=False, or
                         removed.append(f)
                     else:
                         added.append(f)
-                        m[f], is_touched = repo._filecommit(
-                            fctx, m1, m2, linkrev, trp, writefilecopymeta,
+                        m[f], is_touched = _filecommit(
+                            repo, fctx, m1, m2, linkrev, trp, writefilecopymeta,
                         )
                         if is_touched:
                             touched.append(f)
@@ -213,3 +216,139 def commitctx(repo, ctx, error=False, or
             # if minimal phase was 0 we don't need to retract anything
             phases.registernew(repo, tr, targetphase, [n])
         return n
+
+
+def _filecommit(
+    repo, fctx, manifest1, manifest2, linkrev, tr, includecopymeta,
+):
+    """
+    commit an individual file as part of a larger transaction
+
+    input:
+
+        fctx:       a file context with the content we are trying to commit
+        manifest1:  manifest of changeset first parent
+        manifest2:  manifest of changeset second parent
+        linkrev:    revision number of the changeset being created
+        tr:         current transation
+        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: (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 comparisons which are usually done. It
+        # assumes that bare filectx is used and no merge happened, hence no
+        # need to create a new file revision in this case.
+        node = fctx.filenode()
+        if node in [fparent1, fparent2]:
+            repo.ui.debug(b'reusing %s filelog entry\n' % fname)
+            if (
+                fparent1 != nullid and manifest1.flags(fname) != fctx.flags()
+            ) or (
+                fparent2 != nullid and manifest2.flags(fname) != fctx.flags()
+            ):
+                touched = 'modified'
+            return node, touched
+
+    flog = repo.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
+        # of this new revision.  If this is a merge, the first
+        # parent will be the nullid (meaning "look up the copy data")
+        # and the second one will be the other parent.  For example:
+        #
+        # 0 --- 1 --- 3   rev1 changes file foo
+        #   \       /     rev2 renames foo to bar and changes it
+        #    \- 2 -/      rev3 should have bar with all changes and
+        #                      should record that bar descends from
+        #                      bar in rev2 and foo in rev1
+        #
+        # this allows this merge to succeed:
+        #
+        # 0 --- 1 --- 3   rev4 reverts the content change from rev2
+        #   \       /     merging rev3 and rev4 should use bar@rev2
+        #    \- 2 --- 4        as the merge base
+        #
+
+        cnode = manifest1.get(cfname)
+        newfparent = fparent2
+
+        if manifest2:  # branch merge
+            if fparent2 == nullid or cnode is None:  # copied on remote side
+                if cfname in manifest2:
+                    cnode = manifest2[cfname]
+                    newfparent = fparent1
+
+        # Here, we used to search backwards through history to try to find
+        # where the file copy came from if the source of a copy was not in
+        # the parent directory. However, this doesn't actually make sense to
+        # do (what does a copy from something not in your working copy even
+        # mean?) and it causes bugs (eg, issue4476). Instead, we will warn
+        # the user that copy information was dropped, so if they didn't
+        # expect this outcome it can be fixed, but this is the correct
+        # behavior in this circumstance.
+
+        if cnode:
+            repo.ui.debug(b" %s: copy %s:%s\n" % (fname, cfname, hex(cnode)))
+            if includecopymeta:
+                meta[b"copy"] = cfname
+                meta[b"copyrev"] = hex(cnode)
+            fparent1, fparent2 = nullid, newfparent
+        else:
+            repo.ui.warn(
+                _(
+                    b"warning: can't find ancestor for '%s' "
+                    b"copied from '%s'!\n"
+                )
+                % (fname, cfname)
+            )
+
+    elif fparent1 == nullid:
+        fparent1, fparent2 = fparent2, nullid
+    elif fparent2 != nullid:
+        # is one parent an ancestor of the other?
+        fparentancestors = flog.commonancestorsheads(fparent1, fparent2)
+        if fparent1 in fparentancestors:
+            fparent1, fparent2 = fparent2, nullid
+        elif fparent2 in fparentancestors:
+            fparent2 = nullid
+        elif not fparentancestors:
+            # TODO: this whole if-else might be simplified much more
+            ms = mergestate.mergestate.read(repo)
+            if (
+                fname in ms
+                and ms[fname] == mergestate.MERGE_RECORD_MERGED_OTHER
+            ):
+                fparent1, fparent2 = fparent2, nullid
+
+    # is the file changed?
+    text = fctx.data()
+    if fparent2 != nullid or meta or flog.cmp(fparent1, text):
+        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():
+        touched = 'modified'
+        fnode = fparent1
+    else:
+        fnode = fparent1
+    return fnode, touched

mercurial/localrepo.py

@@ -2771,145 +2771,6 class localrepository(object):
         """Returns the wlock if it's held, or None if it's not."""
         return self._currentlock(self._wlockref)
 
-    def _filecommit(
-        self, fctx, manifest1, manifest2, linkrev, tr, includecopymeta,
-    ):
-        """
-        commit an individual file as part of a larger transaction
-
-        input:
-
-            fctx:       a file context with the content we are trying to commit
-            manifest1:  manifest of changeset first parent
-            manifest2:  manifest of changeset second parent
-            linkrev:    revision number of the changeset being created
-            tr:         current transation
-            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: (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 comparisons which are usually done. It
-            # assumes that bare filectx is used and no merge happened, hence no
-            # need to create a new file revision in this case.
-            node = fctx.filenode()
-            if node in [fparent1, fparent2]:
-                self.ui.debug(b'reusing %s filelog entry\n' % fname)
-                if (
-                    fparent1 != nullid
-                    and manifest1.flags(fname) != fctx.flags()
-                ) or (
-                    fparent2 != nullid
-                    and manifest2.flags(fname) != fctx.flags()
-                ):
-                    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
-            # of this new revision.  If this is a merge, the first
-            # parent will be the nullid (meaning "look up the copy data")
-            # and the second one will be the other parent.  For example:
-            #
-            # 0 --- 1 --- 3   rev1 changes file foo
-            #   \       /     rev2 renames foo to bar and changes it
-            #    \- 2 -/      rev3 should have bar with all changes and
-            #                      should record that bar descends from
-            #                      bar in rev2 and foo in rev1
-            #
-            # this allows this merge to succeed:
-            #
-            # 0 --- 1 --- 3   rev4 reverts the content change from rev2
-            #   \       /     merging rev3 and rev4 should use bar@rev2
-            #    \- 2 --- 4        as the merge base
-            #
-
-            cnode = manifest1.get(cfname)
-            newfparent = fparent2
-
-            if manifest2:  # branch merge
-                if fparent2 == nullid or cnode is None:  # copied on remote side
-                    if cfname in manifest2:
-                        cnode = manifest2[cfname]
-                        newfparent = fparent1
-
-            # Here, we used to search backwards through history to try to find
-            # where the file copy came from if the source of a copy was not in
-            # the parent directory. However, this doesn't actually make sense to
-            # do (what does a copy from something not in your working copy even
-            # mean?) and it causes bugs (eg, issue4476). Instead, we will warn
-            # the user that copy information was dropped, so if they didn't
-            # expect this outcome it can be fixed, but this is the correct
-            # behavior in this circumstance.
-
-            if cnode:
-                self.ui.debug(
-                    b" %s: copy %s:%s\n" % (fname, cfname, hex(cnode))
-                )
-                if includecopymeta:
-                    meta[b"copy"] = cfname
-                    meta[b"copyrev"] = hex(cnode)
-                fparent1, fparent2 = nullid, newfparent
-            else:
-                self.ui.warn(
-                    _(
-                        b"warning: can't find ancestor for '%s' "
-                        b"copied from '%s'!\n"
-                    )
-                    % (fname, cfname)
-                )
-
-        elif fparent1 == nullid:
-            fparent1, fparent2 = fparent2, nullid
-        elif fparent2 != nullid:
-            # is one parent an ancestor of the other?
-            fparentancestors = flog.commonancestorsheads(fparent1, fparent2)
-            if fparent1 in fparentancestors:
-                fparent1, fparent2 = fparent2, nullid
-            elif fparent2 in fparentancestors:
-                fparent2 = nullid
-            elif not fparentancestors:
-                # TODO: this whole if-else might be simplified much more
-                ms = mergestatemod.mergestate.read(self)
-                if (
-                    fname in ms
-                    and ms[fname] == mergestatemod.MERGE_RECORD_MERGED_OTHER
-                ):
-                    fparent1, fparent2 = fparent2, nullid
-
-        # is the file changed?
-        text = fctx.data()
-        if fparent2 != nullid or meta or flog.cmp(fparent1, text):
-            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():
-            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"""
         if match.isexact() or match.prefix():

tests/test-annotate.t

@@ -479,14 +479,12 and its ancestor by overriding "repo._fi
 
   $ cat > ../legacyrepo.py <<EOF
   > from __future__ import absolute_import
-  > from mercurial import error, node
-  > def reposetup(ui, repo):
-  >     class legacyrepo(repo.__class__):
-  >         def _filecommit(self, fctx, manifest1, manifest2,
+  > from mercurial import commit, error, extensions, node
+  > def _filecommit(orig, repo, fctx, manifest1, manifest2,
   >                         linkrev, tr, includecopymeta):
   >             fname = fctx.path()
   >             text = fctx.data()
-  >             flog = self.file(fname)
+  >     flog = repo.file(fname)
   >             fparent1 = manifest1.get(fname, node.nullid)
   >             fparent2 = manifest2.get(fname, node.nullid)
   >             meta = {}
@@ -497,7 +495,8 and its ancestor by overriding "repo._fi
   >                 return flog.add(text, meta, tr, linkrev,
   >                                 fparent1, fparent2), 'modified'
   >             raise error.Abort('only merging is supported')
-  >     repo.__class__ = legacyrepo
+  > def uisetup(ui):
+  >     extensions.wrapfunction(commit, '_filecommit', _filecommit)
   > EOF
 
   $ cat > baz <<EOF

tests/test-fastannotate-hg.t

@@ -481,25 +481,25 and (2) the extension to allow filelog m
 and its ancestor by overriding "repo._filecommit".
 
   $ cat > ../legacyrepo.py <<EOF
-  > from mercurial import error, node
-  > def reposetup(ui, repo):
-  >     class legacyrepo(repo.__class__):
-  >         def _filecommit(self, fctx, manifest1, manifest2,
+  > from __future__ import absolute_import
+  > from mercurial import commit, error, extensions, node
+  > def _filecommit(orig, repo, fctx, manifest1, manifest2,
   >                         linkrev, tr, includecopymeta):
   >             fname = fctx.path()
   >             text = fctx.data()
-  >             flog = self.file(fname)
+  >     flog = repo.file(fname)
   >             fparent1 = manifest1.get(fname, node.nullid)
   >             fparent2 = manifest2.get(fname, node.nullid)
   >             meta = {}
-  >             copy = fctx.renamed()
-  >             if copy and copy[0] != fname:
+  >     copy = fctx.copysource()
+  >     if copy and copy != fname:
   >                 raise error.Abort('copying is not supported')
   >             if fparent2 != node.nullid:
   >                 return flog.add(text, meta, tr, linkrev,
   >                                 fparent1, fparent2), 'modified'
   >             raise error.Abort('only merging is supported')
-  >     repo.__class__ = legacyrepo
+  > def uisetup(ui):
+  >     extensions.wrapfunction(commit, '_filecommit', _filecommit)
   > EOF
 
   $ cat > baz <<EOF