Patchwork [3,of,3] histedit: test that an aborted histedit can be rerun (with obsolete)

# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1490536534 -7200
#      Sun Mar 26 15:55:34 2017 +0200
# Node ID 59c6489c75dcf13989f4d4a343f3a18d1475e20d
# Parent  37931386562505e783e9c0b7f78287fabb71a7c2
# EXP-Topic backout-histedit
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 59c6489c75dc
histedit: test that an aborted histedit can be rerun (with obsolete)

In the future, this should help catching issue as the one introduced in
6f0b7475cf9a.

I'm -1 on this series.

If the goal is to workaround obsolete cycles, the code could do additional
checks "if the destination is obsoleted" and do "hg touch"-like thing
automatically.

That workaround will be no longer necessary if my obsolete cycle patches are
landed.

Internally, we have several repo corruption reports because of "histedit
--abort" is using strip.

Excerpts from Pierre-Yves David's message of 2017-03-26 16:16:09 +0200:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1490536534 -7200
> #      Sun Mar 26 15:55:34 2017 +0200
> # Node ID 59c6489c75dcf13989f4d4a343f3a18d1475e20d
> # Parent  37931386562505e783e9c0b7f78287fabb71a7c2
> # EXP-Topic backout-histedit
> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ 
> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/  -r 59c6489c75dc
> histedit: test that an aborted histedit can be rerun (with obsolete)
> 
> In the future, this should help catching issue as the one introduced in
> 6f0b7475cf9a.
> 
> diff --git a/tests/test-histedit-obsolete.t b/tests/test-histedit-obsolete.t
> --- a/tests/test-histedit-obsolete.t
> +++ b/tests/test-histedit-obsolete.t
> @@ -503,3 +503,74 @@ Note that there is a few reordering in t
>    abort: cannot edit history that contains merges
>    [255]
>    $ cd ..
> +
> +Check abort behavior
> +-------------------------------------------
> +
> +We checks that abort properly clean the repository so the same histedit can be
> +attempted later.
> +
> +  $ cp -R base abort
> +  $ cd abort
> +  $ hg histedit -r 'b449568bf7fc' --commands - << EOF
> +  > pick b449568bf7fc 13 f
> +  > pick 7395e1ff83bd 15 h
> +  > pick 6b70183d2492 14 g
> +  > pick b605fb7503f2 16 i
> +  > roll 3a6c53ee7f3d 17 j
> +  > edit ee118ab9fa44 18 k
> +  > EOF
> +  Editing (ee118ab9fa44), you may commit or record as needed now.
> +  (hg histedit --continue to resume)
> +  [1]
> +
> +  $ hg histedit --abort
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  saved backup bundle to $TESTTMP/abort/.hg/strip-backup/4dc06258baa6-dff4ef05-backup.hg (glob)
> +
> +  $ hg log -G
> +  @  18:ee118ab9fa44 (secret) k
> +  |
> +  o  17:3a6c53ee7f3d (secret) j
> +  |
> +  o  16:b605fb7503f2 (secret) i
> +  |
> +  o  15:7395e1ff83bd (draft) h
> +  |
> +  o  14:6b70183d2492 (draft) g
> +  |
> +  o  13:b449568bf7fc (draft) f
> +  |
> +  o  12:40db8afa467b (public) c
> +  |
> +  o  0:cb9a9f314b8b (public) a
> +  
> +  $ hg histedit -r 'b449568bf7fc' --commands - << EOF
> +  > pick b449568bf7fc 13 f
> +  > pick 7395e1ff83bd 15 h
> +  > pick 6b70183d2492 14 g
> +  > pick b605fb7503f2 16 i
> +  > pick 3a6c53ee7f3d 17 j
> +  > edit ee118ab9fa44 18 k
> +  > EOF
> +  Editing (ee118ab9fa44), you may commit or record as needed now.
> +  (hg histedit --continue to resume)
> +  [1]
> +  $ hg histedit --continue
> +  $ hg log -G
> +  @  23:175d6b286a22 (secret) k
> +  |
> +  o  22:44ca09d59ae4 (secret) j
> +  |
> +  o  21:31747692a644 (secret) i
> +  |
> +  o  20:9985cd4f21fa (draft) g
> +  |
> +  o  19:4dc06258baa6 (draft) h
> +  |
> +  o  13:b449568bf7fc (draft) f
> +  |
> +  o  12:40db8afa467b (public) c
> +  |
> +  o  0:cb9a9f314b8b (public) a
> +

Excerpts from Jun Wu's message of 2017-03-26 09:59:43 -0700:
> I'm -1 on this series.
> 
> If the goal is to workaround obsolete cycles, the code could do additional
> checks "if the destination is obsoleted" and do "hg touch"-like thing
> automatically.

