Patchwork [1,of,2] commit: pass mergestate into _filecommit() instead of re-reading it

login
register
mail settings
Submitter Pulkit Goyal
Date Sept. 10, 2020, 6:16 a.m.
Message ID <e82b76915ffaa62dcdb8.1599718612@workspace>
Download mbox | patch
Permalink /patch/47120/
State Accepted
Headers show

Comments

Pulkit Goyal - Sept. 10, 2020, 6:16 a.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1599122644 -19800
#      Thu Sep 03 14:14:04 2020 +0530
# Node ID e82b76915ffaa62dcdb8a088a877f41c2a2a87ca
# Parent  092035fbd42926fcaf274c60857d3e758d419526
# EXP-Topic merge-newnode
commit: pass mergestate into _filecommit() instead of re-reading it

mergestate reading although cheap is not free. Let's read mergestate once on top
and pass it into `_filecommit()`.

In upcoming patches, we will be reading mergestate more in `_filecommit()`.

Differential Revision: https://phab.mercurial-scm.org/D8984
Yuya Nishihara - Sept. 11, 2020, 12:38 a.m.
On Thu, 10 Sep 2020 11:46:52 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1599122644 -19800
> #      Thu Sep 03 14:14:04 2020 +0530
> # Node ID e82b76915ffaa62dcdb8a088a877f41c2a2a87ca
> # Parent  092035fbd42926fcaf274c60857d3e758d419526
> # EXP-Topic merge-newnode
> commit: pass mergestate into _filecommit() instead of re-reading it

>  def _filecommit(
> -    repo, fctx, manifest1, manifest2, linkrev, tr, includecopymeta,
> +    repo, fctx, manifest1, manifest2, linkrev, tr, includecopymeta, ms=None,

Maybe better to not allow omitting "ms" argument since that could be source
of bugs.

> @@ -324,8 +329,7 @@ def _filecommit(
>              fparent2 = nullid
>          elif not fparentancestors:
>              # TODO: this whole if-else might be simplified much more
> -            ms = mergestate.mergestate.read(repo)
> -            if ms.extras(fname).get(b'filenode-source') == b'other':
> +            if ms and ms.extras(fname).get(b'filenode-source') == b'other':
>                  fparent1, fparent2 = fparent2, nullid

and if I understand it, merge should be active here?

Patch

diff --git a/mercurial/commit.py b/mercurial/commit.py
--- a/mercurial/commit.py
+++ b/mercurial/commit.py
@@ -157,6 +157,9 @@  def _process_files(tr, ctx, error=False)
     m = mctx.read()
     m1 = m1ctx.read()
     m2 = m2ctx.read()
+    ms = mergestate.mergestate.read(repo)
+    if not ms.active():
+        ms = None
 
     files = metadata.ChangingFiles()
 
@@ -175,7 +178,7 @@  def _process_files(tr, ctx, error=False)
             else:
                 added.append(f)
                 m[f], is_touched = _filecommit(
-                    repo, fctx, m1, m2, linkrev, tr, writefilecopymeta,
+                    repo, fctx, m1, m2, linkrev, tr, writefilecopymeta, ms
                 )
                 if is_touched:
                     if is_touched == 'added':
@@ -211,7 +214,7 @@  def _process_files(tr, ctx, error=False)
 
 
 def _filecommit(
-    repo, fctx, manifest1, manifest2, linkrev, tr, includecopymeta,
+    repo, fctx, manifest1, manifest2, linkrev, tr, includecopymeta, ms=None,
 ):
     """
     commit an individual file as part of a larger transaction
@@ -226,6 +229,8 @@  def _filecommit(
         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).
+        ms:         mergestate object (None if no mergestate exists
+                    while committing)
 
     output: (filenode, touched)
 
@@ -324,8 +329,7 @@  def _filecommit(
             fparent2 = nullid
         elif not fparentancestors:
             # TODO: this whole if-else might be simplified much more
-            ms = mergestate.mergestate.read(repo)
-            if ms.extras(fname).get(b'filenode-source') == b'other':
+            if ms and ms.extras(fname).get(b'filenode-source') == b'other':
                 fparent1, fparent2 = fparent2, nullid
 
     # is the file changed?
diff --git a/tests/test-annotate.t b/tests/test-annotate.t
--- a/tests/test-annotate.t
+++ b/tests/test-annotate.t
@@ -481,7 +481,7 @@  and its ancestor by overriding "repo._fi
   > from __future__ import absolute_import
   > from mercurial import commit, error, extensions, node
   > def _filecommit(orig, repo, fctx, manifest1, manifest2,
-  >                 linkrev, tr, includecopymeta):
+  >                 linkrev, tr, includecopymeta, ms=None):
   >     fname = fctx.path()
   >     text = fctx.data()
   >     flog = repo.file(fname)
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
@@ -484,7 +484,7 @@  and its ancestor by overriding "repo._fi
   > from __future__ import absolute_import
   > from mercurial import commit, error, extensions, node
   > def _filecommit(orig, repo, fctx, manifest1, manifest2,
-  >                 linkrev, tr, includecopymeta):
+  >                 linkrev, tr, includecopymeta, ms=None):
   >     fname = fctx.path()
   >     text = fctx.data()
   >     flog = repo.file(fname)