| 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
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.
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)