Patchwork [5,of,5] merge.applyupdates: only attempt to merge files in mergeactions

login
register
mail settings
Submitter Siddharth Agarwal
Date Nov. 12, 2015, 10:29 p.m.
Message ID <7ef8c6fd040c8ff0efd4.1447367374@dev666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11386/
State Accepted
Headers show

Comments

Siddharth Agarwal - Nov. 12, 2015, 10:29 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1447367342 28800
#      Thu Nov 12 14:29:02 2015 -0800
# Node ID 7ef8c6fd040c8ff0efd49755ebbdc8e16ca50e47
# Parent  bf354f5a168277cf493fc7c95d74d91c6d23447f
merge.applyupdates: only attempt to merge files in mergeactions

This only makes a difference when a merge driver is active -- in that case we
don't want to try and merge all the files, just the ones still unresolved after
the merge driver's preprocess step is over.
Martin von Zweigbergk - Nov. 13, 2015, 1:44 a.m.
On Thu, Nov 12, 2015 at 2:33 PM Siddharth Agarwal <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1447367342 28800
> #      Thu Nov 12 14:29:02 2015 -0800
> # Node ID 7ef8c6fd040c8ff0efd49755ebbdc8e16ca50e47
> # Parent  bf354f5a168277cf493fc7c95d74d91c6d23447f
> merge.applyupdates: only attempt to merge files in mergeactions
>
> This only makes a difference when a merge driver is active -- in that case
> we
> don't want to try and merge all the files, just the ones still unresolved
> after
> the merge driver's preprocess step is over.
>

What would be the consequence of merging all the files? Would they be
merged the same way as before, so it's just a waste of time? If not, is it
worth testing this case and/or applying this to stable?


>
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -970,7 +970,7 @@ def applyupdates(repo, actions, wctx, mc
>
>      # premerge
>      tocomplete = []
> -    for f, args, msg in actions['m']:
> +    for f, args, msg in mergeactions:
>          repo.ui.debug(" %s: %s -> m (premerge)\n" % (f, msg))
>          z += 1
>          progress(_updating, z, item=f, total=numupdates, unit=_files)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
Siddharth Agarwal - Nov. 13, 2015, 3:22 a.m.
On 11/12/15 17:44, Martin von Zweigbergk wrote:
>
>
> On Thu, Nov 12, 2015 at 2:33 PM Siddharth Agarwal <sid0@fb.com 
> <mailto:sid0@fb.com>> wrote:
>
>     # HG changeset patch
>     # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>>
>     # Date 1447367342 28800
>     #      Thu Nov 12 14:29:02 2015 -0800
>     # Node ID 7ef8c6fd040c8ff0efd49755ebbdc8e16ca50e47
>     # Parent  bf354f5a168277cf493fc7c95d74d91c6d23447f
>     merge.applyupdates: only attempt to merge files in mergeactions
>
>     This only makes a difference when a merge driver is active -- in
>     that case we
>     don't want to try and merge all the files, just the ones still
>     unresolved after
>     the merge driver's preprocess step is over.
>
>
> What would be the consequence of merging all the files? Would they be 
> merged the same way as before, so it's just a waste of time? If not, 
> is it worth testing this case and/or applying this to stable?

Yeah, it's just a waste of time. (Plus, this change leads nicely to some 
upcoming patches that I have.)

The merge driver stuff is still experimental, so this has no 
user-visible impact in supported features. This shouldn't go on stable.

