Patchwork [STABLE] merge: avoid superfluous filemerges when grafting through renames (issue5407)

login
register
mail settings
Submitter Gábor Stefanik
Date Oct. 25, 2016, 10:21 p.m.
Message ID <1f40c1fe34442812291b.1477434084@waste.org>
Download mbox | patch
Permalink /patch/17195/
State Accepted
Headers show

Comments

Gábor Stefanik - Oct. 25, 2016, 10:21 p.m.
# HG changeset patch
# User Gábor Stefanik <gabor.stefanik@nng.com>
# Date 1477422113 -7200
#      Tue Oct 25 21:01:53 2016 +0200
# Branch stable
# Node ID 1f40c1fe34442812291b000c5e1a8b22ad14f091
# Parent  76c57e1fe79b0980b377b4f305635dea393d6315
merge: avoid superfluous filemerges when grafting through renames (issue5407)

This is a fix for a regression introduced by the patches for issue4028.

The test changes are due to us doing fewer _checkcopies searches now, which
makes some test outputs revert to the pre-issue4028 behavior. That issue itself
remains fixed, we only skip copy tracing for files where it isn't relevant.
As a nice side effect, this makes copy detection much faster when tracing
backwards through lots of renames.
Gábor Stefanik - Oct. 26, 2016, 11:05 a.m.
>



--------------------------------------------------------------------------
This message, including its attachments, is confidential. For more information please read NNG's email policy here:
http://www.nng.com/emailpolicy/
By responding to this email you accept the email policy.


-----Original Message-----
> From: Mercurial-devel [mailto:mercurial-devel-bounces@mercurial-scm.org]

> On Behalf Of Gábor Stefanik

> Sent: Wednesday, October 26, 2016 12:21 AM

> To: mercurial-devel@mercurial-scm.org

> Subject: [PATCH STABLE] merge: avoid superfluous filemerges when grafting

> through renames (issue5407)

>

> # HG changeset patch

> # User Gábor Stefanik <gabor.stefanik@nng.com> # Date 1477422113 -7200

> #      Tue Oct 25 21:01:53 2016 +0200

> # Branch stable

> # Node ID 1f40c1fe34442812291b000c5e1a8b22ad14f091

> # Parent  76c57e1fe79b0980b377b4f305635dea393d6315

> merge: avoid superfluous filemerges when grafting through renames

> (issue5407)

>

> This is a fix for a regression introduced by the patches for issue4028.

>

> The test changes are due to us doing fewer _checkcopies searches now,

> which makes some test outputs revert to the pre-issue4028 behavior. That

> issue itself remains fixed, we only skip copy tracing for files where it isn't

> relevant.

> As a nice side effect, this makes copy detection much faster when tracing

> backwards through lots of renames.

>

> diff --git a/mercurial/copies.py b/mercurial/copies.py

> --- a/mercurial/copies.py

> +++ b/mercurial/copies.py

> @@ -631,6 +631,10 @@

>      backwards = not remotebase and base != tca and f in mb

>      getfctx = _makegetfctx(ctx)

>

> +    if m1[f] == mb.get(f) and not remotebase:


Note: this remotebase check is probably not needed, and removing it actually speeds
things up further. However, it also causes additional debug output changes in tests
(i.e. more output reverts to pre-4028 behavior), so I decided to keep it in for stable.

Let me know if it's OK to just remove it and adjust the tests.

After 4.0, if we remove the remotebase check, we can possibly get rid of or simplify
the "ping-pong" rename handling code.

> +        # Nothing to merge

> +        return

> +

>      of = None

>      seen = set([f])

>      for oc in getfctx(f, m1[f]).ancestors():

> diff --git a/tests/test-graft.t b/tests/test-graft.t

> --- a/tests/test-graft.t

> +++ b/tests/test-graft.t

> @@ -181,9 +181,6 @@

>      searching for copies back to rev 1

>      unmatched files in other (from topological common ancestor):

>       c

> -    all copies found (* = to merge, ! = divergent, % = renamed and deleted):

