Patchwork commit: block amend while histedit is in progress (issue4800)

login
register
mail settings
Submitter timeless@mozdev.org
Date Feb. 14, 2016, 7:36 a.m.
Message ID <800740823f9d3c5ac6cd.1455435390@waste.org>
Download mbox | patch
Permalink /patch/13180/
State Superseded
Commit f6b5b041c6c9a0d33bbc4115d47cafc4ff701411
Headers show

Comments

timeless@mozdev.org - Feb. 14, 2016, 7:36 a.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1455435350 0
#      Sun Feb 14 07:35:50 2016 +0000
# Node ID 800740823f9d3c5ac6cd349139405b83e35d5da9
# Parent  a036e1ae1fbe88ab99cb861ebfc2e4da7a3912ca
commit: block amend while histedit is in progress (issue4800)

Currently histedit gets confused if an amend happens while histedit
is in progress. Since we have a checkunfinished command, we are
temporarily honoring it.

Note: eventually this guard will be removed. Please do not expect
this behavior to remain.
Pierre-Yves David - Feb. 14, 2016, 6:14 p.m.
On 02/14/2016 07:36 AM, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1455435350 0
> #      Sun Feb 14 07:35:50 2016 +0000
> # Node ID 800740823f9d3c5ac6cd349139405b83e35d5da9
> # Parent  a036e1ae1fbe88ab99cb861ebfc2e4da7a3912ca
> commit: block amend while histedit is in progress (issue4800)
>
> Currently histedit gets confused if an amend happens while histedit
> is in progress. Since we have a checkunfinished command, we are
> temporarily honoring it.
>
> Note: eventually this guard will be removed. Please do not expect
> this behavior to remain.

This seems a sensible reaction until we have a better fix. But this is 
quite a usuability bummer as, for example it will prevent people to 
amend their -new- commit during an edit.

I'm CCing Kostia who is working on a fix for issue4800. Maybe it will be 
ready soon enough so that we don't need this patch ?

As more long terms vision, we should probably grow "checkrewrite" that 
would check if it sensible to rewrite a changeset or not.|
Pierre-Yves David - Feb. 26, 2016, 6:09 p.m.
On 02/14/2016 07:14 PM, Pierre-Yves David wrote:
>
>
> On 02/14/2016 07:36 AM, timeless wrote:
>> # HG changeset patch
>> # User timeless <timeless@mozdev.org>
>> # Date 1455435350 0
>> #      Sun Feb 14 07:35:50 2016 +0000
>> # Node ID 800740823f9d3c5ac6cd349139405b83e35d5da9
>> # Parent  a036e1ae1fbe88ab99cb861ebfc2e4da7a3912ca
>> commit: block amend while histedit is in progress (issue4800)
>>
>> Currently histedit gets confused if an amend happens while histedit
>> is in progress. Since we have a checkunfinished command, we are
>> temporarily honoring it.
>>
>> Note: eventually this guard will be removed. Please do not expect
>> this behavior to remain.
>
> This seems a sensible reaction until we have a better fix. But this is
> quite a usuability bummer as, for example it will prevent people to
> amend their -new- commit during an edit.
>
> I'm CCing Kostia who is working on a fix for issue4800. Maybe it will be
> ready soon enough so that we don't need this patch ?
>
> As more long terms vision, we should probably grow "checkrewrite" that
> would check if it sensible to rewrite a changeset or not.|

So, Kostia landed a better fix (using obsolescence history to update 
replacement instead of crashing) for evolution user. But we still need 
something for user without evolution. Disabling commit --amend as a 
first move make sense here. Ideally we would check if the amend is 
touching something "forbiden" by reading the histedit state file, but 
this is much more complicated. So disabling ci --amend here seems sensible

Can you send a V2 ?

Cheers,
timeless - Feb. 26, 2016, 7:03 p.m.
Sent

On Fri, Feb 26, 2016 at 1:09 PM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 02/14/2016 07:14 PM, Pierre-Yves David wrote:
>>
>>
>>
>> On 02/14/2016 07:36 AM, timeless wrote:
>>>
>>> # HG changeset patch
>>> # User timeless <timeless@mozdev.org>
>>> # Date 1455435350 0
>>> #      Sun Feb 14 07:35:50 2016 +0000
>>> # Node ID 800740823f9d3c5ac6cd349139405b83e35d5da9
>>> # Parent  a036e1ae1fbe88ab99cb861ebfc2e4da7a3912ca
>>> commit: block amend while histedit is in progress (issue4800)
>>>
>>> Currently histedit gets confused if an amend happens while histedit
>>> is in progress. Since we have a checkunfinished command, we are
>>> temporarily honoring it.
>>>
>>> Note: eventually this guard will be removed. Please do not expect
>>> this behavior to remain.
>>
>>
>> This seems a sensible reaction until we have a better fix. But this is
>> quite a usuability bummer as, for example it will prevent people to
>> amend their -new- commit during an edit.
>>
>> I'm CCing Kostia who is working on a fix for issue4800. Maybe it will be
>> ready soon enough so that we don't need this patch ?
>>
>> As more long terms vision, we should probably grow "checkrewrite" that
>> would check if it sensible to rewrite a changeset or not.|
>
>
> So, Kostia landed a better fix (using obsolescence history to update
> replacement instead of crashing) for evolution user. But we still need
> something for user without evolution. Disabling commit --amend as a first
> move make sense here. Ideally we would check if the amend is touching
> something "forbiden" by reading the histedit state file, but this is much
> more complicated. So disabling ci --amend here seems sensible
>
> Can you send a V2 ?
>
> Cheers,
>
>
> --
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - Feb. 26, 2016, 9:07 p.m.
On Fri, Feb 26, 2016 at 07:09:08PM +0100, Pierre-Yves David wrote:
>
>
> On 02/14/2016 07:14 PM, Pierre-Yves David wrote:
> >
> >
> >On 02/14/2016 07:36 AM, timeless wrote:
> >># HG changeset patch
> >># User timeless <timeless@mozdev.org>
> >># Date 1455435350 0
> >>#      Sun Feb 14 07:35:50 2016 +0000
> >># Node ID 800740823f9d3c5ac6cd349139405b83e35d5da9
> >># Parent  a036e1ae1fbe88ab99cb861ebfc2e4da7a3912ca
> >>commit: block amend while histedit is in progress (issue4800)
> >>
> >>Currently histedit gets confused if an amend happens while histedit
> >>is in progress. Since we have a checkunfinished command, we are
> >>temporarily honoring it.
> >>
> >>Note: eventually this guard will be removed. Please do not expect
> >>this behavior to remain.
> >
> >This seems a sensible reaction until we have a better fix. But this is
> >quite a usuability bummer as, for example it will prevent people to
> >amend their -new- commit during an edit.
> >
> >I'm CCing Kostia who is working on a fix for issue4800. Maybe it will be
> >ready soon enough so that we don't need this patch ?
> >
> >As more long terms vision, we should probably grow "checkrewrite" that
> >would check if it sensible to rewrite a changeset or not.|
>
> So, Kostia landed a better fix (using obsolescence history to update
> replacement instead of crashing) for evolution user. But we still need
> something for user without evolution. Disabling commit --amend as a first
> move make sense here. Ideally we would check if the amend is touching
> something "forbiden" by reading the histedit state file, but this is much
> more complicated. So disabling ci --amend here seems sensible
>
> Can you send a V2 ?

