Patchwork [4,of,6,V2] merge: include obsolete calculation for branchtips

login
register
mail settings
Submitter Sean Farley
Date Jan. 12, 2014, 10:25 p.m.
Message ID <b97f7e3553f1694abd36.1389565552@laptop.local>
Download mbox | patch
Permalink /patch/3312/
State Superseded
Headers show

Comments

Sean Farley - Jan. 12, 2014, 10:25 p.m.
# HG changeset patch
# User Sean Farley <sean.michael.farley@gmail.com>
# Date 1383779296 21600
#      Wed Nov 06 17:08:16 2013 -0600
# Node ID b97f7e3553f1694abd360e30c0e4a9eac59f393a
# Parent  460212c85977dbeec5007558fa26293a4a0d11f4
merge: include obsolete calculation for branchtips

Previously, a bare update would ignore any successor changesets thus
potentially leaving you on an obsolete head. This happens commonly when there
is an old bookmark that hasn't been moved forward which is the motivating
reason for this patch series.

Now, we will check for successor changesets if two conditions hold:
  1) we are doing a bare update
  2) *and* the branchtip returned normally is obsolete

If we are in this situation, then we calculate the branchtip of the successor
set and update to that changeset.

In addition, we also preserves the no-op update clause added by Siddharth
Agarwal in ab2362e1672e by checking that the successor set is not the same as
set(p1).

Tests will be updated in the next patch because it requires a non-obvious fix
to the test case.
Pierre-Yves David - Jan. 13, 2014, 9:42 a.m.
On 01/12/2014 02:25 PM, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1383779296 21600
> #      Wed Nov 06 17:08:16 2013 -0600
> # Node ID b97f7e3553f1694abd360e30c0e4a9eac59f393a
> # Parent  460212c85977dbeec5007558fa26293a4a0d11f4
> merge: include obsolete calculation for branchtips
>
> Previously, a bare update would ignore any successor changesets thus
> potentially leaving you on an obsolete head. This happens commonly when there
> is an old bookmark that hasn't been moved forward which is the motivating
> reason for this patch series.
>
> Now, we will check for successor changesets if two conditions hold:
>    1) we are doing a bare update
>    2) *and* the branchtip returned normally is obsolete

This is a slghlty different version of what we discussed in New-York.

2) should be "if current working directory parent is a head and obsolete"

If update can move up along standard topology it should alway just do it.

>
> If we are in this situation, then we calculate the branchtip of the successor
> set and update to that changeset.
>
> In addition, we also preserves the no-op update clause added by Siddharth
> Agarwal in ab2362e1672e by checking that the successor set is not the same as
> set(p1).
>
> Tests will be updated in the next patch because it requires a non-obvious fix
> to the test case.
>
> diff --git a/mercurial/merge.py b/mercurial/merge.py

Code sound fine from afar. I need to apply the patch and have a deeper look.
Pierre-Yves David - Jan. 13, 2014, 9:52 a.m.
On 01/12/2014 02:25 PM, Sean Farley wrote:
> Tests will be updated in the next patch because it requires a non-obvious fix
> to the test case.
Do you mean test are failing? or just that some of the changes in this 
commit are not tested ?

Why can't you put them in this changeset ?
Sean Farley - Jan. 13, 2014, 5:14 p.m.
pierre-yves.david@ens-lyon.org writes:

> On 01/12/2014 02:25 PM, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley@gmail.com>
>> # Date 1383779296 21600
>> #      Wed Nov 06 17:08:16 2013 -0600
>> # Node ID b97f7e3553f1694abd360e30c0e4a9eac59f393a
>> # Parent  460212c85977dbeec5007558fa26293a4a0d11f4
>> merge: include obsolete calculation for branchtips
>>
>> Previously, a bare update would ignore any successor changesets thus
>> potentially leaving you on an obsolete head. This happens commonly when there
>> is an old bookmark that hasn't been moved forward which is the motivating
>> reason for this patch series.
>>
>> Now, we will check for successor changesets if two conditions hold:
>>    1) we are doing a bare update
>>    2) *and* the branchtip returned normally is obsolete
>
> This is a slghlty different version of what we discussed in New-York.
>
> 2) should be "if current working directory parent is a head and obsolete"

Hmm, then I was probably confused.

> If update can move up along standard topology it should alway just do it.

Then should a bare update from an obsolete head go to the tip of just the
successors or the branchtip of the foreground set?