> -     src: 'c' -> dst: 'b' *

> -    checking for directory renames

>    resolving manifests

>     branchmerge: True, force: True, partial: False

>     ancestor: 4c60f11aa304, local: 6b9e5368ca4e+, remote: 97f8bfe72746 @@ -

> 200,9 +197,6 @@

>      searching for copies back to rev 1

>      unmatched files in other (from topological common ancestor):

>       c

> -    all copies found (* = to merge, ! = divergent, % = renamed and deleted):

> -     src: 'c' -> dst: 'b' *

> -    checking for directory renames

>    resolving manifests

>     branchmerge: True, force: True, partial: False

>     ancestor: 4c60f11aa304, local: 1905859650ec+, remote: 9c233e8e184d @@ -

> 1280,3 +1274,15 @@

>

>    $ hg cat f2c

>    c2e

> +

> +Check superfluous filemerge of files renamed in the past but untouched

> +by graft

> +

> +  $ echo a > a

> +  $ hg ci -qAma

> +  $ hg mv a b

> +  $ echo b > b

> +  $ hg ci -qAmb

> +  $ echo c > c

> +  $ hg ci -qAmc

> +  $ hg up -q .~2

> +  $ hg graft tip -qt:fail

> diff --git a/tests/test-merge-local.t b/tests/test-merge-local.t

> --- a/tests/test-merge-local.t

> +++ b/tests/test-merge-local.t

> @@ -66,7 +66,7 @@

>    merging zzz1_merge_ok

>    merging zzz2_merge_bad