FYI I have sent the series doing the above.

> 
> That workaround will be no longer necessary if my obsolete cycle patches are
> landed.
> 
> Internally, we have several repo corruption reports because of "histedit
> --abort" is using strip.
> 
> Excerpts from Pierre-Yves David's message of 2017-03-26 16:16:09 +0200:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> > # Date 1490536534 -7200
> > #      Sun Mar 26 15:55:34 2017 +0200
> > # Node ID 59c6489c75dcf13989f4d4a343f3a18d1475e20d
> > # Parent  37931386562505e783e9c0b7f78287fabb71a7c2
> > # EXP-Topic backout-histedit
> > # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/  
> > #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/   -r 59c6489c75dc
> > histedit: test that an aborted histedit can be rerun (with obsolete)
> > 
> > In the future, this should help catching issue as the one introduced in
> > 6f0b7475cf9a.
> > 
> > diff --git a/tests/test-histedit-obsolete.t b/tests/test-histedit-obsolete.t
> > --- a/tests/test-histedit-obsolete.t
> > +++ b/tests/test-histedit-obsolete.t
> > @@ -503,3 +503,74 @@ Note that there is a few reordering in t
> >    abort: cannot edit history that contains merges
> >    [255]
> >    $ cd ..
> > +
> > +Check abort behavior
> > +-------------------------------------------
> > +
> > +We checks that abort properly clean the repository so the same histedit can be
> > +attempted later.
> > +
> > +  $ cp -R base abort
> > +  $ cd abort
> > +  $ hg histedit -r 'b449568bf7fc' --commands - << EOF
> > +  > pick b449568bf7fc 13 f
> > +  > pick 7395e1ff83bd 15 h
> > +  > pick 6b70183d2492 14 g
> > +  > pick b605fb7503f2 16 i
> > +  > roll 3a6c53ee7f3d 17 j
> > +  > edit ee118ab9fa44 18 k
> > +  > EOF
> > +  Editing (ee118ab9fa44), you may commit or record as needed now.
> > +  (hg histedit --continue to resume)
> > +  [1]
> > +
> > +  $ hg histedit --abort
> > +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> > +  saved backup bundle to $TESTTMP/abort/.hg/strip-backup/4dc06258baa6-dff4ef05-backup.hg (glob)
> > +
> > +  $ hg log -G
> > +  @  18:ee118ab9fa44 (secret) k
> > +  |
> > +  o  17:3a6c53ee7f3d (secret) j
> > +  |
> > +  o  16:b605fb7503f2 (secret) i
> > +  |
> > +  o  15:7395e1ff83bd (draft) h
> > +  |
> > +  o  14:6b70183d2492 (draft) g
> > +  |
> > +  o  13:b449568bf7fc (draft) f
> > +  |
> > +  o  12:40db8afa467b (public) c
> > +  |
> > +  o  0:cb9a9f314b8b (public) a
> > +  
> > +  $ hg histedit -r 'b449568bf7fc' --commands - << EOF
> > +  > pick b449568bf7fc 13 f
> > +  > pick 7395e1ff83bd 15 h
> > +  > pick 6b70183d2492 14 g
> > +  > pick b605fb7503f2 16 i
> > +  > pick 3a6c53ee7f3d 17 j
> > +  > edit ee118ab9fa44 18 k
> > +  > EOF
> > +  Editing (ee118ab9fa44), you may commit or record as needed now.
> > +  (hg histedit --continue to resume)
> > +  [1]
> > +  $ hg histedit --continue
> > +  $ hg log -G
> > +  @  23:175d6b286a22 (secret) k
> > +  |
> > +  o  22:44ca09d59ae4 (secret) j
> > +  |
> > +  o  21:31747692a644 (secret) i
> > +  |
> > +  o  20:9985cd4f21fa (draft) g
> > +  |
> > +  o  19:4dc06258baa6 (draft) h
> > +  |
> > +  o  13:b449568bf7fc (draft) f
> > +  |
> > +  o  12:40db8afa467b (public) c
> > +  |
> > +  o  0:cb9a9f314b8b (public) a
> > +

On 03/26/2017 08:42 PM, Jun Wu wrote:
> Excerpts from Jun Wu's message of 2017-03-26 09:59:43 -0700:
>> I'm -1 on this series.
>>
>> If the goal is to workaround obsolete cycles, the code could do additional
>> checks "if the destination is obsoleted" and do "hg touch"-like thing
>> automatically.
>
> FYI I have sent the series doing the above.

I've commented on that series too, your new behavior still introduce 
issue in distributed case.

