Patchwork [4,of,4] revset: drop TODO comment about sorting issue of fullreposet

login
register
mail settings
Submitter Yuya Nishihara
Date Jan. 5, 2017, 2:48 p.m.
Message ID <65c2380fdab26efeee43.1483627728@mimosa>
Download mbox | patch
Permalink /patch/18105/
State Accepted
Headers show

Comments

Yuya Nishihara - Jan. 5, 2017, 2:48 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1463226764 -32400
#      Sat May 14 20:52:44 2016 +0900
# Node ID 65c2380fdab26efeee4356170bbad53504bcd614
# Parent  1ae32abc1f69e33392d7f929b8c7130efc915e78
revset: drop TODO comment about sorting issue of fullreposet

The bootstrapping issue was addressed at the parsing phase and we expect
that fullreposet.__and__() fully complies to the smartset API, in which
'self & other' should return a result set in self's order. See also
90455e7bf543.
via Mercurial-devel - Jan. 5, 2017, 5:11 p.m.
On Thu, Jan 5, 2017 at 6:48 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1463226764 -32400
> #      Sat May 14 20:52:44 2016 +0900
> # Node ID 65c2380fdab26efeee4356170bbad53504bcd614
> # Parent  1ae32abc1f69e33392d7f929b8c7130efc915e78
> revset: drop TODO comment about sorting issue of fullreposet

Queue this series. Thanks for fixing!

>
> The bootstrapping issue was addressed at the parsing phase and we expect
> that fullreposet.__and__() fully complies to the smartset API, in which
> 'self & other' should return a result set in self's order. See also
> 90455e7bf543.
>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -3814,17 +3814,6 @@ class fullreposet(spanset):
>              # object.
>              other = baseset(other - self._hiddenrevs)
>
> -        # XXX As fullreposet is also used as bootstrap, this is wrong.
> -        #
> -        # With a giveme312() revset returning [3,1,2], this makes
> -        #   'hg log -r "giveme312()"' -> 1, 2, 3 (wrong)
> -        # We cannot just drop it because other usage still need to sort it:
> -        #   'hg log -r "all() and giveme312()"' -> 1, 2, 3 (right)
> -        #
> -        # There is also some faulty revset implementations that rely on it
> -        # (eg: children as of its state in e8075329c5fb)
> -        #
> -        # When we fix the two points above we can move this into the if clause
>          other.sort(reverse=self.isdescending())
>          return other
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -3814,17 +3814,6 @@  class fullreposet(spanset):
             # object.
             other = baseset(other - self._hiddenrevs)
 
-        # XXX As fullreposet is also used as bootstrap, this is wrong.
-        #
-        # With a giveme312() revset returning [3,1,2], this makes
-        #   'hg log -r "giveme312()"' -> 1, 2, 3 (wrong)
-        # We cannot just drop it because other usage still need to sort it:
-        #   'hg log -r "all() and giveme312()"' -> 1, 2, 3 (right)
-        #
-        # There is also some faulty revset implementations that rely on it
-        # (eg: children as of its state in e8075329c5fb)
-        #
-        # When we fix the two points above we can move this into the if clause
         other.sort(reverse=self.isdescending())
         return other