Patchwork [STABLE] histedit: save manually edited commit message into ".hg/last-message.txt"

login
register
mail settings
Submitter Katsunori FUJIWARA
Date March 23, 2014, 4:01 p.m.
Message ID <dd35a6ff046d4516384e.1395590465@juju>
Download mbox | patch
Permalink /patch/4042/
State Superseded
Commit a0f437e2f5a900fccd7812e66ce27b68a6ac4fad
Headers show

Comments

Katsunori FUJIWARA - March 23, 2014, 4:01 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1395590238 -32400
#      Mon Mar 24 00:57:18 2014 +0900
# Branch stable
# Node ID dd35a6ff046d4516384e32f55741b474b35f72b6
# Parent  e5641536e4d509b2dc5fab783344f86ea61b17c2
histedit: save manually edited commit message into ".hg/last-message.txt"

Before this patch, manually edited commit message for "message"
command in histedit-ing is not saved into ".hg/last-message.txt" until
it is saved by "localrepository.savecommitmessage()" in
"localrepository.commit()".

This may lose such commit message, if unexpected exception is raised.

This patch saves manually edited commit message for "message" comand
in histedit-ing into ".hg/last-message.txt" just after user editing.

This is the simplest implementation to fix on stable. Editing and
saving commit message should be centralized into the framework of
"localrepository.commit()" with "editor" argument in the future.

This patch uses repository wrapping class for exception raising before
saving commit message in "localrepository.commit()" easily and
certainly, because such exception requires corner case condition.
Katsunori FUJIWARA - March 23, 2014, 4:19 p.m.
At Mon, 24 Mar 2014 01:01:05 +0900,
FUJIWARA Katsunori wrote:
> 
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1395590238 -32400
> #      Mon Mar 24 00:57:18 2014 +0900
> # Branch stable
> # Node ID dd35a6ff046d4516384e32f55741b474b35f72b6
> # Parent  e5641536e4d509b2dc5fab783344f86ea61b17c2
> histedit: save manually edited commit message into ".hg/last-message.txt"

This fixes one more "last-message.txt" saving problem which I
overlooked in the series already imported into stable branch.


> Before this patch, manually edited commit message for "message"
> command in histedit-ing is not saved into ".hg/last-message.txt" until
> it is saved by "localrepository.savecommitmessage()" in
> "localrepository.commit()".
> 
> This may lose such commit message, if unexpected exception is raised.
> 
> This patch saves manually edited commit message for "message" comand
> in histedit-ing into ".hg/last-message.txt" just after user editing.
> 
> This is the simplest implementation to fix on stable. Editing and
> saving commit message should be centralized into the framework of
> "localrepository.commit()" with "editor" argument in the future.
> 
> This patch uses repository wrapping class for exception raising before
> saving commit message in "localrepository.commit()" easily and
> certainly, because such exception requires corner case condition.
> 
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -398,6 +398,7 @@
>              _('Fix up the change and run hg histedit --continue'))
>      message = oldctx.description() + '\n'
>      message = ui.edit(message, ui.username())
> +    repo.savecommitmessage(message)
>      commit = commitfuncfor(repo, oldctx)
>      new = commit(text=message, user=oldctx.user(), date=oldctx.date(),
>                   extra=oldctx.extra())
> diff --git a/tests/test-histedit-edit.t b/tests/test-histedit-edit.t
> --- a/tests/test-histedit-edit.t
> +++ b/tests/test-histedit-edit.t
> @@ -189,6 +189,49 @@
>    
>  
>  modify the message
> +
> +check saving last-message.txt, at first
> +
> +  $ cat > $TESTDIR/commitfailure.py <<EOF
> +  > from mercurial import util
> +  > def reposetup(ui, repo):
> +  >     class commitfailure(repo.__class__):
> +  >         def commit(self, *args, **kwargs):
> +  >             raise util.Abort('emulating unexpected abort')
> +  >     repo.__class__ = commitfailure
> +  > EOF
> +  $ cat > .hg/hgrc <<EOF
> +  > [extensions]
> +  > commitfailure = $TESTDIR/commitfailure.py
> +  > EOF
> +
> +  $ cat > $TESTDIR/editor.sh <<EOF
> +  > echo "==== before editing"
> +  > cat \$1
> +  > echo "===="
> +  > echo "check saving last-message.txt" >> \$1
> +  > EOF
> +  $ rm -f .hg/last-message.txt
> +  $ HGEDITOR="sh $TESTDIR/editor.sh" hg histedit tip --commands - 2>&1 << EOF | fixbundle
> +  > mess 1fd3b2fe7754 f
> +  > EOF
> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  ==== before editing
> +  f
> +  ====
> +  abort: emulating unexpected abort
> +  $ cat .hg/last-message.txt
> +  f
> +  check saving last-message.txt
> +
> +  $ cat > .hg/hgrc <<EOF
> +  > [extensions]
> +  > commitfailure = !
> +  > EOF
> +  $ hg histedit --abort -q
> +
> +then, check "modify the message" itself
> +
>    $ hg histedit tip --commands - 2>&1 << EOF | fixbundle
>    > mess 1fd3b2fe7754 f
>    > EOF
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Matt Mackall - April 1, 2014, 9:57 p.m.
On Mon, 2014-03-24 at 01:01 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1395590238 -32400
> #      Mon Mar 24 00:57:18 2014 +0900
> # Branch stable
> # Node ID dd35a6ff046d4516384e32f55741b474b35f72b6
> # Parent  e5641536e4d509b2dc5fab783344f86ea61b17c2
> histedit: save manually edited commit message into ".hg/last-message.txt"

