Patchwork [3,of,4] merge: get the default update destination from the function

login
register
mail settings
Submitter Pierre-Yves David
Date Oct. 7, 2015, 7:02 p.m.
Message ID <224f781d36bdfa50e8db.1444244543@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/10864/
State Superseded
Commit c8b332b1eb1f84299cdf596e6a1f84c7e1a1cf8e
Headers show

Comments

Pierre-Yves David - Oct. 7, 2015, 7:02 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1443503483 25200
#      Mon Sep 28 22:11:23 2015 -0700
# Node ID 224f781d36bdfa50e8db7a3e02d5398848a88b20
# Parent  cb49c9ec614d20e21bc5affae1439a8abe727f5e
merge: get the default update destination from the function

There is no value in using the revset instead of the extracted function.
Augie Fackler - Oct. 8, 2015, 5:33 p.m.
On Wed, Oct 07, 2015 at 12:02:23PM -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1443503483 25200
> #      Mon Sep 28 22:11:23 2015 -0700
> # Node ID 224f781d36bdfa50e8db7a3e02d5398848a88b20
> # Parent  cb49c9ec614d20e21bc5affae1439a8abe727f5e
> merge: get the default update destination from the function
>
> There is no value in using the revset instead of the extracted function.
>

it seems there was: you just introduced an import cycle that you're
kludging around. Is it that important?

>
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -992,13 +992,12 @@ def update(repo, node, branchmerge, forc
>          pas = [None]
>          if ancestor is not None:
>              pas = [repo[ancestor]]
>
>          if node is None:
> -            nodes = list(repo.set('_updatedefaultdest()'))
> -            if nodes:
> -                node = nodes[0].node()
> +            from .scmutil import destupdate # avoid cycle
> +            node = repo[destupdate(repo)].node()
>
>          overwrite = force and not branchmerge
>
>          p2 = repo[node]
>          if pas[0] is None:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Oct. 8, 2015, 5:45 p.m.
On 10/08/2015 10:33 AM, Augie Fackler wrote:
> On Wed, Oct 07, 2015 at 12:02:23PM -0700, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1443503483 25200
>> #      Mon Sep 28 22:11:23 2015 -0700
>> # Node ID 224f781d36bdfa50e8db7a3e02d5398848a88b20
>> # Parent  cb49c9ec614d20e21bc5affae1439a8abe727f5e
>> merge: get the default update destination from the function
>>
>> There is no value in using the revset instead of the extracted function.
>>
>
> it seems there was: you just introduced an import cycle that you're
> kludging around. Is it that important?

Is it important to stop using the revset? Yes, revset can't return other 
value like "bookmark to be updated".

To be honest I forgot about this cycle business, I can think of another 
location an break it in a follow up if you want to.
Augie Fackler - Oct. 8, 2015, 6 p.m.
> On Oct 8, 2015, at 13:45, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
> 
> 
> 
> On 10/08/2015 10:33 AM, Augie Fackler wrote:
>> On Wed, Oct 07, 2015 at 12:02:23PM -0700, Pierre-Yves David wrote:
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>>> # Date 1443503483 25200
>>> #      Mon Sep 28 22:11:23 2015 -0700
>>> # Node ID 224f781d36bdfa50e8db7a3e02d5398848a88b20
>>> # Parent  cb49c9ec614d20e21bc5affae1439a8abe727f5e
>>> merge: get the default update destination from the function
>>> 
>>> There is no value in using the revset instead of the extracted function.
>>> 
>> 
>> it seems there was: you just introduced an import cycle that you're
>> kludging around. Is it that important?
> 
> Is it important to stop using the revset? Yes, revset can't return other value like "bookmark to be updated".
> 
> To be honest I forgot about this cycle business, I can think of another location an break it in a follow up if you want to.

I look forward to the resend of this series that doesn't introduce the cycle. Dropping patches 2:: for now.

> 
> -- 
> Pierre-Yves David

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -992,13 +992,12 @@  def update(repo, node, branchmerge, forc
         pas = [None]
         if ancestor is not None:
             pas = [repo[ancestor]]
 
         if node is None:
-            nodes = list(repo.set('_updatedefaultdest()'))
-            if nodes:
-                node = nodes[0].node()
+            from .scmutil import destupdate # avoid cycle
+            node = repo[destupdate(repo)].node()
 
         overwrite = force and not branchmerge
 
         p2 = repo[node]
         if pas[0] is None: