Patchwork [2,of,2,STABLE] rebase: restrict rebase destination to the pulled set (issue5214)

login
register
mail settings
Submitter Pierre-Yves David
Date April 30, 2016, 5:20 p.m.
Message ID <69addd286d938fcd032c.1462036854@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/14847/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Pierre-Yves David - April 30, 2016, 5:20 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1462034379 -7200
#      Sat Apr 30 18:39:39 2016 +0200
# Branch stable
# Node ID 69addd286d938fcd032c37657be606792cc8ecb3
# Parent  ff7be3798ec9465d9306a30e01ab976c941c0967
# EXP-Topic issue5214
rebase: restrict rebase destination to the pulled set (issue5214)

Before this patch, `hg pull --rebase` would be a strict sequence of `hg pull`
followed by `hg rebase` if anything was pulled.

Now that rebase pick his default destination the same way than merge, than
`hg rebase` step would abort in the case the repo already had multiple anonymous
heads (because of the ambiguity). (changed in fac3a24be50e)

The intend of the user with `hg pull --rebase` is clearly to rebase on pulled
content. This used to be (mostly) enforced by the former default destination for
rebase, "tipmost changeset of the branch" as the tipmost would likely a
changeset that just got pulled. But this intended was no longer enforced with
the new defaul destination (unified with merge).

This changeset makes use of the '_destspace' mechanism introduced in the previous
changeset to enforce this.

This partially fixes issue5214 as no change at all have been made to the new
handling of the case with bookmark (unified with merge).
Pierre-Yves David - April 30, 2016, 5:31 p.m.
This two patches fixes a work-flow regression for people using 'hg pull 
--rebase' along-side multiple local anonymous heads.

I consider this a release blocker. See issue5214 for details (beware 
that the issue end up having two faces discussed in the same bug, we 
should probably split it).

On 04/30/2016 07:20 PM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1462034379 -7200
> #      Sat Apr 30 18:39:39 2016 +0200
> # Branch stable
> # Node ID 69addd286d938fcd032c37657be606792cc8ecb3
> # Parent  ff7be3798ec9465d9306a30e01ab976c941c0967
> # EXP-Topic issue5214
> rebase: restrict rebase destination to the pulled set (issue5214)
>
> Before this patch, `hg pull --rebase` would be a strict sequence of `hg pull`
> followed by `hg rebase` if anything was pulled.
>
> Now that rebase pick his default destination the same way than merge, than
> `hg rebase` step would abort in the case the repo already had multiple anonymous
> heads (because of the ambiguity). (changed in fac3a24be50e)
>
> The intend of the user with `hg pull --rebase` is clearly to rebase on pulled
> content. This used to be (mostly) enforced by the former default destination for
> rebase, "tipmost changeset of the branch" as the tipmost would likely a
> changeset that just got pulled. But this intended was no longer enforced with
> the new defaul destination (unified with merge).
>
> This changeset makes use of the '_destspace' mechanism introduced in the previous
> changeset to enforce this.
>
> This partially fixes issue5214 as no change at all have been made to the new
> handling of the case with bookmark (unified with merge).
>
> diff -r ff7be3798ec9 -r 69addd286d93 hgext/rebase.py
> --- a/hgext/rebase.py	Sat Apr 30 18:41:08 2016 +0200
> +++ b/hgext/rebase.py	Sat Apr 30 18:39:39 2016 +0200
> @@ -1244,6 +1244,9 @@
>                   # --source.
>                   if 'source' in opts:
>                       del opts['source']
> +                # revsprepull is the len of the repo, not revnum of tip.
> +                destspace = list(repo.changelog.revs(start=revsprepull))
> +                opts['_destspace'] = destspace
>                   try:
>                       rebase(ui, repo, **opts)
>                   except error.NoMergeDestAbort:
> diff -r ff7be3798ec9 -r 69addd286d93 tests/test-rebase-pull.t
> --- a/tests/test-rebase-pull.t	Sat Apr 30 18:41:08 2016 +0200
> +++ b/tests/test-rebase-pull.t	Sat Apr 30 18:39:39 2016 +0200
> @@ -311,3 +311,83 @@
>     |
>     o  0: 'C1'
>     
> +
> +Multiple pre-existing heads on the branch
> +-----------------------------------------
> +
> +Pull bring content, but nothing on the current branch, we should not consider
> +pre-existing heads.
> +
> +  $ cd ../a
> +  $ hg branch unrelatedbranch
> +  marked working directory as branch unrelatedbranch
> +  (branches are permanent and global, did you want a bookmark?)
> +  $ echo B1 > B1
> +  $ hg commit -Am B1
> +  adding B1
> +  $ cd ../c
> +  $ hg up 'desc(L2)'
> +  2 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
> +  nothing to rebase
> +
> +There is two local heads and we pull a third one.
> +The second local head should not confuse the `hg pull rebase`.
> +
> +  $ hg up 'desc(R6)'
> +  1 files updated, 0 files merged, 2 files removed, 0 files unresolved
> +  $ echo M1 > M1
> +  $ hg commit -Am M1
> +  adding M1
> +  $ cd ../a
> +  $ hg up 'desc(R6)'
> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  $ echo R7 > R7
> +  $ hg commit -Am R7
> +  adding R7
> +  $ cd ../c
> +  $ hg up 'desc(L2)'
> +  2 files updated, 0 files merged, 2 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 7:864e0a2d2614 "L1"
> +  rebasing 8:6dc0ea5dcf55 "L2"
> +  saved backup bundle to $TESTTMP/c/.hg/strip-backup/864e0a2d2614-2f72c89c-backup.hg (glob)
> +  $ hg tglog
> +  @  12: 'L2'
> +  |
> +  o  11: 'L1'
> +  |
> +  o  10: 'R7'
> +  |
> +  | o  9: 'M1'
> +  |/
> +  | o  8: 'B1' unrelatedbranch
> +  |/
> +  o  7: 'R6'
> +  |
> +  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@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - May 1, 2016, 6:24 a.m.
On Sat, 30 Apr 2016 19:20:54 +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1462034379 -7200
> #      Sat Apr 30 18:39:39 2016 +0200
> # Branch stable
> # Node ID 69addd286d938fcd032c37657be606792cc8ecb3
> # Parent  ff7be3798ec9465d9306a30e01ab976c941c0967
> # EXP-Topic issue5214
> rebase: restrict rebase destination to the pulled set (issue5214)

