Patchwork [evolve-ext] uncommit: abort when rev specifies the current changeset

login
register
mail settings
Submitter Nathan Goldbaum
Date Oct. 29, 2014, 1:07 a.m.
Message ID <34989d80484df48893be.1414544862@ROUS>
Download mbox | patch
Permalink /patch/6490/
State Not Applicable
Headers show

Comments

Nathan Goldbaum - Oct. 29, 2014, 1:07 a.m.
# HG changeset patch
# User Nathan Goldbaum <ngoldbau@ucsc.edu>
# Date 1414544788 25200
#      Tue Oct 28 18:06:28 2014 -0700
# Node ID 34989d80484df48893be948ff0a47dd33aaeaf2e
# Parent  b7d85cd8ec7bd5a4ba5a61d156745269a999450b
uncommit: abort when rev specifies the current changeset

The uncommit command now fails when the rev argument indicates the current
changeset. Previously this aborted in the changeset obsolescence machinery when
the current commit tries to obsolete itself.

Using the rev argument like this indicates a misunderstanding, so printing an
uncommit-specific error should make it easier to understand what went wrong.
Pierre-Yves David - Nov. 1, 2014, 1:52 p.m.
On 10/29/2014 01:07 AM, Nathan Goldbaum wrote:
> # HG changeset patch
> # User Nathan Goldbaum <ngoldbau@ucsc.edu>
> # Date 1414544788 25200
> #      Tue Oct 28 18:06:28 2014 -0700
> # Node ID 34989d80484df48893be948ff0a47dd33aaeaf2e
> # Parent  b7d85cd8ec7bd5a4ba5a61d156745269a999450b
> uncommit: abort when rev specifies the current changeset
>
> The uncommit command now fails when the rev argument indicates the current
> changeset. Previously this aborted in the changeset obsolescence machinery when
> the current commit tries to obsolete itself.
>
> Using the rev argument like this indicates a misunderstanding, so printing an
> uncommit-specific error should make it easier to understand what went wrong.

The intend looks good. We should probably check for both p1 and p2. (not 
just p1).

The message is a bit cryptic to me.

    abort: cannot reapply changes to the current changeset

We should probably mention "uncommit" and "parent" in the error. Simple 
proposal:

   abort: uncommit to parent make no sense

>
> diff -r b7d85cd8ec7b -r 34989d80484d hgext/evolve.py
> --- a/hgext/evolve.py	Sat Oct 25 22:25:42 2014 -0400
> +++ b/hgext/evolve.py	Tue Oct 28 18:06:28 2014 -0700
> @@ -1979,6 +1979,9 @@
>           rev = None
>           if opts.get('rev'):
>               rev = scmutil.revsingle(repo, opts.get('rev'))
> +            if repo[None].p1() == rev:
> +                raise util.Abort(_('cannot reapply changes to the current '
> +                                   'changeset'))
>
>           # Recommit the filtered changeset
>           tr = repo.transaction('uncommit')
> diff -r b7d85cd8ec7b -r 34989d80484d tests/test-uncommit.t
> --- a/tests/test-uncommit.t	Sat Oct 25 22:25:42 2014 -0400
> +++ b/tests/test-uncommit.t	Tue Oct 28 18:06:28 2014 -0700
> @@ -347,6 +347,9 @@
>     $ hg cat b --rev 0
>     b: no such file in rev 07f494440405
>     [1]
> +  $ hg uncommit --rev . b
> +  abort: cannot reapply changes to the current changeset
> +  [255]
>     $ hg uncommit --rev 0 b
>     $ hg cat b --rev .
>     b: no such file in rev 5b27f6b17da2
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Sean Farley - Nov. 1, 2014, 5:40 p.m.
Pierre-Yves David writes:

> On 10/29/2014 01:07 AM, Nathan Goldbaum wrote:
>> # HG changeset patch
>> # User Nathan Goldbaum <ngoldbau@ucsc.edu>
>> # Date 1414544788 25200
>> #      Tue Oct 28 18:06:28 2014 -0700
>> # Node ID 34989d80484df48893be948ff0a47dd33aaeaf2e
>> # Parent  b7d85cd8ec7bd5a4ba5a61d156745269a999450b
>> uncommit: abort when rev specifies the current changeset
>>
>> The uncommit command now fails when the rev argument indicates the current
>> changeset. Previously this aborted in the changeset obsolescence machinery when
>> the current commit tries to obsolete itself.
>>
>> Using the rev argument like this indicates a misunderstanding, so printing an
>> uncommit-specific error should make it easier to understand what went wrong.
>
> The intend looks good. We should probably check for both p1 and p2. (not 
> just p1).
>
> The message is a bit cryptic to me.
>
>     abort: cannot reapply changes to the current changeset
>
> We should probably mention "uncommit" and "parent" in the error. Simple 
> proposal:
>
>    abort: uncommit to parent make no sense

I've been following this patch discussion on IRC and must admit that I
am a bit confused. How is 'hg uncommit foo' different from 'hg uncommit
--rev . foo'?
Pierre-Yves David - Nov. 1, 2014, 5:42 p.m.
On 11/01/2014 05:40 PM, Sean Farley wrote:
> I've been following this patch discussion on IRC and must admit that I
> am a bit confused. How is 'hg uncommit foo' different from 'hg uncommit
> --rev . foo'?

`hg uncommit foo` is `hg uncommit --rev .^ foo`
Sean Farley - Nov. 1, 2014, 5:46 p.m.
Pierre-Yves David writes:

> On 11/01/2014 05:40 PM, Sean Farley wrote:
>> I've been following this patch discussion on IRC and must admit that I
>> am a bit confused. How is 'hg uncommit foo' different from 'hg uncommit
>> --rev . foo'?
>
> `hg uncommit foo` is `hg uncommit --rev .^ foo`

Ah, a misunderstanding with what --rev does. Ok, thanks.

Patch

diff -r b7d85cd8ec7b -r 34989d80484d hgext/evolve.py
--- a/hgext/evolve.py	Sat Oct 25 22:25:42 2014 -0400
+++ b/hgext/evolve.py	Tue Oct 28 18:06:28 2014 -0700
@@ -1979,6 +1979,9 @@ 
         rev = None
         if opts.get('rev'):
             rev = scmutil.revsingle(repo, opts.get('rev'))
+            if repo[None].p1() == rev:
+                raise util.Abort(_('cannot reapply changes to the current '
+                                   'changeset'))
 
         # Recommit the filtered changeset
         tr = repo.transaction('uncommit')
diff -r b7d85cd8ec7b -r 34989d80484d tests/test-uncommit.t
--- a/tests/test-uncommit.t	Sat Oct 25 22:25:42 2014 -0400
+++ b/tests/test-uncommit.t	Tue Oct 28 18:06:28 2014 -0700
@@ -347,6 +347,9 @@ 
   $ hg cat b --rev 0
   b: no such file in rev 07f494440405
   [1]
+  $ hg uncommit --rev . b
+  abort: cannot reapply changes to the current changeset
+  [255]
   $ hg uncommit --rev 0 b
   $ hg cat b --rev .
   b: no such file in rev 5b27f6b17da2