Patchwork [STABLE] commands: advance current active bookmark at pull --update correctly

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Jan. 28, 2016, 11:16 a.m.
Message ID <3678de8c5edb6f62f972.1453979783@feefifofum>
Download mbox | patch
Permalink /patch/12899/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Katsunori FUJIWARA - Jan. 28, 2016, 11:16 a.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1453979406 -32400
#      Thu Jan 28 20:10:06 2016 +0900
# Branch stable
# Node ID 3678de8c5edb6f62f9722be5a028b4ec49a7fadc
# Parent  4511e8dac4c798e5fed91e629aa9802b01c2b6c3
commands: advance current active bookmark at pull --update correctly

Before this patch, "hg pull --update" doesn't advance current active
bookmark correctly, if pulling itself doesn't advance it, even though
"hg pull" + "hg update" does so.

Existing test for "pull --update works the same as pull && update" in
test-bookmarks.t doesn't examine this case, because pulling itself
advance current active bookmark before actual updating the working
directory in that test case.

To advance current active bookmark at "hg pull --update" correctly,
this patch examines 'movemarkfrom' instead of 'not checkout'.

Even if 'not checkout' at the invocation of postincoming(), 'checkout'
is overwritten by "the revision to update to" value returned by
destutil.destupdate() in such case. Therefore, 'not checkout'
condition means "update destination is revision #0", and isn't
suitable for examining whether active bookmark should be advanced.

Even though examination around "movemarkfrom == repo['.'].node()" may
seem a little redundant just for this issue, this makes it easier to
compare (and unify in the future, maybe) with the same logic to update
bookmark at "hg update" below.

        if not ret and movemarkfrom:
            if movemarkfrom == repo['.'].node():
                pass # no-op update
            elif bookmarks.update(repo, [movemarkfrom], repo['.'].node()):
                ui.status(_("updating bookmark %s\n") % repo._activebookmark)
            else:
                # this can happen with a non-linear update
                ui.status(_("(leaving bookmark %s)\n") %
                          repo._activebookmark)
                bookmarks.deactivate(repo)