These changes look good and safe to me, so pushed to the committed repo,
thanks. (Someone might want to do English tweak. I'm not the right person.)

> Before this patch, `hg pull --rebase` would be a strict sequence of `hg pull`
> followed by `hg rebase` if anything was pulled.
> 
> Now that rebase pick his default destination the same way than merge, than
> `hg rebase` step would abort in the case the repo already had multiple anonymous
> heads (because of the ambiguity). (changed in fac3a24be50e)
> 
> The intend of the user with `hg pull --rebase` is clearly to rebase on pulled
> content. This used to be (mostly) enforced by the former default destination for
> rebase, "tipmost changeset of the branch" as the tipmost would likely a
> changeset that just got pulled. But this intended was no longer enforced with
> the new defaul destination (unified with merge).
> 
> This changeset makes use of the '_destspace' mechanism introduced in the previous
> changeset to enforce this.
> 
> This partially fixes issue5214 as no change at all have been made to the new
> handling of the case with bookmark (unified with merge).


> diff -r ff7be3798ec9 -r 69addd286d93 hgext/rebase.py
> --- a/hgext/rebase.py	Sat Apr 30 18:41:08 2016 +0200
> +++ b/hgext/rebase.py	Sat Apr 30 18:39:39 2016 +0200
> @@ -1244,6 +1244,9 @@
>                  # --source.
>                  if 'source' in opts:
>                      del opts['source']
> +                # revsprepull is the len of the repo, not revnum of tip.
> +                destspace = list(repo.changelog.revs(start=revsprepull))
> +                opts['_destspace'] = destspace

We could use a smart set for _destspace handling, but that is beyond the scope
of this patch.

Patch

diff -r ff7be3798ec9 -r 69addd286d93 hgext/rebase.py
--- a/hgext/rebase.py	Sat Apr 30 18:41:08 2016 +0200
+++ b/hgext/rebase.py	Sat Apr 30 18:39:39 2016 +0200
@@ -1244,6 +1244,9 @@ 
                 # --source.
                 if 'source' in opts:
                     del opts['source']
+                # revsprepull is the len of the repo, not revnum of tip.
+                destspace = list(repo.changelog.revs(start=revsprepull))
+                opts['_destspace'] = destspace
                 try:
                     rebase(ui, repo, **opts)
                 except error.NoMergeDestAbort:
diff -r ff7be3798ec9 -r 69addd286d93 tests/test-rebase-pull.t
--- a/tests/test-rebase-pull.t	Sat Apr 30 18:41:08 2016 +0200
+++ b/tests/test-rebase-pull.t	Sat Apr 30 18:39:39 2016 +0200
@@ -311,3 +311,83 @@ 
   |
   o  0: 'C1'
   
+
+Multiple pre-existing heads on the branch
+-----------------------------------------
+
+Pull bring content, but nothing on the current branch, we should not consider
+pre-existing heads.
+
+  $ cd ../a
+  $ hg branch unrelatedbranch
+  marked working directory as branch unrelatedbranch
+  (branches are permanent and global, did you want a bookmark?)
+  $ echo B1 > B1
+  $ hg commit -Am B1
+  adding B1
+  $ cd ../c
+  $ hg up 'desc(L2)'
+  2 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
+  nothing to rebase
+
+There is two local heads and we pull a third one.
+The second local head should not confuse the `hg pull rebase`.
+
+  $ hg up 'desc(R6)'
+  1 files updated, 0 files merged, 2 files removed, 0 files unresolved
+  $ echo M1 > M1
+  $ hg commit -Am M1
+  adding M1
+  $ cd ../a
+  $ hg up 'desc(R6)'
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ echo R7 > R7
+  $ hg commit -Am R7
+  adding R7
+  $ cd ../c
+  $ hg up 'desc(L2)'
+  2 files updated, 0 files merged, 2 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 7:864e0a2d2614 "L1"
+  rebasing 8:6dc0ea5dcf55 "L2"
+  saved backup bundle to $TESTTMP/c/.hg/strip-backup/864e0a2d2614-2f72c89c-backup.hg (glob)
+  $ hg tglog
+  @  12: 'L2'
+  |
+  o  11: 'L1'
+  |
+  o  10: 'R7'
+  |
+  | o  9: 'M1'
+  |/
+  | o  8: 'B1' unrelatedbranch
+  |/
+  o  7: 'R6'
+  |
+  o  6: 'R5'
+  |
+  o  5: 'R4'
+  |
+  o  4: 'R3'
+  |
+  o  3: 'R2'
+  |
+  o  2: 'R1'
+  |
+  o  1: 'C2'
+  |
+  o  0: 'C1'
+