Patchwork [1,of,2,V2] revpair: smartset compatibility

login
register
mail settings
Submitter Pierre-Yves David
Date March 29, 2014, 12:53 a.m.
Message ID <545b3e8742c8f284242c.1396054382@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/4144/
State Superseded
Commit 97b2f26dfc43be63ef48f7cdddfb984ecfd462fc
Headers show

Comments

Pierre-Yves David - March 29, 2014, 12:53 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1395366265 25200
#      Thu Mar 20 18:44:25 2014 -0700
# Node ID 545b3e8742c8f284242c5d241edd375d3674bdca
# Parent  8a6a86c9a5b58ccc020de1ff0429e72dfa5599fc
revpair: smartset compatibility

Since recent revset changes, revrange now return a smartset. This smart set
probably does not support indexing (_addset does not). This led to crash.
Instead when the smartset is ordered with use the `min` and `max` method of
smart set. Otherwise we turn is into a list and use indexing on it.

The tests have been updated to catch such regression.
Simon King - March 29, 2014, 1:21 a.m.
On 29 Mar 2014, at 00:53, pierre-yves.david@ens-lyon.org wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1395366265 25200
> #      Thu Mar 20 18:44:25 2014 -0700
> # Node ID 545b3e8742c8f284242c5d241edd375d3674bdca
> # Parent  8a6a86c9a5b58ccc020de1ff0429e72dfa5599fc
> revpair: smartset compatibility
> 
> Since recent revset changes, revrange now return a smartset. This smart set
> probably does not support indexing (_addset does not). This led to crash.
> Instead when the smartset is ordered with use the `min` and `max` method of
> smart set. Otherwise we turn is into a list and use indexing on it.
> 
> The tests have been updated to catch such regression.
> 
> diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
> --- a/mercurial/scmutil.py
> +++ b/mercurial/scmutil.py
> @@ -468,17 +468,30 @@ def revpair(repo, revs):
>     if not revs:
>         return repo.dirstate.p1(), None
> 
>     l = revrange(repo, revs)
> 
> -    if len(l) == 0:
> +    if not l:
> +        first = second = None
> +    elif l.isascending():
> +        first = l.min()
> +        second = l.max()
> +    elif l.isascending():

Should this be l.isdescending()?

> +        first = l.max()
> +        second = l.min()
> +    else:
> +        l = list(l)
> +        first = l[0]
> +        second = l[-1]
> +
> +    if first is None:
>         raise util.Abort(_('empty revision range'))
> 
> -    if len(l) == 1 and len(revs) == 1 and _revrangesep not in revs[0]:
> -        return repo.lookup(l[0]), None
> +    if first==second and len(revs) == 1 and _revrangesep not in revs[0]:
> +        return repo.lookup(first), None
> 
> -    return repo.lookup(l[0]), repo.lookup(l[-1])
> +    return repo.lookup(first), repo.lookup(second)
> 
> _revrangesep = ':'
> 
> def revrange(repo, revs):
>     """Yield revision as strings from a list of revision specifications."""
> diff --git a/tests/test-revset.t b/tests/test-revset.t
> --- a/tests/test-revset.t
> +++ b/tests/test-revset.t
> @@ -742,10 +742,47 @@ multiple revspecs
>   4
>   5
>   6
>   7
> 
> +test usage in revpair (with "+")
> +
> +(real pair)
> +
> +  $ hg diff -r 'tip^^' -r 'tip'
> +  diff -r 2326846efdab -r 24286f4ae135 .hgtags
> +  --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +  +++ b/.hgtags	Thu Jan 01 00:00:00 1970 +0000
> +  @@ -0,0 +1,1 @@
> +  +e0cc66ef77e8b6f711815af4e001a6594fde3ba5 1.0
> +  $ hg diff -r 'tip^^::tip'
> +  diff -r 2326846efdab -r 24286f4ae135 .hgtags
> +  --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +  +++ b/.hgtags	Thu Jan 01 00:00:00 1970 +0000
> +  @@ -0,0 +1,1 @@
> +  +e0cc66ef77e8b6f711815af4e001a6594fde3ba5 1.0
> +
> +(single rev)
> +
> +  $ hg diff -r 'tip^' -r 'tip^'
> +  $ hg diff -r 'tip^::tip^ or tip^'
> +
> +(single rev that does not looks like a range)
> +
> +  $ hg diff -r 'tip^ or tip^'
> +  diff -r d5d0dcbdc4d9 .hgtags
> +  --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +  +++ b/.hgtags	* (glob)
> +  @@ -0,0 +1,1 @@
> +  +e0cc66ef77e8b6f711815af4e001a6594fde3ba5 1.0
> +
> +(no rev)
> +
> +  $ hg diff -r 'author("babar") or author("celeste")'
> +  abort: empty revision range
> +  [255]
> +
> aliases:
> 
>   $ echo '[revsetalias]' >> .hg/hgrc
>   $ echo 'm = merge()' >> .hg/hgrc
>   $ echo 'sincem = descendants(m)' >> .hg/hgrc
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - March 29, 2014, 3:31 a.m.
On 03/28/2014 06:21 PM, Simon King wrote:
>
> On 29 Mar 2014, at 00:53, pierre-yves.david@ens-lyon.org wrote:
>
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1395366265 25200
>> #      Thu Mar 20 18:44:25 2014 -0700
>> # Node ID 545b3e8742c8f284242c5d241edd375d3674bdca
>> # Parent  8a6a86c9a5b58ccc020de1ff0429e72dfa5599fc
>> revpair: smartset compatibility
>>
>> Since recent revset changes, revrange now return a smartset. This smart set
>> probably does not support indexing (_addset does not). This led to crash.
>> Instead when the smartset is ordered with use the `min` and `max` method of
>> smart set. Otherwise we turn is into a list and use indexing on it.
>>
>> The tests have been updated to catch such regression.
>>
>> diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
>> --- a/mercurial/scmutil.py
>> +++ b/mercurial/scmutil.py
>> @@ -468,17 +468,30 @@ def revpair(repo, revs):
>>      if not revs:
>>          return repo.dirstate.p1(), None
>>
>>      l = revrange(repo, revs)
>>
>> -    if len(l) == 0:
>> +    if not l:
>> +        first = second = None
>> +    elif l.isascending():
>> +        first = l.min()
>> +        second = l.max()
>> +    elif l.isascending():
>
> Should this be l.isdescending()?

