Patchwork [08,of,11] filecommit: reuse filenode matching fparents regardless of type(fctx)

login
register
mail settings
Submitter Jun Wu
Date May 10, 2017, 8:34 a.m.
Message ID <e4ad86c15b06986e51d4.1494405267@x1c>
Download mbox | patch
Permalink /patch/20554/
State Accepted
Headers show

Comments

Jun Wu - May 10, 2017, 8:34 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1494387835 25200
#      Tue May 09 20:43:55 2017 -0700
# Node ID e4ad86c15b06986e51d4754f3c11bd24cbae30bb
# Parent  eb7b96bc42d08670983d5d75635198f62c3459a8
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r e4ad86c15b06
filecommit: reuse filenode matching fparents regardless of type(fctx)

context.filectx is not the only class providing reusable filenode.
overlayfilectx may provide reusable filenode too. So drop the isinstance
check.

Other filectx classes (memfilectx and workingfilectx) are subclasses of
committablefilectx, which has set filenode to None. So filenode won't be
reused as expected.

The debug message is changed slightly to be distinguish from other kind of
reuses introduced in a future patch.
Yuya Nishihara - May 11, 2017, 3:15 p.m.
On Wed, 10 May 2017 01:34:27 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1494387835 25200
> #      Tue May 09 20:43:55 2017 -0700
> # Node ID e4ad86c15b06986e51d4754f3c11bd24cbae30bb
> # Parent  eb7b96bc42d08670983d5d75635198f62c3459a8
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r e4ad86c15b06
> filecommit: reuse filenode matching fparents regardless of type(fctx)
> 
> context.filectx is not the only class providing reusable filenode.
> overlayfilectx may provide reusable filenode too. So drop the isinstance
> check.
> 
> Other filectx classes (memfilectx and workingfilectx) are subclasses of
> committablefilectx, which has set filenode to None. So filenode won't be
> reused as expected.
> 
> The debug message is changed slightly to be distinguish from other kind of
> reuses introduced in a future patch.
> 
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1423,11 +1423,10 @@ class localrepository(object):
>          fparent1 = manifest1.get(fname, nullid)
>          fparent2 = manifest2.get(fname, nullid)
> -        if isinstance(fctx, context.filectx):
> -            node = fctx.filenode()
> -            if node in [fparent1, fparent2]:
> -                self.ui.debug('reusing %s filelog entry\n' % fname)
> -                if manifest1.flags(fname) != fctx.flags():
> -                    changelist.append(fname)
> -                return node
> +        node = getattr(fctx, 'filenode', lambda: None)()

Nit: it appears fctx.filenode() is a mandatory API of basefilectx.

I've double-reviewed up to this patch, and timed out. I'll look the remainder
tomorrow.
Jun Wu - May 11, 2017, 3:41 p.m.
Excerpts from Yuya Nishihara's message of 2017-05-12 00:15:41 +0900:
> Nit: it appears fctx.filenode() is a mandatory API of basefilectx.

Good catch. The only exception is tests/drawdag.py. I'll update it.

> I've double-reviewed up to this patch, and timed out. I'll look the
> remainder tomorrow.

Thanks!

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1423,11 +1423,10 @@  class localrepository(object):
         fparent1 = manifest1.get(fname, nullid)
         fparent2 = manifest2.get(fname, nullid)
-        if isinstance(fctx, context.filectx):
-            node = fctx.filenode()
-            if node in [fparent1, fparent2]:
-                self.ui.debug('reusing %s filelog entry\n' % fname)
-                if manifest1.flags(fname) != fctx.flags():
-                    changelist.append(fname)
-                return node
+        node = getattr(fctx, 'filenode', lambda: None)()
+        if node is not None and node in [fparent1, fparent2]:
+            self.ui.debug('reusing %s filelog node (parent match)\n' % fname)
+            if manifest1.flags(fname) != fctx.flags():
+                changelist.append(fname)
+            return node
 
         flog = self.file(fname)