Patchwork [evolve-ext,v3] evolve: make split respect rev args passed without --rev or -r

login
register
mail settings
Submitter Kostia Balytskyi
Date Feb. 1, 2016, 4:46 p.m.
Message ID <a4e9419585c8137237bf.1454345195@dev1902.lla1.facebook.com>
Download mbox | patch
Permalink /patch/12936/
State Deferred
Headers show

Comments

Kostia Balytskyi - Feb. 1, 2016, 4:46 p.m.
# HG changeset patch
# User Kostia Balytskyi <ikostia@fb.com>
# Date 1454345046 28800
#      Mon Feb 01 08:44:06 2016 -0800
# Node ID a4e9419585c8137237bf268a69a8935ed2d14d26
# Parent  983f2e4dbe5d4d97d6ca82f6679913d75c1f2577
evolve: make split respect rev args passed without --rev or -r

Currently, if one runs `hg split .` or `hg split`, it will fail
with an exception. This happens becuase we only expect revision
args to be passed as --rev/-r ones and don't treat unnamed args
properly or add default values if no args are provided.
Laurent Charignon - Feb. 1, 2016, 5:06 p.m.
> On Feb 1, 2016, at 8:46 AM, Kostia Balytskyi <ikostia@fb.com> wrote:
> 
> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1454345046 28800
> #      Mon Feb 01 08:44:06 2016 -0800
> # Node ID a4e9419585c8137237bf268a69a8935ed2d14d26
> # Parent  983f2e4dbe5d4d97d6ca82f6679913d75c1f2577
> evolve: make split respect rev args passed without --rev or -r
> 
> Currently, if one runs `hg split .` or `hg split`, it will fail
> with an exception. This happens becuase we only expect revision
> args to be passed as --rev/-r ones and don't treat unnamed args
> properly or add default values if no args are provided.
> 
> diff --git a/hgext/evolve.py b/hgext/evolve.py
> --- a/hgext/evolve.py
> +++ b/hgext/evolve.py
> @@ -2729,15 +2729,12 @@
>     tr = wlock = lock = None
>     newcommits = []
> 
> -    revopt = opts.get('rev')
> -    if revopt:
> -        revs = scmutil.revrange(repo, revopt)
> -        if len(revs) != 1:
> -            raise error.Abort(_("you can only specify one revision to split"))
> -        else:
> -            rev = list(revs)[0]
> +    revarg = (list(revs) + opts.get('rev')) or '.'
> +    revs = scmutil.revrange(repo, revarg)
> +    if len(revs) != 1:
> +        raise error.Abort(_("you can only specify one revision to split"))
>     else:
> -        rev = '.'
> +        rev = list(revs)[0]
> 


This patch looks good to me.
Pierre-Yves and I had discussion about command that behave differently based on the number of results from a call to scmutil.revrange.
If I recall correctly, it is not a desirable behavior, but I don't think that we could do it differently here, isn't it Pierre-Yves?