It should >.<

Interesting enough, the code still behave properly, this branch is just 
death and descending case is handle by the unordered one.

Will re-patchbomb soon.

Patch

diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -468,17 +468,30 @@  def revpair(repo, revs):
     if not revs:
         return repo.dirstate.p1(), None
 
     l = revrange(repo, revs)
 
-    if len(l) == 0:
+    if not l:
+        first = second = None
+    elif l.isascending():
+        first = l.min()
+        second = l.max()
+    elif l.isascending():
+        first = l.max()
+        second = l.min()
+    else:
+        l = list(l)
+        first = l[0]
+        second = l[-1]
+
+    if first is None:
         raise util.Abort(_('empty revision range'))
 
-    if len(l) == 1 and len(revs) == 1 and _revrangesep not in revs[0]:
-        return repo.lookup(l[0]), None
+    if first==second and len(revs) == 1 and _revrangesep not in revs[0]:
+        return repo.lookup(first), None
 
-    return repo.lookup(l[0]), repo.lookup(l[-1])
+    return repo.lookup(first), repo.lookup(second)
 
 _revrangesep = ':'
 
 def revrange(repo, revs):
     """Yield revision as strings from a list of revision specifications."""
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -742,10 +742,47 @@  multiple revspecs
   4
   5
   6
   7
 
+test usage in revpair (with "+")
+
+(real pair)
+
+  $ hg diff -r 'tip^^' -r 'tip'
+  diff -r 2326846efdab -r 24286f4ae135 .hgtags
+  --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+  +++ b/.hgtags	Thu Jan 01 00:00:00 1970 +0000
+  @@ -0,0 +1,1 @@
+  +e0cc66ef77e8b6f711815af4e001a6594fde3ba5 1.0
+  $ hg diff -r 'tip^^::tip'
+  diff -r 2326846efdab -r 24286f4ae135 .hgtags
+  --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+  +++ b/.hgtags	Thu Jan 01 00:00:00 1970 +0000
+  @@ -0,0 +1,1 @@
+  +e0cc66ef77e8b6f711815af4e001a6594fde3ba5 1.0
+
+(single rev)
+
+  $ hg diff -r 'tip^' -r 'tip^'
+  $ hg diff -r 'tip^::tip^ or tip^'
+
+(single rev that does not looks like a range)
+
+  $ hg diff -r 'tip^ or tip^'
+  diff -r d5d0dcbdc4d9 .hgtags
+  --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+  +++ b/.hgtags	* (glob)
+  @@ -0,0 +1,1 @@
+  +e0cc66ef77e8b6f711815af4e001a6594fde3ba5 1.0
+
+(no rev)
+
+  $ hg diff -r 'author("babar") or author("celeste")'
+  abort: empty revision range
+  [255]
+
 aliases:
 
   $ echo '[revsetalias]' >> .hg/hgrc
   $ echo 'm = merge()' >> .hg/hgrc
   $ echo 'sincem = descendants(m)' >> .hg/hgrc