Patchwork bookmarks: resolve divergent bookmark when moving forward

login
register
mail settings
Submitter Sean Farley
Date March 27, 2013, 10:41 p.m.
Message ID <3839baf52f2f24c28948.1364424116@laptop.local>
Download mbox | patch
Permalink /patch/1201/
State Superseded, archived
Headers show

Comments

Sean Farley - March 27, 2013, 10:41 p.m.
# HG changeset patch
# User Sean Farley <sean.michael.farley@gmail.com>
# Date 1364421288 18000
#      Wed Mar 27 16:54:48 2013 -0500
# Node ID 3839baf52f2f24c289487111a95e9e835d1e1c4d
# Parent  a7d0ddc7540b2691f8a5af8006588a80e582754a
bookmarks: resolve divergent bookmark when moving forward

This patch handles both cases of moving a bookmark forward 1) when running 'hg
up' on a bookmark that has descendents, and 2) when running 'hg book NAME' on a
descendents of a the active bookmark.
Stephen Lee - March 28, 2013, 2:14 a.m.
On Thu, Mar 28, 2013 at 9:41 AM, Sean Farley
<sean.michael.farley@gmail.com> wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1364421288 18000
> #      Wed Mar 27 16:54:48 2013 -0500
> # Node ID 3839baf52f2f24c289487111a95e9e835d1e1c4d
> # Parent  a7d0ddc7540b2691f8a5af8006588a80e582754a
> bookmarks: resolve divergent bookmark when moving forward
>
> This patch handles both cases of moving a bookmark forward 1) when running 'hg
> up' on a bookmark that has descendents, and 2) when running 'hg book NAME' on a
> descendents of a the active bookmark.
>

What do you mean by "handling" these cases? What happens before and
after this patch?
I'm interested in making the bookmark workflow smoother - in what way
does this help?

Thanks,
Steve

> diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
> --- a/mercurial/bookmarks.py
> +++ b/mercurial/bookmarks.py
> @@ -182,11 +182,11 @@
>          new = repo[node]
>          if old.descendant(new):
>              marks[cur] = new.node()
>              update = True
>
> -    if deletedivergent(repo, parents, cur):
> +    if deletedivergent(repo, parents, cur) or deletedivergent(repo, node, cur):
>          update = True
>
>      if update:
>          marks.write()
>      return update
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -865,10 +865,11 @@
>          tgt = cur
>          if rev:
>              tgt = scmutil.revsingle(repo, rev).node()
>          checkconflict(repo, mark, force, tgt)
>          marks[mark] = tgt
> +        bookmarks.deletedivergent(repo, [tgt], mark)
>          if not inactive and cur == marks[mark]:
>              bookmarks.setcurrent(repo, mark)
>          elif cur != tgt and mark == repo._bookmarkcurrent:
>              bookmarks.setcurrent(repo, None)
>          marks.write()
Sean Farley - March 28, 2013, 3:38 a.m.
Stephen Lee writes:

> On Thu, Mar 28, 2013 at 9:41 AM, Sean Farley
> <sean.michael.farley@gmail.com> wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley@gmail.com>
>> # Date 1364421288 18000
>> #      Wed Mar 27 16:54:48 2013 -0500
>> # Node ID 3839baf52f2f24c289487111a95e9e835d1e1c4d
>> # Parent  a7d0ddc7540b2691f8a5af8006588a80e582754a
>> bookmarks: resolve divergent bookmark when moving forward
>>
>> This patch handles both cases of moving a bookmark forward 1) when running 'hg
>> up' on a bookmark that has descendents, and 2) when running 'hg book NAME' on a
>> descendents of a the active bookmark.
>>
>
> What do you mean by "handling" these cases? What happens before and
> after this patch?

Sorry; I meant to flag this as RFC so I could get some
feedback. Anyways, before this patch when there was a divergent
bookmark on a target changeset that you're moving a bookmark to, then
both bookmarks would be there. Example,

$ hg init test && cd test
$ echo a>a
$ hg ci -Am0
$ echo b>>a
$ hg ci -m1
$ hg book -r 0 @
$ hg book -r 1 @default
$ hg up @
$ hg up