Yuya Nishihara - Jan. 28, 2016, 2:20 p.m.
On Thu, 28 Jan 2016 20:16:23 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1453979406 -32400
> #      Thu Jan 28 20:10:06 2016 +0900
> # Branch stable
> # Node ID 3678de8c5edb6f62f9722be5a028b4ec49a7fadc
> # Parent  4511e8dac4c798e5fed91e629aa9802b01c2b6c3
> commands: advance current active bookmark at pull --update correctly
> 
> Before this patch, "hg pull --update" doesn't advance current active
> bookmark correctly, if pulling itself doesn't advance it, even though
> "hg pull" + "hg update" does so.
> 
> Existing test for "pull --update works the same as pull && update" in
> test-bookmarks.t doesn't examine this case, because pulling itself
> advance current active bookmark before actual updating the working
> directory in that test case.
> 
> To advance current active bookmark at "hg pull --update" correctly,
> this patch examines 'movemarkfrom' instead of 'not checkout'.
> 
> Even if 'not checkout' at the invocation of postincoming(), 'checkout'
> is overwritten by "the revision to update to" value returned by
> destutil.destupdate() in such case. Therefore, 'not checkout'
> condition means "update destination is revision #0", and isn't
> suitable for examining whether active bookmark should be advanced.
> 
> Even though examination around "movemarkfrom == repo['.'].node()" may
> seem a little redundant just for this issue, this makes it easier to
> compare (and unify in the future, maybe) with the same logic to update
> bookmark at "hg update" below.
> 
>         if not ret and movemarkfrom:
>             if movemarkfrom == repo['.'].node():
>                 pass # no-op update
>             elif bookmarks.update(repo, [movemarkfrom], repo['.'].node()):
>                 ui.status(_("updating bookmark %s\n") % repo._activebookmark)
>             else:
>                 # this can happen with a non-linear update
>                 ui.status(_("(leaving bookmark %s)\n") %
>                           repo._activebookmark)
>                 bookmarks.deactivate(repo)
> 
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -5542,8 +5542,10 @@ def postincoming(ui, repo, modheads, opt
>              msg = _("not updating: %s") % str(inst)
>              hint = inst.hint
>              raise error.UpdateAbort(msg, hint=hint)
> -        if not ret and not checkout:
> -            if bookmarks.update(repo, [movemarkfrom], repo['.'].node()):
> +        if not ret and movemarkfrom:
> +            if movemarkfrom == repo['.'].node():
> +                pass # no-op update
> +            elif bookmarks.update(repo, [movemarkfrom], repo['.'].node()):
>                  ui.status(_("updating bookmark %s\n") % repo._activebookmark)

This looks like a follow-up for 70ac5f724fbd, and looks good to me.

https://selenic.com/repo/hg/rev/70ac5f724fbd

Pushed to the clowncopter, thanks.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5542,8 +5542,10 @@  def postincoming(ui, repo, modheads, opt
             msg = _("not updating: %s") % str(inst)
             hint = inst.hint
             raise error.UpdateAbort(msg, hint=hint)
-        if not ret and not checkout:
-            if bookmarks.update(repo, [movemarkfrom], repo['.'].node()):
+        if not ret and movemarkfrom:
+            if movemarkfrom == repo['.'].node():
+                pass # no-op update
+            elif bookmarks.update(repo, [movemarkfrom], repo['.'].node()):
                 ui.status(_("updating bookmark %s\n") % repo._activebookmark)
         return ret
     if modheads > 1:
diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
--- a/tests/test-bookmarks.t
+++ b/tests/test-bookmarks.t
@@ -781,3 +781,61 @@  test clearing only a single divergent bo
      foo@2                     2:db815d6d32e6
      four                      3:9ba5f110a0b3
      should-end-on-two         2:db815d6d32e6
+
+pull --update works the same as pull && update (case #2)
+
+It is assumed that "hg pull" itself doesn't update current active
+bookmark ('Y' in tests below).
+
+  $ hg pull -q ../cloned-bookmarks-update
+  divergent bookmark Z stored as Z@2
+
+(pulling revision on another named branch with --update updates
+neither the working directory nor current active bookmark: "no-op"
+case)
+
+  $ echo yy >> y
+  $ hg commit -m yy
+
+  $ hg -R ../cloned-bookmarks-update bookmarks | grep ' Y '
+   * Y                         3:125c9a1d6df6
+  $ hg -R ../cloned-bookmarks-update pull . --update
+  pulling from .
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  divergent bookmark Z stored as Z@default
+  adding remote bookmark foo
+  adding remote bookmark four
+  adding remote bookmark should-end-on-two
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg -R ../cloned-bookmarks-update parents -T "{rev}:{node|short}\n"
+  3:125c9a1d6df6
+  $ hg -R ../cloned-bookmarks-update bookmarks | grep ' Y '
+   * Y                         3:125c9a1d6df6
+
+(pulling revision on current named/topological branch with --update
+updates the working directory and current active bookmark)
+
+  $ hg update -C -q 125c9a1d6df6
+  $ echo xx >> x
+  $ hg commit -m xx
+
+  $ hg -R ../cloned-bookmarks-update bookmarks | grep ' Y '
+   * Y                         3:125c9a1d6df6
+  $ hg -R ../cloned-bookmarks-update pull . --update
+  pulling from .
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  divergent bookmark Z stored as Z@default
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  updating bookmark Y
+  $ hg -R ../cloned-bookmarks-update parents -T "{rev}:{node|short}\n"
+  6:81dcce76aa0b
+  $ hg -R ../cloned-bookmarks-update bookmarks | grep ' Y '
+   * Y                         6:81dcce76aa0b