I think we should backout for now (since this is a regression, that 
exist on default right now).
In the mean time we take a step back and think about the situation cold 
headed.

See comment on the other series for details,

Cheers,

Excerpts from Pierre-Yves David's message of 2017-03-27 09:19:52 +0200:
> 
> On 03/26/2017 08:42 PM, Jun Wu wrote:
> > Excerpts from Jun Wu's message of 2017-03-26 09:59:43 -0700:
> >> I'm -1 on this series.
> >>
> >> If the goal is to workaround obsolete cycles, the code could do additional
> >> checks "if the destination is obsoleted" and do "hg touch"-like thing
> >> automatically.
> >
> > FYI I have sent the series doing the above.
> 
> I've commented on that series too, your new behavior still introduce 
> issue in distributed case.
> 
> I think we should backout for now (since this is a regression, that 
> exist on default right now).
> In the mean time we take a step back and think about the situation cold 
> headed.
> 
> See comment on the other series for details,
> 
> Cheers,
> 

I have resolved the issue you commented at [1]. I'm still not convinced that
we have to go back to use strip.

[1]: https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-March/095688.html

On 03/27/2017 12:31 PM, Jun Wu wrote:
> Excerpts from Pierre-Yves David's message of 2017-03-27 09:19:52 +0200:
>>
>> On 03/26/2017 08:42 PM, Jun Wu wrote:
>>> Excerpts from Jun Wu's message of 2017-03-26 09:59:43 -0700:
>>>> I'm -1 on this series.
>>>>
>>>> If the goal is to workaround obsolete cycles, the code could do additional
>>>> checks "if the destination is obsoleted" and do "hg touch"-like thing
>>>> automatically.
>>>
>>> FYI I have sent the series doing the above.
>>
>> I've commented on that series too, your new behavior still introduce
>> issue in distributed case.
>>
>> I think we should backout for now (since this is a regression, that
>> exist on default right now).
>> In the mean time we take a step back and think about the situation cold
>> headed.
>>
>> See comment on the other series for details,
>>
>> Cheers,
>>
>
> I have resolved the issue you commented at [1].

Actually no there are still issues. Let us channel the discussion here, 
spreading the discussion across multiple threads is getting confusing. 
Especially when series rely on other unlanded series.

> I'm still not convinced that we have to go back to use strip.

So far, we have a regression at hand and no known path forward fully 
solving the issue. I would like you to slow down and discuss possible 
alternative to backout in this thread. Please to not rush to writing 
code and do not send a new series until we reached a consensus here on 
the problem space and a viable solution.

Cheers,

On Mon, Mar 27, 2017 at 02:04:44PM +0200, Pierre-Yves David wrote:
>
>
> On 03/27/2017 12:31 PM, Jun Wu wrote:
> > Excerpts from Pierre-Yves David's message of 2017-03-27 09:19:52 +0200:
> > >
> > > On 03/26/2017 08:42 PM, Jun Wu wrote:
> > > > Excerpts from Jun Wu's message of 2017-03-26 09:59:43 -0700:
> > > > > I'm -1 on this series.
> > > > >
> > > > > If the goal is to workaround obsolete cycles, the code could do additional
> > > > > checks "if the destination is obsoleted" and do "hg touch"-like thing
> > > > > automatically.
> > > >
> > > > FYI I have sent the series doing the above.
> > >
> > > I've commented on that series too, your new behavior still introduce
> > > issue in distributed case.
> > >
> > > I think we should backout for now (since this is a regression, that
> > > exist on default right now).
> > > In the mean time we take a step back and think about the situation cold
> > > headed.
> > >
> > > See comment on the other series for details,
> > >
> > > Cheers,
> > >
> >
> > I have resolved the issue you commented at [1].
>
> Actually no there are still issues. Let us channel the discussion here,
> spreading the discussion across multiple threads is getting confusing.
> Especially when series rely on other unlanded series.
>
> > I'm still not convinced that we have to go back to use strip.
>
> So far, we have a regression at hand and no known path forward fully solving
> the issue. I would like you to slow down and discuss possible alternative to
> backout in this thread. Please to not rush to writing code and do not send a
> new series until we reached a consensus here on the problem space and a
> viable solution.

Pierre-Yves is right: this is a regression, and needs to be fixed
before 4.2 is final. Given that the freeze is in three weeks, I'm
going to take this series and if we can work out a more elegant fix in
the next couple of weeks I'll be very happy.

In general, regressions are The Worst. We've got a few open regression
bugs in the BTS that have been open unacceptably long, and I'm
inclined to be aggressive about avoiding any more cases like that.

(Jun: don't take my acceptance of these changes as a rejection of your
ideas! It's just temporary damage control until we get a better
solution in place.)

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