I'm assuming the v2 you're asking for would only block amends for
non-evolve users. Is that correct?

(It's unclear from what I read here, so I just want to confirm.)

>
> Cheers,
>
> --
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Pierre-Yves David - Feb. 26, 2016, 9:20 p.m.
On 02/26/2016 10:07 PM, Augie Fackler wrote:
> On Fri, Feb 26, 2016 at 07:09:08PM +0100, Pierre-Yves David wrote:
>>
>>
>> On 02/14/2016 07:14 PM, Pierre-Yves David wrote:
>>>
>>>
>>> On 02/14/2016 07:36 AM, timeless wrote:
>>>> # HG changeset patch
>>>> # User timeless <timeless@mozdev.org>
>>>> # Date 1455435350 0
>>>> #      Sun Feb 14 07:35:50 2016 +0000
>>>> # Node ID 800740823f9d3c5ac6cd349139405b83e35d5da9
>>>> # Parent  a036e1ae1fbe88ab99cb861ebfc2e4da7a3912ca
>>>> commit: block amend while histedit is in progress (issue4800)
>>>>
>>>> Currently histedit gets confused if an amend happens while histedit
>>>> is in progress. Since we have a checkunfinished command, we are
>>>> temporarily honoring it.
>>>>
>>>> Note: eventually this guard will be removed. Please do not expect
>>>> this behavior to remain.
>>>
>>> This seems a sensible reaction until we have a better fix. But this is
>>> quite a usuability bummer as, for example it will prevent people to
>>> amend their -new- commit during an edit.
>>>
>>> I'm CCing Kostia who is working on a fix for issue4800. Maybe it will be
>>> ready soon enough so that we don't need this patch ?
>>>
>>> As more long terms vision, we should probably grow "checkrewrite" that
>>> would check if it sensible to rewrite a changeset or not.|
>>
>> So, Kostia landed a better fix (using obsolescence history to update
>> replacement instead of crashing) for evolution user. But we still need
>> something for user without evolution. Disabling commit --amend as a first
>> move make sense here. Ideally we would check if the amend is touching
>> something "forbiden" by reading the histedit state file, but this is much
>> more complicated. So disabling ci --amend here seems sensible
>>
>> Can you send a V2 ?
>
> I'm assuming the v2 you're asking for would only block amends for
> non-evolve users. Is that correct?
>
> (It's unclear from what I read here, so I just want to confirm.)

Exactly.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1681,6 +1681,7 @@ 
         allowunstable = obsolete.isenabled(repo, obsolete.allowunstableopt)
         if not allowunstable and old.children():
             raise error.Abort(_('cannot amend changeset with children'))
+        cmdutil.checkunfinished(repo)
 
         # commitfunc is used only for temporary amend commit by cmdutil.amend
         def commitfunc(ui, repo, message, match, opts):
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
@@ -445,3 +445,35 @@ 
   > pick 6f2f0241f119
   > pick 8cde254db839
   > EOF
+
+commit --amend should abort if histedit is in progress
+(issue4800) - This should be removed once histedit is
+able to use successor chains to figure out what happened.
+
+  $ hg -q up 8cde254db839
+  $ hg histedit 6f2f0241f119 --commands - <<EOF
+  > pick 8cde254db839
+  > edit 6f2f0241f119
+  > EOF
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  merging foo
+  warning: conflicts while merging foo! (edit, then use 'hg resolve --mark')
+  Fix up the change (pick 8cde254db839)
+  (hg histedit --continue to resume)
+  [1]
+  $ hg resolve -m --all
+  (no more unresolved files)
+  continue: hg histedit --continue
+  $ hg histedit --cont
+  merging foo
+  warning: conflicts while merging foo! (edit, then use 'hg resolve --mark')
+  Editing (6f2f0241f119), you may commit or record as needed now.
+  (hg histedit --continue to resume)
+  [1]
+  $ hg resolve -m --all
+  (no more unresolved files)
+  continue: hg histedit --continue
+  $ hg commit --amend -m 'reject this fold'
+  abort: histedit in progress
+  (use 'hg histedit --continue' or 'hg histedit --abort')
+  [255]