Patchwork [2,of,9,V2] merge: disable `m2-vs-ma` optimization if new filenode config is true

login
register
mail settings
Submitter Pulkit Goyal
Date Sept. 25, 2020, 9:16 a.m.
Message ID <0fa4c552709af5063f1a.1601025411@workspace>
Download mbox | patch
Permalink /patch/47277/
State Accepted
Headers show

Comments

Pulkit Goyal - Sept. 25, 2020, 9:16 a.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1600074612 -19800
#      Mon Sep 14 14:40:12 2020 +0530
# Node ID 0fa4c552709af5063f1a07630e64785048ab3735
# Parent  2459a55b887107613818e0d968ef80748dafaadb
# EXP-Topic merge-newnode
merge: disable `m2-vs-ma` optimization if new filenode config is true

The `m2-vs-ma` optimization filters out the file which have not changed between
second parent and the ancestor of the merge. This results in the m1-vs-m2 diff
not processing those files. These files will be required when we are creating
new filenode for salvaged cases as we need to track them to store in mergestate
that file can be salvaged.

Differential Revision: https://phab.mercurial-scm.org/D9027
Yuya Nishihara - Sept. 25, 2020, 11:34 a.m.
On Fri, 25 Sep 2020 14:46:51 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1600074612 -19800
> #      Mon Sep 14 14:40:12 2020 +0530
> # Node ID 0fa4c552709af5063f1a07630e64785048ab3735
> # Parent  2459a55b887107613818e0d968ef80748dafaadb
> # EXP-Topic merge-newnode
> merge: disable `m2-vs-ma` optimization if new filenode config is true
> 
> The `m2-vs-ma` optimization filters out the file which have not changed between
> second parent and the ancestor of the merge. This results in the m1-vs-m2 diff
> not processing those files. These files will be required when we are creating
> new filenode for salvaged cases as we need to track them to store in mergestate
> that file can be salvaged.
> 
> Differential Revision: https://phab.mercurial-scm.org/D9027
> 
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -776,7 +776,13 @@ def manifestmerge(
>      # - ma is the same as m1 or m2, which we're just going to diff again later
>      # - The caller specifically asks for a full diff, which is useful during bid
>      #   merge.
> -    if pa not in ([wctx, p2] + wctx.parents()) and not forcefulldiff:
> +    # - we are tracking salvaged files specifically hence should process all
> +    #   files
> +    if (
> +        pa not in ([wctx, p2] + wctx.parents())
> +        and not forcefulldiff
> +        and repo.ui.configbool(b'experimental', b'merge-track-salvaged')
> +    ):

Okay, this breaks many tests. Maybe it should be:

  and not (forcefulldiff or merge-track-salvaged)

I've dropped the series. Please send V3.
Pulkit Goyal - Sept. 25, 2020, 11:56 a.m.
On Fri, Sep 25, 2020 at 5:05 PM Yuya Nishihara <yuya@tcha.org> wrote:
>
> On Fri, 25 Sep 2020 14:46:51 +0530, Pulkit Goyal wrote:
> > # HG changeset patch
> > # User Pulkit Goyal <7895pulkit@gmail.com>
> > # Date 1600074612 -19800
> > #      Mon Sep 14 14:40:12 2020 +0530
> > # Node ID 0fa4c552709af5063f1a07630e64785048ab3735
> > # Parent  2459a55b887107613818e0d968ef80748dafaadb
> > # EXP-Topic merge-newnode
> > merge: disable `m2-vs-ma` optimization if new filenode config is true
> >
> > The `m2-vs-ma` optimization filters out the file which have not changed between
> > second parent and the ancestor of the merge. This results in the m1-vs-m2 diff
> > not processing those files. These files will be required when we are creating
> > new filenode for salvaged cases as we need to track them to store in mergestate
> > that file can be salvaged.
> >
> > Differential Revision: https://phab.mercurial-scm.org/D9027
> >
> > diff --git a/mercurial/merge.py b/mercurial/merge.py
> > --- a/mercurial/merge.py
> > +++ b/mercurial/merge.py
> > @@ -776,7 +776,13 @@ def manifestmerge(
> >      # - ma is the same as m1 or m2, which we're just going to diff again later
> >      # - The caller specifically asks for a full diff, which is useful during bid
> >      #   merge.
> > -    if pa not in ([wctx, p2] + wctx.parents()) and not forcefulldiff:
> > +    # - we are tracking salvaged files specifically hence should process all
> > +    #   files
> > +    if (
> > +        pa not in ([wctx, p2] + wctx.parents())
> > +        and not forcefulldiff
> > +        and repo.ui.configbool(b'experimental', b'merge-track-salvaged')
> > +    ):
>
> Okay, this breaks many tests. Maybe it should be:
>
>   and not (forcefulldiff or merge-track-salvaged)

Yes!
>
> I've dropped the series. Please send V3.

Oops, sorry about that. I failed to detect that because of a commit
which fixes the tests in WIP stack.

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -776,7 +776,13 @@  def manifestmerge(
     # - ma is the same as m1 or m2, which we're just going to diff again later
     # - The caller specifically asks for a full diff, which is useful during bid
     #   merge.
-    if pa not in ([wctx, p2] + wctx.parents()) and not forcefulldiff:
+    # - we are tracking salvaged files specifically hence should process all
+    #   files
+    if (
+        pa not in ([wctx, p2] + wctx.parents())
+        and not forcefulldiff
+        and repo.ui.configbool(b'experimental', b'merge-track-salvaged')
+    ):
         # Identify which files are relevant to the merge, so we can limit the
         # total m1-vs-m2 diff to just those files. This has significant
         # performance benefits in large repositories.