>    warning: conflicts while merging zzz2_merge_bad! (edit, then use 'hg

> resolve --mark')

> -  2 files updated, 1 files merged, 2 files removed, 1 files unresolved

> +  2 files updated, 1 files merged, 3 files removed, 1 files unresolved

>    use 'hg resolve' to retry unresolved file merges

>    [1]

>

> @@ -104,7 +104,7 @@

>    merging zzz1_merge_ok

>    merging zzz2_merge_bad

>    warning: conflicts while merging zzz2_merge_bad! (edit, then use 'hg

> resolve --mark')

> -  2 files updated, 1 files merged, 2 files removed, 1 files unresolved

> +  2 files updated, 1 files merged, 3 files removed, 1 files unresolved

>    use 'hg resolve' to retry unresolved file merges

>    [1]

>

> diff --git a/tests/test-up-local-change.t b/tests/test-up-local-change.t

> --- a/tests/test-up-local-change.t

> +++ b/tests/test-up-local-change.t

> @@ -242,4 +242,11 @@

>    -a

>    +b

>

> +test for superfluous filemerge of clean files renamed in the past

> +

> +  $ hg up -qC tip

> +  $ echo c > c

> +  $ hg add c

> +  $ hg up -qt:fail 0

> +

>    $ cd ..

> _______________________________________________

> Mercurial-devel mailing list

> Mercurial-devel@mercurial-scm.org

> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - Oct. 27, 2016, 1:24 p.m.
On Tue, 25 Oct 2016 17:21:24 -0500, Gábor Stefanik wrote:
> # HG changeset patch
> # User Gábor Stefanik <gabor.stefanik@nng.com>
> # Date 1477422113 -7200
> #      Tue Oct 25 21:01:53 2016 +0200
> # Branch stable
> # Node ID 1f40c1fe34442812291b000c5e1a8b22ad14f091
> # Parent  76c57e1fe79b0980b377b4f305635dea393d6315
> merge: avoid superfluous filemerges when grafting through renames (issue5407)
> 
> This is a fix for a regression introduced by the patches for issue4028.
> 
> The test changes are due to us doing fewer _checkcopies searches now, which
> makes some test outputs revert to the pre-issue4028 behavior. That issue itself
> remains fixed, we only skip copy tracing for files where it isn't relevant.
> As a nice side effect, this makes copy detection much faster when tracing
> backwards through lots of renames.
> 
> diff --git a/mercurial/copies.py b/mercurial/copies.py
> --- a/mercurial/copies.py
> +++ b/mercurial/copies.py
> @@ -631,6 +631,10 @@
>      backwards = not remotebase and base != tca and f in mb
>      getfctx = _makegetfctx(ctx)
>  
> +    if m1[f] == mb.get(f) and not remotebase:
> +        # Nothing to merge
> +        return

I'm not 100% sure, but this change looks good and I have no particular concern.
Queued, thanks.

I expect this will be 2nd-reviewed by marmoute as he's reviewed the previous
round of the mergecopies series.

Patch

diff --git a/mercurial/copies.py b/mercurial/copies.py
--- a/mercurial/copies.py
+++ b/mercurial/copies.py
@@ -631,6 +631,10 @@ 
     backwards = not remotebase and base != tca and f in mb
     getfctx = _makegetfctx(ctx)
 
+    if m1[f] == mb.get(f) and not remotebase:
+        # Nothing to merge
+        return
+
     of = None
     seen = set([f])
     for oc in getfctx(f, m1[f]).ancestors():
diff --git a/tests/test-graft.t b/tests/test-graft.t
--- a/tests/test-graft.t
+++ b/tests/test-graft.t
@@ -181,9 +181,6 @@ 
     searching for copies back to rev 1
     unmatched files in other (from topological common ancestor):
      c
-    all copies found (* = to merge, ! = divergent, % = renamed and deleted):
-     src: 'c' -> dst: 'b' *
-    checking for directory renames
   resolving manifests
    branchmerge: True, force: True, partial: False
    ancestor: 4c60f11aa304, local: 6b9e5368ca4e+, remote: 97f8bfe72746
@@ -200,9 +197,6 @@ 
     searching for copies back to rev 1
     unmatched files in other (from topological common ancestor):
      c
-    all copies found (* = to merge, ! = divergent, % = renamed and deleted):
-     src: 'c' -> dst: 'b' *
-    checking for directory renames
   resolving manifests
    branchmerge: True, force: True, partial: False
    ancestor: 4c60f11aa304, local: 1905859650ec+, remote: 9c233e8e184d
@@ -1280,3 +1274,15 @@ 
   
   $ hg cat f2c
   c2e
+
+Check superfluous filemerge of files renamed in the past but untouched by graft
+
+  $ echo a > a
+  $ hg ci -qAma
+  $ hg mv a b
+  $ echo b > b
+  $ hg ci -qAmb
+  $ echo c > c
+  $ hg ci -qAmc
+  $ hg up -q .~2
+  $ hg graft tip -qt:fail
diff --git a/tests/test-merge-local.t b/tests/test-merge-local.t
--- a/tests/test-merge-local.t
+++ b/tests/test-merge-local.t
@@ -66,7 +66,7 @@ 
   merging zzz1_merge_ok
   merging zzz2_merge_bad
   warning: conflicts while merging zzz2_merge_bad! (edit, then use 'hg resolve --mark')
-  2 files updated, 1 files merged, 2 files removed, 1 files unresolved
+  2 files updated, 1 files merged, 3 files removed, 1 files unresolved
   use 'hg resolve' to retry unresolved file merges
   [1]
 
@@ -104,7 +104,7 @@ 
   merging zzz1_merge_ok
   merging zzz2_merge_bad
   warning: conflicts while merging zzz2_merge_bad! (edit, then use 'hg resolve --mark')
-  2 files updated, 1 files merged, 2 files removed, 1 files unresolved
+  2 files updated, 1 files merged, 3 files removed, 1 files unresolved
   use 'hg resolve' to retry unresolved file merges
   [1]
 
diff --git a/tests/test-up-local-change.t b/tests/test-up-local-change.t
--- a/tests/test-up-local-change.t
+++ b/tests/test-up-local-change.t
@@ -242,4 +242,11 @@ 
   -a
   +b
 
+test for superfluous filemerge of clean files renamed in the past
+
+  $ hg up -qC tip
+  $ echo c > c
+  $ hg add c
+  $ hg up -qt:fail 0
+
   $ cd ..