>
>     diff --git a/mercurial/merge.py b/mercurial/merge.py
>     --- a/mercurial/merge.py
>     +++ b/mercurial/merge.py
>     @@ -970,7 +970,7 @@ def applyupdates(repo, actions, wctx, mc
>
>          # premerge
>          tocomplete = []
>     -    for f, args, msg in actions['m']:
>     +    for f, args, msg in mergeactions:
>              repo.ui.debug(" %s: %s -> m (premerge)\n" % (f, msg))
>              z += 1
>              progress(_updating, z, item=f, total=numupdates, unit=_files)
>     _______________________________________________
>     Mercurial-devel mailing list
>     Mercurial-devel@selenic.com <mailto:Mercurial-devel@selenic.com>
>     https://selenic.com/mailman/listinfo/mercurial-devel
>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Martin von Zweigbergk - Nov. 13, 2015, 6:24 a.m.
On Thu, Nov 12, 2015 at 7:22 PM Siddharth Agarwal <sid@less-broken.com>
wrote:

> On 11/12/15 17:44, Martin von Zweigbergk wrote:
> >
> >
> > On Thu, Nov 12, 2015 at 2:33 PM Siddharth Agarwal <sid0@fb.com
> > <mailto:sid0@fb.com>> wrote:
> >
> >     # HG changeset patch
> >     # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>>
> >     # Date 1447367342 28800
> >     #      Thu Nov 12 14:29:02 2015 -0800
> >     # Node ID 7ef8c6fd040c8ff0efd49755ebbdc8e16ca50e47
> >     # Parent  bf354f5a168277cf493fc7c95d74d91c6d23447f
> >     merge.applyupdates: only attempt to merge files in mergeactions
> >
> >     This only makes a difference when a merge driver is active -- in
> >     that case we
> >     don't want to try and merge all the files, just the ones still
> >     unresolved after
> >     the merge driver's preprocess step is over.
> >
> >
> > What would be the consequence of merging all the files? Would they be
> > merged the same way as before, so it's just a waste of time? If not,
> > is it worth testing this case and/or applying this to stable?
>
> Yeah, it's just a waste of time. (Plus, this change leads nicely to some
> upcoming patches that I have.)
>
> The merge driver stuff is still experimental, so this has no
> user-visible impact in supported features. This shouldn't go on stable.
>

And no user-visible impact in unsupported features either, if I interpret
the paragraph above right :-)

Besides the HGMERGE patches that still are a mystery to me, this series
makes sense, so I'll push these 5 to the clowncopter once tests have
finished running.


>
> >
> >     diff --git a/mercurial/merge.py b/mercurial/merge.py
> >     --- a/mercurial/merge.py
> >     +++ b/mercurial/merge.py
> >     @@ -970,7 +970,7 @@ def applyupdates(repo, actions, wctx, mc
> >
> >          # premerge
> >          tocomplete = []
> >     -    for f, args, msg in actions['m']:
> >     +    for f, args, msg in mergeactions:
> >              repo.ui.debug(" %s: %s -> m (premerge)\n" % (f, msg))
> >              z += 1
> >              progress(_updating, z, item=f, total=numupdates,
> unit=_files)
> >     _______________________________________________
> >     Mercurial-devel mailing list
> >     Mercurial-devel@selenic.com <mailto:Mercurial-devel@selenic.com>
> >     https://selenic.com/mailman/listinfo/mercurial-devel
> >
> >
> >
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@selenic.com
> > https://selenic.com/mailman/listinfo/mercurial-devel
>
>
Martin von Zweigbergk - Nov. 13, 2015, 7:04 a.m.
On Thu, Nov 12, 2015 at 10:24 PM Martin von Zweigbergk <
martinvonz@google.com> wrote:

> On Thu, Nov 12, 2015 at 7:22 PM Siddharth Agarwal <sid@less-broken.com>
> wrote:
>
>> On 11/12/15 17:44, Martin von Zweigbergk wrote:
>> >
>> >
>> > On Thu, Nov 12, 2015 at 2:33 PM Siddharth Agarwal <sid0@fb.com
>> > <mailto:sid0@fb.com>> wrote:
>> >
>> >     # HG changeset patch
>> >     # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>>
>> >     # Date 1447367342 28800
>> >     #      Thu Nov 12 14:29:02 2015 -0800
>> >     # Node ID 7ef8c6fd040c8ff0efd49755ebbdc8e16ca50e47
>> >     # Parent  bf354f5a168277cf493fc7c95d74d91c6d23447f
>> >     merge.applyupdates: only attempt to merge files in mergeactions
>> >
>> >     This only makes a difference when a merge driver is active -- in
>> >     that case we
>> >     don't want to try and merge all the files, just the ones still
>> >     unresolved after
>> >     the merge driver's preprocess step is over.
>> >
>> >
>> > What would be the consequence of merging all the files? Would they be
>> > merged the same way as before, so it's just a waste of time? If not,
>> > is it worth testing this case and/or applying this to stable?
>>
>> Yeah, it's just a waste of time. (Plus, this change leads nicely to some
>> upcoming patches that I have.)
>>
>> The merge driver stuff is still experimental, so this has no
>> user-visible impact in supported features. This shouldn't go on stable.
>>
>
> And no user-visible impact in unsupported features either, if I interpret
> the paragraph above right :-)
>
> Besides the HGMERGE patches that still are a mystery to me, this series
> makes sense, so I'll push these 5 to the clowncopter once tests have
> finished running.
>