You'll see that the divergent is still lying around when it should be
taken care of automatically. Applying this patch will fix that up but I
wanted to make sure other behavior wasn't affected.

> I'm interested in making the bookmark workflow smoother - in what way
> does this help?

Me too :-) This helps by resolving divergent bookmarks when the
resolution is trivial.
Sean Farley - March 28, 2013, 6:50 p.m.
Kevin Bullock writes:

> On 27 Mar 2013, at 10:38 PM, Sean Farley wrote:
>
>> Stephen Lee writes:
>> 
>>> On Thu, Mar 28, 2013 at 9:41 AM, Sean Farley
>>> <sean.michael.farley@gmail.com> wrote:
>>>> # HG changeset patch
>>>> # User Sean Farley <sean.michael.farley@gmail.com>
>>>> # Date 1364421288 18000
>>>> #      Wed Mar 27 16:54:48 2013 -0500
>>>> # Node ID 3839baf52f2f24c289487111a95e9e835d1e1c4d
>>>> # Parent  a7d0ddc7540b2691f8a5af8006588a80e582754a
>>>> bookmarks: resolve divergent bookmark when moving forward
>>>> 
>>>> This patch handles both cases of moving a bookmark forward 1) when running 'hg
>>>> up' on a bookmark that has descendents, and 2) when running 'hg book NAME' on a
>>>> descendents of a the active bookmark.
>>>> 
>>> 
>>> What do you mean by "handling" these cases? What happens before and
>>> after this patch?
>> 
>> Sorry; I meant to flag this as RFC so I could get some
>> feedback. Anyways, before this patch when there was a divergent
>> bookmark on a target changeset that you're moving a bookmark to, then
>> both bookmarks would be there. Example,
>> 
>> $ hg init test && cd test
>> $ echo a>a
>> $ hg ci -Am0
>> $ echo b>>a
>> $ hg ci -m1
>> $ hg book -r 0 @
>> $ hg book -r 1 @default
>> $ hg up @
>> $ hg up
>> 
>> You'll see that the divergent is still lying around when it should be
>> taken care of automatically. Applying this patch will fix that up but I
>> wanted to make sure other behavior wasn't affected.
>
> Normally this shouldn't happen in a real-life situation, because no divergent bookmarks are ever _created_ unless they actually... diverge. As in, they're on different topological branches. (Note also that rebase should take care of divergent bookmarks itself.)
>
> Do you have a real-life case where this has come up?

I do indeed! It was with the mercurial repo itself when I was trying out
some of Siddharth's new patches. Since I was already on '@' when I
applied the patches, '@' was moved forward. After mucking on his
patches, some time passes and I pull from crew. This creates the
divergent '@default' bookmark. "Oh, right," I say to myself, "I'll just
create a new bookmark for sid0's patches and move '@' back to its
rightful place."

At this point we're in the territory that this patch is trying to
fix. Generalizing, I think this situation is a special case of the
dichotomy of bookmarks and core commands. For example, rebase
deactivates the current bookmark. Try it out:

$ hg up @
$ hg rebase -b BOOK2 -d @

That's next on my todo list. Furthermore, the difference of using
branches and bookmarks arises in common situations for me. For example,
let's follow Alice and Bob:

Alice creates a repo,
$ hg init branch-test && cd branch-test
$ echo a>a
$ hg ci -Am0

and Bob clones it,

$ cd .. && hg clone branch-test{,1}

Now, Alice creates a hotfix branch,

$ cd branch-test
$ hg branch hotfix
$ echo b>>a
$ hg ci -Am1

When Bob pulls, he gets

$ cd ../branch-test1
$ hg pull -u
$ hg log -Gq

o  1:467ddc6e0358
|
@  0:376fdc4d163f

Even if he runs 'hg up' he is still at revision 0.  Compare this to
using bookmarks:

Alice creates a repo (and sets '@'),
$ hg init book-test && cd book-test
$ hg book @
$ echo a>a
$ hg ci -Am0

and Bob clones it,

$ cd .. && hg clone book-test{,1}

Now, Alice "branches" for a hotfix using bookmarks