Queued for stable, thanks.
Matt Mackall - April 1, 2014, 10:07 p.m.
On Mon, 2014-03-24 at 01:01 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1395590238 -32400
> #      Mon Mar 24 00:57:18 2014 +0900
> # Branch stable
> # Node ID dd35a6ff046d4516384e32f55741b474b35f72b6
> # Parent  e5641536e4d509b2dc5fab783344f86ea61b17c2
> histedit: save manually edited commit message into ".hg/last-message.txt"

Oops, I see I already queued a later version of this (that magically
managed not to conflict). Dropped.
Katsunori FUJIWARA - April 2, 2014, 12:17 p.m.
At Tue, 01 Apr 2014 17:07:42 -0500,
Matt Mackall wrote:
> 
> On Mon, 2014-03-24 at 01:01 +0900, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1395590238 -32400
> > #      Mon Mar 24 00:57:18 2014 +0900
> > # Branch stable
> > # Node ID dd35a6ff046d4516384e32f55741b474b35f72b6
> > # Parent  e5641536e4d509b2dc5fab783344f86ea61b17c2
> > histedit: save manually edited commit message into ".hg/last-message.txt"
> 
> Oops, I see I already queued a later version of this (that magically
> managed not to conflict). Dropped.

If you mean 5d22cadd1938 as "a later version", this patch and
5d22cadd1938 are different each other.
 
    http://selenic.com/repo/hg/rev/5d22cadd1938

5d22cadd1938 fixes the problem in "fold" of histedit-ing, and this
patch fixes one in "message".

I noticed my overlooking this problem after your queuing my series
including 5d22cadd1938.

Sorry for ambiguous summary line, but 80 characters are not enough for
me to describe not only purpose of patch but also difference between
them, :-)

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Pierre-Yves David - April 14, 2014, 5:34 a.m.
On 04/02/2014 08:17 AM, FUJIWARA Katsunori wrote:
>
> At Tue, 01 Apr 2014 17:07:42 -0500,
> Matt Mackall wrote:
>>
>> On Mon, 2014-03-24 at 01:01 +0900, FUJIWARA Katsunori wrote:
>>> # HG changeset patch
>>> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
>>> # Date 1395590238 -32400
>>> #      Mon Mar 24 00:57:18 2014 +0900
>>> # Branch stable
>>> # Node ID dd35a6ff046d4516384e32f55741b474b35f72b6
>>> # Parent  e5641536e4d509b2dc5fab783344f86ea61b17c2
>>> histedit: save manually edited commit message into ".hg/last-message.txt"
>>
>> Oops, I see I already queued a later version of this (that magically
>> managed not to conflict). Dropped.
>
> If you mean 5d22cadd1938 as "a later version", this patch and
> 5d22cadd1938 are different each other.
>
>      http://selenic.com/repo/hg/rev/5d22cadd1938
>
> 5d22cadd1938 fixes the problem in "fold" of histedit-ing, and this
> patch fixes one in "message".
>
> I noticed my overlooking this problem after your queuing my series
> including 5d22cadd1938.
>
> Sorry for ambiguous summary line, but 80 characters are not enough for
> me to describe not only purpose of patch but also difference between
> them, :-)

