Patchwork [V2] rebase: add flag to commit to insert commit in stack

login
register
mail settings
Submitter Christian Delahousse
Date Nov. 25, 2015, 11:03 p.m.
Message ID <aa9a87863f9af5ad37d4.1448492588@dev4253.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11664/
State Rejected
Headers show

Comments

Christian Delahousse - Nov. 25, 2015, 11:03 p.m.
# HG changeset patch
# User Christian Delahousse <cdelahousse@fb.com>
# Date 1448492314 28800
#      Wed Nov 25 14:58:34 2015 -0800
# Node ID aa9a87863f9af5ad37d477131196a65d5437d116
# Parent  61fbf5dc12b23e7a2a30cf04ebd9f096c42a1f61
rebase: add flag to commit to insert commit in stack

This patch adds a flag to insert a commit within an existing stack without
forcing the user to do a separate rebase step. From the call to commit --insert,
the tool flag is passed into our call to rebase in case the user specifies it.

If the rebase portion fails, the user is prompted on how to proceed. If --amend
is passed in, the current commit is amended and the stack is maintained
on top of the new commit.

A new test file is created because as there was no existing suitable place to
test the functionality.
Christian Delahousse - Nov. 25, 2015, 11:20 p.m.
This is an update to my previous patch with added tests defining --amend 
hand illustrating merge conflict behaviour.

This patch implements behaviour we've been asked for. I believe 
inserting a commit in a stack covers the most popular use case for any 
insertion behaviour. Why not give easy access to the main use case?

On 11/25/15 3:03 PM, cdelahousse@fb.com wrote:
> # HG changeset patch
> # User Christian Delahousse <cdelahousse@fb.com>
> # Date 1448492314 28800
> #      Wed Nov 25 14:58:34 2015 -0800
> # Node ID aa9a87863f9af5ad37d477131196a65d5437d116
> # Parent  61fbf5dc12b23e7a2a30cf04ebd9f096c42a1f61
> rebase: add flag to commit to insert commit in stack
>
> This patch adds a flag to insert a commit within an existing stack without
> forcing the user to do a separate rebase step. From the call to commit --insert,
> the tool flag is passed into our call to rebase in case the user specifies it.
>
> If the rebase portion fails, the user is prompted on how to proceed. If --amend
> is passed in, the current commit is amended and the stack is maintained
> on top of the new commit.
>
> A new test file is created because as there was no existing suitable place to
> test the functionality.
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -1229,6 +1229,35 @@
>   
>       return obsoletenotrebased
>   
> +def commitinsert(orig, ui, repo, *args, **opts):
> +    '''Insert a commit and rebase the current descendants on top of it
> +    '''
> +    ret = None
> +    if opts.get('insert'):
> +        wlock = lock = None
> +        try:
> +            wlock = repo.wlock()
> +            lock = repo.lock()
> +            descendants = list(repo.revs('(.::) - .'))
> +            ret = orig(ui, repo, *args, **opts)
> +            rebaseopts = {
> +                'dest' : '.',
> +                'rev' : descendants,
> +                'tool': opts.get('tool')
> +            }
> +
> +            try:
> +                rebase(ui, repo, **rebaseopts)
> +            except error.InterventionRequired:
> +                raise error.Abort(_('commit --insert failed during rebase '
> +                    'because of unresolved conflicts'), hint=_('see hg '
> +                    'resolve, then hg rebase --continue or hg rebase --abort'))
> +        finally:
> +            release(lock, wlock)
> +    else:
> +        ret = orig(ui, repo, *args, **opts)
> +    return ret
> +
>   def summaryhook(ui, repo):
>       if not os.path.exists(repo.join('rebasestate')):
>           return
> @@ -1248,6 +1277,10 @@
>   
>   def uisetup(ui):
>       #Replace pull with a decorator to provide --rebase option
> +    entry = extensions.wrapcommand(commands.table, 'commit', commitinsert)
> +    entry[1].append(('', 'insert', None,
> +                     _('insert a commit after the current one and rebase '
> +                        'descendants on top of it')))
>       entry = extensions.wrapcommand(commands.table, 'pull', pullrebase)
>       entry[1].append(('', 'rebase', None,
>                        _("rebase working directory to branch head")))
> diff --git a/tests/test-commit-insert.t b/tests/test-commit-insert.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-commit-insert.t
> @@ -0,0 +1,104 @@
> +  $ cat >> $HGRCPATH << EOF
> +  > [ui]
> +  > logtemplate={rev}:{node|short} {desc|firstline}
> +  > [experimental]
> +  > evolution=createmarkers,allowunstable
> +  > [phases]
> +  > publish=False
> +  > [extensions]
> +  > rebase=
> +  > EOF
> +
> +  $ mkcommit() {
> +  >    echo "$1" > "$1"
> +  >    hg add "$1"
> +  >    hg ci -m "add $1"
> +  > }
> +
> +  $ mkinsert() {
> +  >    echo "$1" > "$1"
> +  >    hg add "$1"
> +  >    hg commit --insert -m "add $1"
> +  > }
> +
> +Test commit --insert inserts a commit midway through the stack and rebases
> +  $ hg init repo
> +  $ cd repo
> +  $ mkcommit a
> +  $ mkcommit b
> +  $ mkcommit c
> +  $ mkcommit d
> +  $ mkcommit e
> +  $ hg up -r 'desc(b)' -q
> +  $ mkcommit f
> +  created new head
> +  $ hg up -r 'desc(b)' -q
> +  $ hg log -G
> +  o  5:62d48859d3ff add f
> +  |
> +  | o  4:9d206ffc875e add e
> +  | |
> +  | o  3:47d2a3944de8 add d
> +  | |
> +  | o  2:4538525df7e2 add c
> +  |/
> +  @  1:7c3bad9141dc add b
> +  |
> +  o  0:1f0dee641bb7 add a
> +
> +  $ mkinsert inserted
> +  created new head
> +  rebasing 2:4538525df7e2 "add c"
> +  rebasing 3:47d2a3944de8 "add d"
> +  rebasing 4:9d206ffc875e "add e"
> +  rebasing 5:62d48859d3ff "add f"
> +  $ hg log -G
> +  o  10:e8b1420e489a add f
> +  |
> +  | o  9:f15deecf3961 add e
> +  | |
> +  | o  8:5c44ffbdf4bc add d
> +  | |
> +  | o  7:40e9af76281f add c
> +  |/
> +  @  6:76c6dadac81f add inserted
> +  |
> +  o  1:7c3bad9141dc add b
> +  |
> +  o  0:1f0dee641bb7 add a
> +
> +Test commit with both --amend and --insert amends the commit properly
> +  $ echo 'content' > inserted
> +  $ hg commit --amend --insert --message 'modified inserted' -q
> +  $ hg log -G
> +  o  16:a3b1913b9e14 add f
> +  |
> +  | o  15:e51039ab4348 add e
> +  | |
> +  | o  14:9bd4195e1b56 add d
> +  | |
> +  | o  13:c2484d20e987 add c
> +  |/
> +  @  12:cc2ad5e24cd3 modified inserted
> +  |
> +  o  1:7c3bad9141dc add b
> +  |
> +  o  0:1f0dee641bb7 add a
> +
> +
> +Test rebase conflict
> +  $ hg up -r 'desc(f)' -q
> +  $ echo 'ff' > f
> +  $ hg commit --amend
> +  $ hg up '.^'
> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  $ echo '' > f
> +  $ hg add f
> +  $ hg commit --insert --message 'add f that will conflict' -q
> +  warning: conflicts while merging f! (edit, then use 'hg resolve --mark')
> +  abort: commit --insert failed during rebase because of unresolved conflicts
> +  (see hg resolve, then hg rebase --continue or hg rebase --abort)
> +  [255]
> +
> +  $ hg rebase --abort -q
> +  rebase aborted
Pierre-Yves David - Nov. 30, 2015, 5:32 p.m.
On 11/25/2015 03:03 PM, cdelahousse@fb.com wrote:
> # HG changeset patch
> # User Christian Delahousse <cdelahousse@fb.com>
> # Date 1448492314 28800
> #      Wed Nov 25 14:58:34 2015 -0800
> # Node ID aa9a87863f9af5ad37d477131196a65d5437d116
> # Parent  61fbf5dc12b23e7a2a30cf04ebd9f096c42a1f61
> rebase: add flag to commit to insert commit in stack
>
> This patch adds a flag to insert a commit within an existing stack without
> forcing the user to do a separate rebase step. From the call to commit --insert,
> the tool flag is passed into our call to rebase in case the user specifies it.
>
> If the rebase portion fails, the user is prompted on how to proceed. If --amend
> is passed in, the current commit is amended and the stack is maintained
> on top of the new commit.
>
> A new test file is created because as there was no existing suitable place to
> test the functionality.

It is probably too early to accepts this patch in core repository.

The need for a way to insert a commit into a stack is real but the right 
way and UI to do it is not clear to me yet.


However, playing with such UI is still a good idea. Let's put this in 
some of our experimental workspace. I would gladly accept a patch for evolve



Here is the subset of what is making me wish some delay in making this 
UI officiel and backward compatible:

- Rebasing the whole stack can lead to exponential obsmarker creation, 
we need someone to sit down and make proposals about how to deter the 
risk of such explosion before adding more commands with such risks.

- Rebasing all branches starting from a sibling might create too much 
conflicts for the user to handler efficiently. especially when there is 
multiple sibling. Having a clear step by step path might be superior here.

- We might have better UI available through a good definition of the 
"current stack". We should let the current experiment unfold before 
setting down UX for backward compatibility,

- On similar topic, this should probably inserts into a wider plan 
regarding history editing UI.

Cheers,
Augie Fackler - Dec. 1, 2015, 2:51 p.m.
On Mon, Nov 30, 2015 at 09:32:42AM -0800, Pierre-Yves David wrote:
>
>
> On 11/25/2015 03:03 PM, cdelahousse@fb.com wrote:
> ># HG changeset patch
> ># User Christian Delahousse <cdelahousse@fb.com>
> ># Date 1448492314 28800
> >#      Wed Nov 25 14:58:34 2015 -0800
> ># Node ID aa9a87863f9af5ad37d477131196a65d5437d116
> ># Parent  61fbf5dc12b23e7a2a30cf04ebd9f096c42a1f61
> >rebase: add flag to commit to insert commit in stack
> >

[snip many sensible points]

> - On similar topic, this should probably inserts into a wider plan regarding
> history editing UI.

Do you have a wiki page where you're collecting workflows that are
important to people? It sounds like some facebook folks have some
fairly interesting workflows I've never seen anywhere else, and it'd
be nice to document their use cases as we get closer to the
long-planned history editing promised land.

>
> Cheers,
>
> --
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1229,6 +1229,35 @@ 
 
     return obsoletenotrebased
 
+def commitinsert(orig, ui, repo, *args, **opts):
+    '''Insert a commit and rebase the current descendants on top of it
+    '''
+    ret = None
+    if opts.get('insert'):
+        wlock = lock = None
+        try:
+            wlock = repo.wlock()
+            lock = repo.lock()
+            descendants = list(repo.revs('(.::) - .'))
+            ret = orig(ui, repo, *args, **opts)
+            rebaseopts = {
+                'dest' : '.',
+                'rev' : descendants,
+                'tool': opts.get('tool')
+            }
+
+            try:
+                rebase(ui, repo, **rebaseopts)
+            except error.InterventionRequired:
+                raise error.Abort(_('commit --insert failed during rebase '
+                    'because of unresolved conflicts'), hint=_('see hg '
+                    'resolve, then hg rebase --continue or hg rebase --abort'))
+        finally:
+            release(lock, wlock)
+    else:
+        ret = orig(ui, repo, *args, **opts)
+    return ret
+
 def summaryhook(ui, repo):
     if not os.path.exists(repo.join('rebasestate')):
         return
@@ -1248,6 +1277,10 @@ 
 
 def uisetup(ui):
     #Replace pull with a decorator to provide --rebase option
+    entry = extensions.wrapcommand(commands.table, 'commit', commitinsert)
+    entry[1].append(('', 'insert', None,
+                     _('insert a commit after the current one and rebase '
+                        'descendants on top of it')))
     entry = extensions.wrapcommand(commands.table, 'pull', pullrebase)
     entry[1].append(('', 'rebase', None,
                      _("rebase working directory to branch head")))
diff --git a/tests/test-commit-insert.t b/tests/test-commit-insert.t
new file mode 100644
--- /dev/null
+++ b/tests/test-commit-insert.t
@@ -0,0 +1,104 @@ 
+  $ cat >> $HGRCPATH << EOF
+  > [ui]
+  > logtemplate={rev}:{node|short} {desc|firstline}
+  > [experimental]
+  > evolution=createmarkers,allowunstable
+  > [phases]
+  > publish=False
+  > [extensions]
+  > rebase=
+  > EOF
+
+  $ mkcommit() {
+  >    echo "$1" > "$1"
+  >    hg add "$1"
+  >    hg ci -m "add $1"
+  > }
+
+  $ mkinsert() {
+  >    echo "$1" > "$1"
+  >    hg add "$1"
+  >    hg commit --insert -m "add $1"
+  > }
+
+Test commit --insert inserts a commit midway through the stack and rebases
+  $ hg init repo
+  $ cd repo
+  $ mkcommit a
+  $ mkcommit b
+  $ mkcommit c
+  $ mkcommit d
+  $ mkcommit e
+  $ hg up -r 'desc(b)' -q
+  $ mkcommit f
+  created new head
+  $ hg up -r 'desc(b)' -q
+  $ hg log -G
+  o  5:62d48859d3ff add f
+  |
+  | o  4:9d206ffc875e add e
+  | |
+  | o  3:47d2a3944de8 add d
+  | |
+  | o  2:4538525df7e2 add c
+  |/
+  @  1:7c3bad9141dc add b
+  |
+  o  0:1f0dee641bb7 add a
+  
+  $ mkinsert inserted
+  created new head
+  rebasing 2:4538525df7e2 "add c"
+  rebasing 3:47d2a3944de8 "add d"
+  rebasing 4:9d206ffc875e "add e"
+  rebasing 5:62d48859d3ff "add f"
+  $ hg log -G
+  o  10:e8b1420e489a add f
+  |
+  | o  9:f15deecf3961 add e
+  | |
+  | o  8:5c44ffbdf4bc add d
+  | |
+  | o  7:40e9af76281f add c
+  |/
+  @  6:76c6dadac81f add inserted
+  |
+  o  1:7c3bad9141dc add b
+  |
+  o  0:1f0dee641bb7 add a
+  
+Test commit with both --amend and --insert amends the commit properly
+  $ echo 'content' > inserted
+  $ hg commit --amend --insert --message 'modified inserted' -q
+  $ hg log -G
+  o  16:a3b1913b9e14 add f
+  |
+  | o  15:e51039ab4348 add e
+  | |
+  | o  14:9bd4195e1b56 add d
+  | |
+  | o  13:c2484d20e987 add c
+  |/
+  @  12:cc2ad5e24cd3 modified inserted
+  |
+  o  1:7c3bad9141dc add b
+  |
+  o  0:1f0dee641bb7 add a
+  
+
+Test rebase conflict
+  $ hg up -r 'desc(f)' -q
+  $ echo 'ff' > f
+  $ hg commit --amend
+  $ hg up '.^'
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ echo '' > f
+  $ hg add f
+  $ hg commit --insert --message 'add f that will conflict' -q
+  warning: conflicts while merging f! (edit, then use 'hg resolve --mark')
+  abort: commit --insert failed during rebase because of unresolved conflicts
+  (see hg resolve, then hg rebase --continue or hg rebase --abort)
+  [255]
+
+  $ hg rebase --abort -q
+  rebase aborted