$ cd book-test
$ hg book hotfix
$ echo b>>a
$ hg ci -Am1

When Bob pulls, he gets

$ cd ../book-test1
$ hg pull -u
$ hg log -Gq

@  1:9402deb9b9a4
|
o  0:6778f8d5a125

Now '@' has been moved forward automatically and will potentially send
Alice through a loop when she pulls from Bob because it will be
divergent from hers.

> Also, the above example should be rolled into your patch as an added test.

Good point. I'll roll in the test after ironing out the above discussion
(since it could change my patch into a series or something).
Sean Farley - March 28, 2013, 10:06 p.m.
Kevin Bullock writes:

> On 28 Mar 2013, at 1:50 PM, Sean Farley wrote:
>
>> Kevin Bullock writes:
>> 
>>> On 27 Mar 2013, at 10:38 PM, Sean Farley wrote:
>>> 
>>>> Stephen Lee writes:
>>>> 
>>>>> On Thu, Mar 28, 2013 at 9:41 AM, Sean Farley
>>>>> <sean.michael.farley@gmail.com> wrote:
>>>>>> # HG changeset patch
>>>>>> # User Sean Farley <sean.michael.farley@gmail.com>
>>>>>> # Date 1364421288 18000
>>>>>> #      Wed Mar 27 16:54:48 2013 -0500
>>>>>> # Node ID 3839baf52f2f24c289487111a95e9e835d1e1c4d
>>>>>> # Parent  a7d0ddc7540b2691f8a5af8006588a80e582754a
>>>>>> bookmarks: resolve divergent bookmark when moving forward
>>>>>> 
>>>>>> This patch handles both cases of moving a bookmark forward 1) when running 'hg
>>>>>> up' on a bookmark that has descendents, and 2) when running 'hg book NAME' on a
>>>>>> descendents of a the active bookmark.
>>>>>> 
>>>>> 
>>>>> What do you mean by "handling" these cases? What happens before and
>>>>> after this patch?
>>>> 
>>>> Sorry; I meant to flag this as RFC so I could get some
>>>> feedback. Anyways, before this patch when there was a divergent
>>>> bookmark on a target changeset that you're moving a bookmark to, then
>>>> both bookmarks would be there. Example,
>>>> 
>>>> $ hg init test && cd test
>>>> $ echo a>a
>>>> $ hg ci -Am0
>>>> $ echo b>>a
>>>> $ hg ci -m1
>>>> $ hg book -r 0 @
>>>> $ hg book -r 1 @default
>>>> $ hg up @
>>>> $ hg up
>>>> 
>>>> You'll see that the divergent is still lying around when it should be
>>>> taken care of automatically. Applying this patch will fix that up but I
>>>> wanted to make sure other behavior wasn't affected.
>>> 
>>> Normally this shouldn't happen in a real-life situation, because no divergent bookmarks are ever _created_ unless they actually... diverge. As in, they're on different topological branches. (Note also that rebase should take care of divergent bookmarks itself.)
>>> 
>>> Do you have a real-life case where this has come up?
>> 
>> I do indeed! It was with the mercurial repo itself when I was trying out
>> some of Siddharth's new patches. Since I was already on '@' when I
>> applied the patches, '@' was moved forward. After mucking on his
>> patches, some time passes and I pull from crew. This creates the
>> divergent '@default' bookmark.
>
> ...on a different topological branch, meaning 'hg update' won't help you here, and 'hg bookmark -r REV NAME' will require you to --force it.
>
>> "Oh, right," I say to myself, "I'll just
>> create a new bookmark for sid0's patches and move '@' back to its
>> rightful place."
>
> @'s "rightful place" is now where @default is. I still don't get how this patch helps the situation you describe.

Oops, I forgot to include the command I ran after I pulled:

$ hg book -r @default -f @

This will now have two bookmarks on the same changeset that are
'divergent' which didn't make sense to me. If I move a bookmark to its
corresponding divergent one (or vice versa), then I would expect the
divergent to be considered resolved.