>     try:
>         wlock = repo.wlock()
> diff --git a/tests/test-split.t b/tests/test-split.t
> --- a/tests/test-split.t
> +++ b/tests/test-split.t
> @@ -335,4 +335,41 @@
>   abort: cannot split commit: ced8fbcce3a7 not a head
>   [255]
> 
> +Changing evolution level to createmarkers
> +  $ echo "[experimental]" >> $HGRCPATH
> +  $ echo "evolution=createmarkers" >> $HGRCPATH
> 
> +Running split without any revision operates on the parent of the working copy
> +  $ hg split << EOF
> +  > q
> +  > EOF
> +  (leaving bookmark bookB)
> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  adding _d
> +  diff --git a/_d b/_d
> +  new file mode 100644
> +  examine changes to '_d'? [Ynesfdaq?] q
> +  
> +  abort: user quit
> +  [255]
> +
> +Running split with tip revision, specified as unnamed argument
> +  $ hg split . << EOF
> +  > q
> +  > EOF
> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  adding _d
> +  diff --git a/_d b/_d
> +  new file mode 100644
> +  examine changes to '_d'? [Ynesfdaq?] q
> +  
> +  abort: user quit
> +  [255]
> +
> +Running split with both unnamed and named revision arguments shows an error msg
> +  $ hg split . --rev .^ << EOF
> +  > q
> +  > EOF
> +  abort: you can only specify one revision to split
> +  [255]
> +
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Feb. 3, 2016, 9:19 p.m.
On 02/01/2016 05:06 PM, Laurent Charignon wrote:
>
>
>> On Feb 1, 2016, at 8:46 AM, Kostia Balytskyi <ikostia@fb.com> wrote:
>>
>> # HG changeset patch
>> # User Kostia Balytskyi <ikostia@fb.com>
>> # Date 1454345046 28800
>> #      Mon Feb 01 08:44:06 2016 -0800
>> # Node ID a4e9419585c8137237bf268a69a8935ed2d14d26
>> # Parent  983f2e4dbe5d4d97d6ca82f6679913d75c1f2577
>> evolve: make split respect rev args passed without --rev or -r
>>
>> Currently, if one runs `hg split .` or `hg split`, it will fail
>> with an exception. This happens becuase we only expect revision
>> args to be passed as --rev/-r ones and don't treat unnamed args
>> properly or add default values if no args are provided.
>>
>> diff --git a/hgext/evolve.py b/hgext/evolve.py
>> --- a/hgext/evolve.py
>> +++ b/hgext/evolve.py
>> @@ -2729,15 +2729,12 @@
>>      tr = wlock = lock = None
>>      newcommits = []
>>
>> -    revopt = opts.get('rev')
>> -    if revopt:
>> -        revs = scmutil.revrange(repo, revopt)
>> -        if len(revs) != 1:
>> -            raise error.Abort(_("you can only specify one revision to split"))
>> -        else:
>> -            rev = list(revs)[0]
>> +    revarg = (list(revs) + opts.get('rev')) or '.'
>> +    revs = scmutil.revrange(repo, revarg)
>> +    if len(revs) != 1:
>> +        raise error.Abort(_("you can only specify one revision to split"))
>>      else:
>> -        rev = '.'
>> +        rev = list(revs)[0]
>>
>
>
> This patch looks good to me.
> Pierre-Yves and I had discussion about command that behave differently based on the number of results from a call to scmutil.revrange.
> If I recall correctly, it is not a desirable behavior, but I don't think that we could do it differently here, isn't it Pierre-Yves?

If multiple revision are passed to a command expecting a single one, we 
should pick the highest one. That logic is implemented in 
"scmutil.revsingle". Can you get a V4 with this last thing fixed? (sorry 
for not seeing this earlier).

Patch

diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -2729,15 +2729,12 @@ 
     tr = wlock = lock = None
     newcommits = []
 
-    revopt = opts.get('rev')
-    if revopt:
-        revs = scmutil.revrange(repo, revopt)
-        if len(revs) != 1:
-            raise error.Abort(_("you can only specify one revision to split"))
-        else:
-            rev = list(revs)[0]
+    revarg = (list(revs) + opts.get('rev')) or '.'
+    revs = scmutil.revrange(repo, revarg)
+    if len(revs) != 1:
+        raise error.Abort(_("you can only specify one revision to split"))
     else:
-        rev = '.'
+        rev = list(revs)[0]
 
     try:
         wlock = repo.wlock()
diff --git a/tests/test-split.t b/tests/test-split.t
--- a/tests/test-split.t
+++ b/tests/test-split.t
@@ -335,4 +335,41 @@ 
   abort: cannot split commit: ced8fbcce3a7 not a head
   [255]
 
+Changing evolution level to createmarkers
+  $ echo "[experimental]" >> $HGRCPATH
+  $ echo "evolution=createmarkers" >> $HGRCPATH
 
+Running split without any revision operates on the parent of the working copy
+  $ hg split << EOF
+  > q
+  > EOF
+  (leaving bookmark bookB)
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  adding _d
+  diff --git a/_d b/_d
+  new file mode 100644
+  examine changes to '_d'? [Ynesfdaq?] q
+  
+  abort: user quit
+  [255]
+
+Running split with tip revision, specified as unnamed argument
+  $ hg split . << EOF
+  > q
+  > EOF
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  adding _d
+  diff --git a/_d b/_d
+  new file mode 100644
+  examine changes to '_d'? [Ynesfdaq?] q
+  
+  abort: user quit
+  [255]
+
+Running split with both unnamed and named revision arguments shows an error msg
+  $ hg split . --rev .^ << EOF
+  > q
+  > EOF
+  abort: you can only specify one revision to split
+  [255]
+