>> If we are in this situation, then we calculate the branchtip of the successor
>> set and update to that changeset.
>>
>> In addition, we also preserves the no-op update clause added by Siddharth
>> Agarwal in ab2362e1672e by checking that the successor set is not the same as
>> set(p1).
>>
>> Tests will be updated in the next patch because it requires a non-obvious fix
>> to the test case.
>>
>> diff --git a/mercurial/merge.py b/mercurial/merge.py
>
> Code sound fine from afar. I need to apply the patch and have a deeper look.
Sean Farley - Jan. 13, 2014, 5:16 p.m.
pierre-yves.david@ens-lyon.org writes:

> On 01/12/2014 02:25 PM, Sean Farley wrote:
>> Tests will be updated in the next patch because it requires a non-obvious fix
>> to the test case.
> Do you mean test are failing? or just that some of the changes in this 
> commit are not tested ?

I meant that the tests are currently wrong and that this patch fixes the
wrongness of the tests but that the change required should go into
another patch.

> Why can't you put them in this changeset ?

They could be folded into one patch if that's what people want. I
originally thought it'd be easier to review.
Pierre-Yves David - Jan. 14, 2014, 9:27 p.m.
On 01/13/2014 01:42 AM, Pierre-Yves David wrote:
> On 01/12/2014 02:25 PM, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley<sean.michael.farley@gmail.com>
>> # Date 1383779296 21600
>> #      Wed Nov 06 17:08:16 2013 -0600
>> # Node ID b97f7e3553f1694abd360e30c0e4a9eac59f393a
>> # Parent  460212c85977dbeec5007558fa26293a4a0d11f4
>> merge: include obsolete calculation for branchtips
>>
>> Previously, a bare update would ignore any successor changesets thus
>> potentially leaving you on an obsolete head. This happens commonly when there
>> is an old bookmark that hasn't been moved forward which is the motivating
>> reason for this patch series.
>>
>> Now, we will check for successor changesets if two conditions hold:
>>    1) we are doing a bare update
>>    2) *and* the branchtip returned normally is obsolete
>
> This is a slghlty different version of what we discussed in New-York.
>
> 2) should be "if current working directory parent is a head and obsolete"
>
> If update can move up along standard topology it should alway just do it.

This is actually even more different, This patch update to tipmost 
foreground. You want to update to tip most successors set in that case.
Sean Farley - Jan. 14, 2014, 9:51 p.m.
pierre-yves.david@ens-lyon.org writes:

> On 01/13/2014 01:42 AM, Pierre-Yves David wrote:
>> On 01/12/2014 02:25 PM, Sean Farley wrote:
>>> # HG changeset patch
>>> # User Sean Farley<sean.michael.farley@gmail.com>
>>> # Date 1383779296 21600
>>> #      Wed Nov 06 17:08:16 2013 -0600
>>> # Node ID b97f7e3553f1694abd360e30c0e4a9eac59f393a
>>> # Parent  460212c85977dbeec5007558fa26293a4a0d11f4
>>> merge: include obsolete calculation for branchtips
>>>
>>> Previously, a bare update would ignore any successor changesets thus
>>> potentially leaving you on an obsolete head. This happens commonly when there
>>> is an old bookmark that hasn't been moved forward which is the motivating
>>> reason for this patch series.
>>>
>>> Now, we will check for successor changesets if two conditions hold:
>>>    1) we are doing a bare update
>>>    2) *and* the branchtip returned normally is obsolete
>>
>> This is a slghlty different version of what we discussed in New-York.
>>
>> 2) should be "if current working directory parent is a head and obsolete"
>>
>> If update can move up along standard topology it should alway just do it.
>
> This is actually even more different, This patch update to tipmost 
> foreground. You want to update to tip most successors set in that case.