>> At this point we're in the territory that this patch is trying to
>> fix. Generalizing, I think this situation is a special case of the
>> dichotomy of bookmarks and core commands. For example, rebase
>> deactivates the current bookmark. Try it out:
>> 
>> $ hg up @
>> $ hg rebase -b BOOK2 -d @
>
> I'll note here that 'bookmarks' is the core command here; 'rebase' is in an extension. The situation you describe might or might not be a bug, but I'll note that the simpler pattern for this would be:
>
> $ hg up BOOK2
> $ hg rebase -d @

That example does work and keeps BOOK2 as the active bookmark. When the
destination bookmark is active, then it becomes deactivated after rebasing.
Sean Farley - March 29, 2013, 5:18 p.m.
Kevin Bullock writes:

> On 28 Mar 2013, at 5:06 PM, Sean Farley wrote:
>
>> Kevin Bullock writes:
>> 
>>> On 28 Mar 2013, at 1:50 PM, Sean Farley wrote:
>>> 
>>>> Kevin Bullock writes:
>>>> 
>>>>> On 27 Mar 2013, at 10:38 PM, Sean Farley wrote:
>>>>>> $ hg init test && cd test
>>>>>> $ echo a>a
>>>>>> $ hg ci -Am0
>>>>>> $ echo b>>a
>>>>>> $ hg ci -m1
>>>>>> $ hg book -r 0 @
>>>>>> $ hg book -r 1 @default
>>>>>> $ hg up @
>>>>>> $ hg up
>>>>>> 
>>>>>> You'll see that the divergent is still lying around when it should be
>>>>>> taken care of automatically. Applying this patch will fix that up but I
>>>>>> wanted to make sure other behavior wasn't affected.
>>>>> 
>>>>> Normally this shouldn't happen in a real-life situation, because no divergent bookmarks are ever _created_ unless they actually... diverge. As in, they're on different topological branches. (Note also that rebase should take care of divergent bookmarks itself.)
>>>>> 
>>>>> Do you have a real-life case where this has come up?
>>>> 
>>>> I do indeed! It was with the mercurial repo itself when I was trying out
>>>> some of Siddharth's new patches. Since I was already on '@' when I
>>>> applied the patches, '@' was moved forward. After mucking on his
>>>> patches, some time passes and I pull from crew. This creates the
>>>> divergent '@default' bookmark.
>>> 
>>> ...on a different topological branch, meaning 'hg update' won't help you here, and 'hg bookmark -r REV NAME' will require you to --force it.
>>> 
>>>> "Oh, right," I say to myself, "I'll just
>>>> create a new bookmark for sid0's patches and move '@' back to its
>>>> rightful place."
>>> 
>>> @'s "rightful place" is now where @default is. I still don't get how this patch helps the situation you describe.
>> 
>> Oops, I forgot to include the command I ran after I pulled:
>> 
>> $ hg book -r @default -f @
>> 
>> This will now have two bookmarks on the same changeset that are
>> 'divergent' which didn't make sense to me. If I move a bookmark to its
>> corresponding divergent one (or vice versa), then I would expect the
>> divergent to be considered resolved.
>
> _That_ makes sense. I think it would be sufficient to just make `hg bookmark` handle this case directly.

Ok, I can restructure the patch (add more explanation, a test, etc.) and
make `hg bookmark` handle removing the divergent bookmark.

Patch

diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -182,11 +182,11 @@ 
         new = repo[node]
         if old.descendant(new):
             marks[cur] = new.node()
             update = True
 
-    if deletedivergent(repo, parents, cur):
+    if deletedivergent(repo, parents, cur) or deletedivergent(repo, node, cur):
         update = True
 
     if update:
         marks.write()
     return update
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -865,10 +865,11 @@ 
         tgt = cur
         if rev:
             tgt = scmutil.revsingle(repo, rev).node()
         checkconflict(repo, mark, force, tgt)
         marks[mark] = tgt
+        bookmarks.deletedivergent(repo, [tgt], mark)
         if not inactive and cur == marks[mark]:
             bookmarks.setcurrent(repo, mark)
         elif cur != tgt and mark == repo._bookmarkcurrent:
             bookmarks.setcurrent(repo, None)
         marks.write()