I've re-clowncopterized this for stable. Consider sending a proper patch 
in default before the end of the week.

Thanks for the patch! This family of bug is usually very frustrating.
Katsunori FUJIWARA - April 14, 2014, 5:07 p.m.
At Mon, 14 Apr 2014 01:34:35 -0400,
Pierre-Yves David wrote:
> 
> On 04/02/2014 08:17 AM, FUJIWARA Katsunori wrote:
> >
> > At Tue, 01 Apr 2014 17:07:42 -0500,
> > Matt Mackall wrote:
> >>
> >> On Mon, 2014-03-24 at 01:01 +0900, FUJIWARA Katsunori wrote:
> >>> # HG changeset patch
> >>> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> >>> # Date 1395590238 -32400
> >>> #      Mon Mar 24 00:57:18 2014 +0900
> >>> # Branch stable
> >>> # Node ID dd35a6ff046d4516384e32f55741b474b35f72b6
> >>> # Parent  e5641536e4d509b2dc5fab783344f86ea61b17c2
> >>> histedit: save manually edited commit message into ".hg/last-message.txt"
> >>
> >> Oops, I see I already queued a later version of this (that magically
> >> managed not to conflict). Dropped.
> >
> > If you mean 5d22cadd1938 as "a later version", this patch and
> > 5d22cadd1938 are different each other.
> >
> >      http://selenic.com/repo/hg/rev/5d22cadd1938
> >
> > 5d22cadd1938 fixes the problem in "fold" of histedit-ing, and this
> > patch fixes one in "message".
> >
> > I noticed my overlooking this problem after your queuing my series
> > including 5d22cadd1938.
> >
> > Sorry for ambiguous summary line, but 80 characters are not enough for
> > me to describe not only purpose of patch but also difference between
> > them, :-)
> 
> I've re-clowncopterized this for stable. Consider sending a proper patch 
> in default before the end of the week.
> 
> Thanks for the patch! This family of bug is usually very frustrating.

Thank you for your comments.

And please let me confirm some points.

  (1) is this patch already queued ?

    According to Augie's post, "being clowncopterized" means "being
    queued".

      http://selenic.com/pipermail/mercurial-devel/2014-April/057954.html

    But you also said "consider sending a proper patch ....". Does
    this mean that this patch is not yet queued (and you recommend me
    to resend) ?

  (2) can it be canceled ?

    I forgot that this patch includes problem fixed by Sean's
    e259d4c462b5 (using TESTDIR instead of TESTTMP incorrectly) at
    this replying.

    If it can be canceled, I'll resend revised version of
    this. Otherwise, I'll send new one fixing this problem after
    appearance of this patch in the official public repositories.

  (3) is this patch suitable for "default" (as you recommend to
      resend) ?

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Pierre-Yves David - April 14, 2014, 5:14 p.m.
On 04/14/2014 01:07 PM, FUJIWARA Katsunori wrote:
>
> At Mon, 14 Apr 2014 01:34:35 -0400,
> Pierre-Yves David wrote:
>>
>> On 04/02/2014 08:17 AM, FUJIWARA Katsunori wrote:
>>>
>>> At Tue, 01 Apr 2014 17:07:42 -0500,
>>> Matt Mackall wrote:
>>>>
>>>> On Mon, 2014-03-24 at 01:01 +0900, FUJIWARA Katsunori wrote:
>>>>> # HG changeset patch
>>>>> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
>>>>> # Date 1395590238 -32400
>>>>> #      Mon Mar 24 00:57:18 2014 +0900
>>>>> # Branch stable
>>>>> # Node ID dd35a6ff046d4516384e32f55741b474b35f72b6
>>>>> # Parent  e5641536e4d509b2dc5fab783344f86ea61b17c2
>>>>> histedit: save manually edited commit message into ".hg/last-message.txt"
>>>>
>>>> Oops, I see I already queued a later version of this (that magically
>>>> managed not to conflict). Dropped.
>>>
>>> If you mean 5d22cadd1938 as "a later version", this patch and
>>> 5d22cadd1938 are different each other.
>>>
>>>       http://selenic.com/repo/hg/rev/5d22cadd1938
>>>
>>> 5d22cadd1938 fixes the problem in "fold" of histedit-ing, and this
>>> patch fixes one in "message".
>>>
>>> I noticed my overlooking this problem after your queuing my series
>>> including 5d22cadd1938.
>>>
>>> Sorry for ambiguous summary line, but 80 characters are not enough for
>>> me to describe not only purpose of patch but also difference between
>>> them, :-)
>>
>> I've re-clowncopterized this for stable. Consider sending a proper patch
>> in default before the end of the week.
>>
>> Thanks for the patch! This family of bug is usually very frustrating.
>
> Thank you for your comments.
>
> And please let me confirm some points.
>
>    (1) is this patch already queued ?

yes it is available there: http://42.netv6.net/clowncopter/

>
>      According to Augie's post, "being clowncopterized" means "being
>      queued".
>
>        http://selenic.com/pipermail/mercurial-devel/2014-April/057954.html
>
>      But you also said "consider sending a proper patch ....". Does
>      this mean that this patch is not yet queued (and you recommend me
>      to resend) ?

You description say: this is a simple fix for stable. I've better idea 
for default. I would be happy to review this idea.

>
>    (2) can it be canceled ?
>
>      I forgot that this patch includes problem fixed by Sean's
>      e259d4c462b5 (using TESTDIR instead of TESTTMP incorrectly) at
>      this replying.
>
>      If it can be canceled, I'll resend revised version of
>      this. Otherwise, I'll send new one fixing this problem after
>      appearance of this patch in the official public repositories.

It did not made it into main yet, it can be rewritten or dropped.

>
>    (3) is this patch suitable for "default" (as you recommend to
>        resend) ?

not sures this matters since the freeze is in less than 100 hours
Katsunori FUJIWARA - April 14, 2014, 5:56 p.m.
At Mon, 14 Apr 2014 13:14:38 -0400,
Pierre-Yves David wrote:
> 
> On 04/14/2014 01:07 PM, FUJIWARA Katsunori wrote:
> >
> > At Mon, 14 Apr 2014 01:34:35 -0400,
> > Pierre-Yves David wrote:
> >>
> >> On 04/02/2014 08:17 AM, FUJIWARA Katsunori wrote:
> >>>
> >>> At Tue, 01 Apr 2014 17:07:42 -0500,
> >>> Matt Mackall wrote:
> >>>>
> >>>> On Mon, 2014-03-24 at 01:01 +0900, FUJIWARA Katsunori wrote:
> >>>>> # HG changeset patch
> >>>>> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> >>>>> # Date 1395590238 -32400
> >>>>> #      Mon Mar 24 00:57:18 2014 +0900
> >>>>> # Branch stable
> >>>>> # Node ID dd35a6ff046d4516384e32f55741b474b35f72b6
> >>>>> # Parent  e5641536e4d509b2dc5fab783344f86ea61b17c2
> >>>>> histedit: save manually edited commit message into ".hg/last-message.txt"
> >>>>
> >>>> Oops, I see I already queued a later version of this (that magically
> >>>> managed not to conflict). Dropped.
> >>>
> >>> If you mean 5d22cadd1938 as "a later version", this patch and
> >>> 5d22cadd1938 are different each other.
> >>>
> >>>       http://selenic.com/repo/hg/rev/5d22cadd1938
> >>>
> >>> 5d22cadd1938 fixes the problem in "fold" of histedit-ing, and this
> >>> patch fixes one in "message".
> >>>
> >>> I noticed my overlooking this problem after your queuing my series
> >>> including 5d22cadd1938.
> >>>
> >>> Sorry for ambiguous summary line, but 80 characters are not enough for
> >>> me to describe not only purpose of patch but also difference between
> >>> them, :-)
> >>
> >> I've re-clowncopterized this for stable. Consider sending a proper patch
> >> in default before the end of the week.
> >>
> >> Thanks for the patch! This family of bug is usually very frustrating.
> >
> > Thank you for your comments.
> >
> > And please let me confirm some points.
> >
> >    (1) is this patch already queued ?
> 
> yes it is available there: http://42.netv6.net/clowncopter/
> 
> >
> >      According to Augie's post, "being clowncopterized" means "being
> >      queued".
> >
> >        http://selenic.com/pipermail/mercurial-devel/2014-April/057954.html
> >
> >      But you also said "consider sending a proper patch ....". Does
> >      this mean that this patch is not yet queued (and you recommend me
> >      to resend) ?
> 
> You description say: this is a simple fix for stable. I've better idea 
> for default. I would be happy to review this idea.
> 
> >
> >    (2) can it be canceled ?
> >
> >      I forgot that this patch includes problem fixed by Sean's
> >      e259d4c462b5 (using TESTDIR instead of TESTTMP incorrectly) at
> >      this replying.
> >
> >      If it can be canceled, I'll resend revised version of
> >      this. Otherwise, I'll send new one fixing this problem after
> >      appearance of this patch in the official public repositories.
> 
> It did not made it into main yet, it can be rewritten or dropped.

Then, please prevent it from being made into main jet :-)

I'll send revised version of it in a couple of 10 minutes (now running
"make tests" ...)

> >
> >    (3) is this patch suitable for "default" (as you recommend to
> >        resend) ?
> 
> not sures this matters since the freeze is in less than 100 hours
> 
> -- 
> Pierre-Yves David
> 


----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Pierre-Yves David - April 14, 2014, 6:14 p.m.
On 04/14/2014 01:56 PM, FUJIWARA Katsunori wrote:
> I'll send revised version of it in a couple of 10 minutes (now running
> "make tests" ...)

Wait, what? You have not access to a Machine running the test suite in 2 
minutes?

I'm sure Henrik Stuart can fix that.
Katsunori FUJIWARA - April 14, 2014, 6:46 p.m.
At Mon, 14 Apr 2014 14:14:46 -0400,
Pierre-Yves David wrote:
> 
> On 04/14/2014 01:56 PM, FUJIWARA Katsunori wrote:
> > I'll send revised version of it in a couple of 10 minutes (now running
> > "make tests" ...)
> 
> Wait, what? You have not access to a Machine running the test suite in 2 
> minutes?
> 
> I'm sure Henrik Stuart can fix that.

It needs about 10 minutes to finish running the whole test suite,
because I use the virtual machine on a little old desktop machine for
environment isolation.

In addition to it, I usually send the patch series to my own e-mail
account once or twice with a little interval for confirmation (and
cooling my brain down) before sending to mercurial-devel :-)

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -398,6 +398,7 @@ 
             _('Fix up the change and run hg histedit --continue'))
     message = oldctx.description() + '\n'
     message = ui.edit(message, ui.username())
+    repo.savecommitmessage(message)
     commit = commitfuncfor(repo, oldctx)
     new = commit(text=message, user=oldctx.user(), date=oldctx.date(),
                  extra=oldctx.extra())
diff --git a/tests/test-histedit-edit.t b/tests/test-histedit-edit.t
--- a/tests/test-histedit-edit.t
+++ b/tests/test-histedit-edit.t
@@ -189,6 +189,49 @@ 
   
 
 modify the message
+
+check saving last-message.txt, at first
+
+  $ cat > $TESTDIR/commitfailure.py <<EOF
+  > from mercurial import util
+  > def reposetup(ui, repo):
+  >     class commitfailure(repo.__class__):
+  >         def commit(self, *args, **kwargs):
+  >             raise util.Abort('emulating unexpected abort')
+  >     repo.__class__ = commitfailure
+  > EOF
+  $ cat > .hg/hgrc <<EOF
+  > [extensions]
+  > commitfailure = $TESTDIR/commitfailure.py
+  > EOF
+
+  $ cat > $TESTDIR/editor.sh <<EOF
+  > echo "==== before editing"
+  > cat \$1
+  > echo "===="
+  > echo "check saving last-message.txt" >> \$1
+  > EOF
+  $ rm -f .hg/last-message.txt
+  $ HGEDITOR="sh $TESTDIR/editor.sh" hg histedit tip --commands - 2>&1 << EOF | fixbundle
+  > mess 1fd3b2fe7754 f
+  > EOF
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  ==== before editing
+  f
+  ====
+  abort: emulating unexpected abort
+  $ cat .hg/last-message.txt
+  f
+  check saving last-message.txt
+
+  $ cat > .hg/hgrc <<EOF
+  > [extensions]
+  > commitfailure = !
+  > EOF
+  $ hg histedit --abort -q
+
+then, check "modify the message" itself
+
   $ hg histedit tip --commands - 2>&1 << EOF | fixbundle
   > mess 1fd3b2fe7754 f
   > EOF