[PATCH 1 of 2] rebase: 'hg pull --rebase' now update only if there was nothing to rebase

[Martin von Zweigbergk]

On Sat, Feb 13, 2016 at 5:15 PM, Pierre-Yves David
<pierre-yves.david at ens-lyon.org> wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at fb.com>
> # Date 1455382772 0
> #      Sat Feb 13 16:59:32 2016 +0000
> # Node ID 0842ac1829bbc270e9e73cce5ccc2781d25e19a8
> # Parent  420968ffb55af1f9182f86da25793920c934c208
> # Available At http://hg.netv6.net/marmoute-wip/mercurial/
> #              hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r 0842ac1829bb
> rebase: 'hg pull --rebase' now update only if there was nothing to rebase
>
> I recently discovered that 'hg pull --rebase' was also running an update. And it
> was running it in all cases as long as the update would move the working copy
> somewhere else...
>
> This felt wrong and it actually is. This 'update' call is introduced in
> 92455c1d6f83. In that commit the intent is very clear. The update should happen
> only when there was nothing to rebase. The implementation did not check if a
> rebase was performed because, at that time, rebase would always leave you on the
> top most changeset. Being on that top most changeset result in a no-op update
> and the step was skipped.
>
> However 9c78ed396075f changed rebase behavior to preserve the working copy
> parent, so if we are not on a head at pull time, the code performs both a rebase
> and an update.
>
> This changeset introduce a test for this case and restore the intended behavior.
>
> There are other issues with this custom update code but they will be addressed
> in later changeset (eg: own destination logic, lack of heads warning).
>
> I'm not super happy with the explicitly comparison 'rebase(...) == 1'

We can at least spell it "== nothingtorebase()". I'll change that in
flight. I'll drop the then-unnecessary comment below as well. And I'll
drop the Yoda notation (we don't normally use that and I see no reason
to do it here).

> but a
> later series will have a cleaner way to handle it anyway (while making 'rebase'
> pick its default destination like 'merge').
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -1163,19 +1163,21 @@ def pullrebase(orig, ui, repo, *args, **
>                      del opts['rev']
>                  # positional argument from pull conflicts with rebase's own
>                  # --source.
>                  if 'source' in opts:
>                      del opts['source']
> -                rebase(ui, repo, **opts)
> -                branch = repo[None].branch()
> -                dest = repo[branch].rev()
> -                if dest != repo['.'].rev():
> -                    # there was nothing to rebase we force an update
> -                    hg.update(repo, dest)
> -                    if bookmarks.update(repo, [movemarkfrom], repo['.'].node()):
> -                        ui.status(_("updating bookmark %s\n")
> -                                  % repo._activebookmark)
> +                if 1 == rebase(ui, repo, **opts):
> +                    # nothing to rebase
> +                    branch = repo[None].branch()
> +                    dest = repo[branch].rev()
> +                    if dest != repo['.'].rev():
> +                        # there was nothing to rebase we force an update
> +                        hg.update(repo, dest)
> +                        if bookmarks.update(repo, [movemarkfrom],
> +                                            repo['.'].node()):
> +                            ui.status(_("updating bookmark %s\n")
> +                                      % repo._activebookmark)
>          finally:
>              release(lock, wlock)
>      else:
>          if opts.get('tool'):
>              raise error.Abort(_('--tool can only be used with --rebase'))
> diff --git a/tests/test-rebase-pull.t b/tests/test-rebase-pull.t
> --- a/tests/test-rebase-pull.t
> +++ b/tests/test-rebase-pull.t
> @@ -207,5 +207,62 @@ pull --rebase works with bundle2 turned
>    |
>    o  1: 'C2'
>    |
>    o  0: 'C1'
>
> +
> +pull --rebase only update if there is nothing to rebase
> +
> +  $ cd ../a
> +  $ echo R5 > R5
> +  $ hg ci -Am R5
> +  adding R5
> +  $ hg tglog
> +  @  6: 'R5'
> +  |
> +  o  5: 'R4'
> +  |
> +  o  4: 'R3'
> +  |
> +  o  3: 'R2'
> +  |
> +  o  2: 'R1'
> +  |
> +  o  1: 'C2'
> +  |
> +  o  0: 'C1'
> +
> +  $ cd ../c
> +  $ echo L2 > L2
> +  $ hg ci -Am L2
> +  adding L2
> +  $ hg up 'desc(L1)'
> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  $ hg pull --rebase
> +  pulling from $TESTTMP/a (glob)
> +  searching for changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 1 changes to 1 files (+1 heads)
> +  rebasing 6:0d0727eb7ce0 "L1"
> +  rebasing 7:c1f58876e3bf "L2"
> +  saved backup bundle to $TESTTMP/c/.hg/strip-backup/0d0727eb7ce0-ef61ccb2-backup.hg (glob)
> +  $ hg tglog
> +  o  8: 'L2'
> +  |
> +  @  7: 'L1'
> +  |
> +  o  6: 'R5'
> +  |
> +  o  5: 'R4'
> +  |
> +  o  4: 'R3'
> +  |
> +  o  3: 'R2'
> +  |
> +  o  2: 'R1'
> +  |
> +  o  1: 'C2'
> +  |
> +  o  0: 'C1'
> +
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel