Patchwork [2,of,2,V2] histedit: allow histedit --continue when not on a descendant

login
register
mail settings
Submitter Durham Goode
Date April 7, 2015, 1:50 a.m.
Message ID <f884cea9a0edf5e4465c.1428371429@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/8522/
State Rejected
Headers show

Comments

Durham Goode - April 7, 2015, 1:50 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1428134405 25200
#      Sat Apr 04 01:00:05 2015 -0700
# Node ID f884cea9a0edf5e4465c37ee9cba796df0943102
# Parent  e511ce4fbc848c03986a2a8e740d153cf119b8d1
histedit: allow histedit --continue when not on a descendant

Previously we would not allow --continue if the current working copy parent was
not a descendant of the commit produced by the previous histedit step. There's
nothing really blocking us from continuing the histedit in this situation, so
let's stop aborting in this case.

In the future we will likely add an 'exec' action to histedit, which means the
user can do whatever they want during the middle of a histedit (like perhaps
calling 'hg update'), which means we'll need to support this case eventually
anyway.
Matt Mackall - April 7, 2015, 10:18 p.m.
On Mon, 2015-04-06 at 18:50 -0700, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1428134405 25200
> #      Sat Apr 04 01:00:05 2015 -0700
> # Node ID f884cea9a0edf5e4465c37ee9cba796df0943102
> # Parent  e511ce4fbc848c03986a2a8e740d153cf119b8d1
> histedit: allow histedit --continue when not on a descendant
> 
> Previously we would not allow --continue if the current working copy parent was
> not a descendant of the commit produced by the previous histedit step. There's
> nothing really blocking us from continuing the histedit in this situation, so
> let's stop aborting in this case.
> 
> In the future we will likely add an 'exec' action to histedit, which means the
> user can do whatever they want during the middle of a histedit (like perhaps
> calling 'hg update'), which means we'll need to support this case eventually
> anyway.

Is this actually dependent on patch 1?
Durham Goode - April 7, 2015, 10:20 p.m.
On 4/7/15 3:18 PM, Matt Mackall wrote:
> On Mon, 2015-04-06 at 18:50 -0700, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1428134405 25200
>> #      Sat Apr 04 01:00:05 2015 -0700
>> # Node ID f884cea9a0edf5e4465c37ee9cba796df0943102
>> # Parent  e511ce4fbc848c03986a2a8e740d153cf119b8d1
>> histedit: allow histedit --continue when not on a descendant
>>
>> Previously we would not allow --continue if the current working copy parent was
>> not a descendant of the commit produced by the previous histedit step. There's
>> nothing really blocking us from continuing the histedit in this situation, so
>> let's stop aborting in this case.
>>
>> In the future we will likely add an 'exec' action to histedit, which means the
>> user can do whatever they want during the middle of a histedit (like perhaps
>> calling 'hg update'), which means we'll need to support this case eventually
>> anyway.
> Is this actually dependent on patch 1?
No, it just seemed easier to send them out in series since they're in a 
related area, and since I tested them when stacked in this order.
Augie Fackler - April 7, 2015, 11:14 p.m.
On Apr 7, 2015 6:20 PM, "Durham Goode" <durham@fb.com> wrote:
>
>
>
> On 4/7/15 3:18 PM, Matt Mackall wrote:
>>
>> On Mon, 2015-04-06 at 18:50 -0700, Durham Goode wrote:
>>>
>>> # HG changeset patch
>>> # User Durham Goode <durham@fb.com>
>>> # Date 1428134405 25200
>>> #      Sat Apr 04 01:00:05 2015 -0700
>>> # Node ID f884cea9a0edf5e4465c37ee9cba796df0943102
>>> # Parent  e511ce4fbc848c03986a2a8e740d153cf119b8d1
>>> histedit: allow histedit --continue when not on a descendant
>>>
>>> Previously we would not allow --continue if the current working copy
parent was
>>> not a descendant of the commit produced by the previous histedit step.
There's
>>> nothing really blocking us from continuing the histedit in this
situation, so
>>> let's stop aborting in this case.
>>>
>>> In the future we will likely add an 'exec' action to histedit, which
means the
>>> user can do whatever they want during the middle of a histedit (like
perhaps
>>> calling 'hg update'), which means we'll need to support this case
eventually
>>> anyway.
>>
>> Is this actually dependent on patch 1?
>
> No, it just seemed easier to send them out in series since they're in a
related area, and since I tested them when stacked in this order.

One test I'd like to see is what happens if you update to something outside
the histedited set.

Maybe we should just chat at the sprint, I'm really in vacation today.
Durham Goode - April 7, 2015, 11:38 p.m.
On 4/7/15 4:14 PM, Augie Fackler wrote:
>
>
> On Apr 7, 2015 6:20 PM, "Durham Goode" <durham@fb.com 
> <mailto:durham@fb.com>> wrote:
> >
> >
> >
> > On 4/7/15 3:18 PM, Matt Mackall wrote:
> >>
> >> On Mon, 2015-04-06 at 18:50 -0700, Durham Goode wrote:
> >>>
> >>> # HG changeset patch
> >>> # User Durham Goode <durham@fb.com <mailto:durham@fb.com>>
> >>> # Date 1428134405 25200
> >>> #      Sat Apr 04 01:00:05 2015 -0700
> >>> # Node ID f884cea9a0edf5e4465c37ee9cba796df0943102
> >>> # Parent  e511ce4fbc848c03986a2a8e740d153cf119b8d1
> >>> histedit: allow histedit --continue when not on a descendant
> >>>
> >>> Previously we would not allow --continue if the current working 
> copy parent was
> >>> not a descendant of the commit produced by the previous histedit 
> step. There's
> >>> nothing really blocking us from continuing the histedit in this 
> situation, so
> >>> let's stop aborting in this case.
> >>>
> >>> In the future we will likely add an 'exec' action to histedit, 
> which means the
> >>> user can do whatever they want during the middle of a histedit 
> (like perhaps
> >>> calling 'hg update'), which means we'll need to support this case 
> eventually
> >>> anyway.
> >>
> >> Is this actually dependent on patch 1?
> >
> > No, it just seemed easier to send them out in series since they're 
> in a related area, and since I tested them when stacked in this order.
>
> One test I'd like to see is what happens if you update to something 
> outside the histedited set.
>
> Maybe we should just chat at the sprint, I'm really in vacation today.
>
K, we can hold onto this series until the sprint.  If you get a chance 
on an airplane or something, you can peek at the whole series to 
formulate an opinion: 
https://bitbucket.org/DurhamG/hg/commits/branch/histedit

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -774,11 +774,7 @@  def gatherchildren(repo, ctx):
     newchildren = [c.node() for c in repo.set('(%d::.)', ctx)]
     if ctx.node() != node.nullid:
         if not newchildren:
-            # `ctx` should match but no result. This means that
-            # currentnode is not a descendant from ctx.
-            msg = _('%s is not an ancestor of working directory')
-            hint = _('use "histedit --abort" to clear broken state')
-            raise util.Abort(msg % ctx, hint=hint)
+            return []
         newchildren.pop(0)  # remove ctx
     return newchildren
 
diff --git a/tests/test-histedit-arguments.t b/tests/test-histedit-arguments.t
--- a/tests/test-histedit-arguments.t
+++ b/tests/test-histedit-arguments.t
@@ -120,6 +120,13 @@  created (and forgotten) by Mercurial ear
 Mercurial earlier than 2.7 by renaming ".hg/histedit-state"
 temporarily.
 
+  $ hg log -G -T '{rev} {shortest(node)} {desc}\n' -r 2::
+  @  4 08d9 five
+  |
+  o  3 c8e6 four
+  |
+  o  2 eb57 three
+  |
   $ HGEDITOR=cat hg histedit -r 4 --commands - << EOF
   > edit 08d98a8350f3 4 five
   > EOF
@@ -130,16 +137,24 @@  temporarily.
   [1]
 
   $ mv .hg/histedit-state .hg/histedit-state.back
-  $ hg update --quiet --clean 2
+  $ hg update -q --clean 2
+  $ echo alpha >> alpha
   $ mv .hg/histedit-state.back .hg/histedit-state
 
   $ hg histedit --continue
-  abort: c8e68270e35a is not an ancestor of working directory
-  (use "histedit --abort" to clear broken state)
-  [255]
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  saved backup bundle to $TESTTMP/foo/.hg/strip-backup/08d98a8350f3-02594089-backup.hg (glob)
+  $ hg log -G -T '{rev} {shortest(node)} {desc}\n' -r 2::
+  @  4 f5ed five
+  |
+  | o  3 c8e6 four
+  |/
+  o  2 eb57 three
+  |
 
-  $ hg histedit --abort
-  $ hg update --quiet --clean
+  $ hg unbundle -q $TESTTMP/foo/.hg/strip-backup/08d98a8350f3-02594089-backup.hg
+  $ hg strip -q -r f5ed --config extensions.strip=
+  $ hg up -q 08d98a8350f3
 
 Test that missing revisions are detected
 ---------------------------------------