Ok, that clarify things. I'll resend.
Pierre-Yves David - Jan. 14, 2014, 9:55 p.m.
On 01/14/2014 01:51 PM, Sean Farley wrote:
> pierre-yves.david@ens-lyon.org writes:
>
>> On 01/13/2014 01:42 AM, Pierre-Yves David wrote:
>>> On 01/12/2014 02:25 PM, Sean Farley wrote:
>>>> # HG changeset patch
>>>> # User Sean Farley<sean.michael.farley@gmail.com>
>>>> # Date 1383779296 21600
>>>> #      Wed Nov 06 17:08:16 2013 -0600
>>>> # Node ID b97f7e3553f1694abd360e30c0e4a9eac59f393a
>>>> # Parent  460212c85977dbeec5007558fa26293a4a0d11f4
>>>> merge: include obsolete calculation for branchtips
>>>>
>>>> Previously, a bare update would ignore any successor changesets thus
>>>> potentially leaving you on an obsolete head. This happens commonly when there
>>>> is an old bookmark that hasn't been moved forward which is the motivating
>>>> reason for this patch series.
>>>>
>>>> Now, we will check for successor changesets if two conditions hold:
>>>>     1) we are doing a bare update
>>>>     2) *and* the branchtip returned normally is obsolete
>>> This is a slghlty different version of what we discussed in New-York.
>>>
>>> 2) should be "if current working directory parent is a head and obsolete"
>>>
>>> If update can move up along standard topology it should alway just do it.
>> This is actually even more different, This patch update to tipmost
>> foreground. You want to update to tip most successors set in that case.
> Ok, that clarify things. I'll resend.

Final clarification of expected behavior.

If changeset have descendant:
     update to tipmost descendant
else (changeset is head) if changeset is obsolete:
     update to tipmost successors (not foreground, actual successors of 
this changeset)
else (no idea were to go)
     sit down and complain about the situation if possible
Sean Farley - Jan. 14, 2014, 10:18 p.m.
pierre-yves.david@ens-lyon.org writes:

> On 01/14/2014 01:51 PM, Sean Farley wrote:
>> pierre-yves.david@ens-lyon.org writes:
>>
>>> On 01/13/2014 01:42 AM, Pierre-Yves David wrote:
>>>> On 01/12/2014 02:25 PM, Sean Farley wrote:
>>>>> # HG changeset patch
>>>>> # User Sean Farley<sean.michael.farley@gmail.com>
>>>>> # Date 1383779296 21600
>>>>> #      Wed Nov 06 17:08:16 2013 -0600
>>>>> # Node ID b97f7e3553f1694abd360e30c0e4a9eac59f393a
>>>>> # Parent  460212c85977dbeec5007558fa26293a4a0d11f4
>>>>> merge: include obsolete calculation for branchtips
>>>>>
>>>>> Previously, a bare update would ignore any successor changesets thus
>>>>> potentially leaving you on an obsolete head. This happens commonly when there
>>>>> is an old bookmark that hasn't been moved forward which is the motivating
>>>>> reason for this patch series.
>>>>>
>>>>> Now, we will check for successor changesets if two conditions hold:
>>>>>     1) we are doing a bare update
>>>>>     2) *and* the branchtip returned normally is obsolete
>>>> This is a slghlty different version of what we discussed in New-York.
>>>>
>>>> 2) should be "if current working directory parent is a head and obsolete"
>>>>
>>>> If update can move up along standard topology it should alway just do it.
>>> This is actually even more different, This patch update to tipmost
>>> foreground. You want to update to tip most successors set in that case.
>> Ok, that clarify things. I'll resend.
>
> Final clarification of expected behavior.
>
> If changeset have descendant:
>      update to tipmost descendant
> else (changeset is head) if changeset is obsolete:
>      update to tipmost successors (not foreground, actual successors of 
> this changeset)
> else (no idea were to go)
>      sit down and complain about the situation if possible

Ok, I'll resend with this feedback.

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -704,10 +704,17 @@  def update(repo, node, branchmerge, forc
                 # Branching is a bit strange to ensure we do the minimal
                 # amount of call to obsolete.background.
                 foreground = obsolete.foreground(repo, [p1.node()])
                 # note: the <node> variable contains a random identifier
 
+                # allow updating to successors
+                if repo[node].node() in foreground:
+                    # get the max revision for the given foreground set
+                    unfil = [repo.unfiltered()[n].rev() for n in foreground]
+                    node = repo[max(unfil)].node()
+                    pa = p1
+
         overwrite = force and not branchmerge
 
         p2 = repo[node]
         if pa is None:
             pa = p1.ancestor(p2)
@@ -733,11 +740,13 @@  def update(repo, node, branchmerge, forc
                 if wc.sub(s).dirty():
                     raise util.Abort(_("uncommitted changes in "
                                        "subrepository '%s'") % s)
 
         elif not overwrite:
-            if p1 == p2: # no-op update
+            # no-op update
+            if (p1 == p2 and
+                not (foreground and foreground != set([p1.node()]))):
                 # call the hooks and exit early
                 repo.hook('preupdate', throw=True, parent1=xp2, parent2='')
                 repo.hook('update', parent1=xp2, parent2='', error=0)
                 return 0, 0, 0, 0