After patch 3/5, test-commit-amend.t seems to hang indefinitely. I suspect
it's waiting for some mergetool (I'm logged in without X forwarding). I'll
drop both of those patches for now until we understand better what happened.


>
>
>>
>> >
>> >     diff --git a/mercurial/merge.py b/mercurial/merge.py
>> >     --- a/mercurial/merge.py
>> >     +++ b/mercurial/merge.py
>> >     @@ -970,7 +970,7 @@ def applyupdates(repo, actions, wctx, mc
>> >
>> >          # premerge
>> >          tocomplete = []
>> >     -    for f, args, msg in actions['m']:
>> >     +    for f, args, msg in mergeactions:
>> >              repo.ui.debug(" %s: %s -> m (premerge)\n" % (f, msg))
>> >              z += 1
>> >              progress(_updating, z, item=f, total=numupdates,
>> unit=_files)
>> >     _______________________________________________
>> >     Mercurial-devel mailing list
>> >     Mercurial-devel@selenic.com <mailto:Mercurial-devel@selenic.com>
>> >     https://selenic.com/mailman/listinfo/mercurial-devel
>> >
>> >
>> >
>> > _______________________________________________
>> > Mercurial-devel mailing list
>> > Mercurial-devel@selenic.com
>> > https://selenic.com/mailman/listinfo/mercurial-devel
>>
>>
Siddharth Agarwal - Nov. 13, 2015, 8:46 a.m.
On 11/12/15 23:04, Martin von Zweigbergk wrote:
>
> After patch 3/5, test-commit-amend.t seems to hang indefinitely. I 
> suspect it's waiting for some mergetool (I'm logged in without X 
> forwarding). I'll drop both of those patches for now until we 
> understand better what happened.

Huh, sorry about that. Do you by any chance have a binary called 
'hgmerge' in your PATH?
Martin von Zweigbergk - Nov. 13, 2015, 4:42 p.m.
On Fri, Nov 13, 2015 at 12:46 AM Siddharth Agarwal <sid@less-broken.com>
wrote:

> On 11/12/15 23:04, Martin von Zweigbergk wrote:
> >
> > After patch 3/5, test-commit-amend.t seems to hang indefinitely. I
> > suspect it's waiting for some mergetool (I'm logged in without X
> > forwarding). I'll drop both of those patches for now until we
> > understand better what happened.
>
> Huh, sorry about that. Do you by any chance have a binary called
> 'hgmerge' in your PATH?
>

Yep, turns out I do.

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -970,7 +970,7 @@  def applyupdates(repo, actions, wctx, mc
 
     # premerge
     tocomplete = []
-    for f, args, msg in actions['m']:
+    for f, args, msg in mergeactions:
         repo.ui.debug(" %s: %s -> m (premerge)\n" % (f, msg))
         z += 1
         progress(_updating, z, item=f, total=numupdates, unit=_files)