Patchwork [1,of,2,STABLE,V2] revset: drop magic of fullreposet membership test (issue4682)

login
register
mail settings
Submitter Yuya Nishihara
Date May 24, 2015, 4:48 a.m.
Message ID <c1758faf54617855d3f2.1432442892@mimosa>
Download mbox | patch
Permalink /patch/9252/
State Accepted
Headers show

Comments

Yuya Nishihara - May 24, 2015, 4:48 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1432430973 -32400
#      Sun May 24 10:29:33 2015 +0900
# Branch stable
# Node ID c1758faf54617855d3f21152cd05e81fecf6cf6b
# Parent  fd905b2bea5967c71e71d25f29b2c7a04cafcadc
revset: drop magic of fullreposet membership test (issue4682)

This patch partially backs out d2de20e1451f and adds an alternative workaround
to functions that evaluate "null" and "wdir()". Because the new workaround is
incomplete, "first(null)" and "min(null)" don't work as expected. But they were
not usable until 3.4 and "null" isn't commonly used, we can postpone a complete
fix for 3.5.

The issue4682 was caused because "branch(default)" is evaluated to
"<filteredset <fullreposet>>", keeping fullreposet magic. The next patch will
fix crash on "branch(null)", but without this patch, it would make
"null in <branch(default)>" be True, which means "children(branch(default))"
would return all revisions but merge (p2 != null).

I believe the right fix is to stop propagating fullreposet magic on filter(),
but it wouldn't fit to stable release. Also, we should discuss how to handle
"null" and "wdir()" in revset before.
Yuya Nishihara - May 24, 2015, 10:24 a.m.
On Sun, 24 May 2015 13:48:12 +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1432430973 -32400
> #      Sun May 24 10:29:33 2015 +0900
> # Branch stable
> # Node ID c1758faf54617855d3f21152cd05e81fecf6cf6b
> # Parent  fd905b2bea5967c71e71d25f29b2c7a04cafcadc
> revset: drop magic of fullreposet membership test (issue4682)
> 
> This patch partially backs out d2de20e1451f and adds an alternative workaround
> to functions that evaluate "null" and "wdir()". Because the new workaround is
> incomplete, "first(null)" and "min(null)" don't work as expected. But they were
> not usable until 3.4 and "null" isn't commonly used, we can postpone a complete
> fix for 3.5.


> @@ -630,9 +630,9 @@ Test null revision
>  Test working-directory revision
>    $ hg debugrevspec 'wdir()'
>    None
> +BROKEN: should include 'None'
>    $ hg debugrevspec 'tip or wdir()'
>    9
> -  None

This part won't be broken in default branch because of 40a2cf1c765b, 'revset:
drop redundant filteredset from right-hand side set of "or" operation'.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -330,7 +330,8 @@  def _getrevsource(repo, r):
 
 def stringset(repo, subset, x):
     x = repo[x].rev()
-    if x in subset:
+    if (x in subset
+        or x == node.nullrev and isinstance(subset, fullreposet)):
         return baseset([x])
     return baseset()
 
@@ -1905,7 +1906,7 @@  def user(repo, subset, x):
 def wdir(repo, subset, x):
     # i18n: "wdir" is a keyword
     getargs(x, 0, 0, _("wdir takes no arguments"))
-    if None in subset:
+    if None in subset or isinstance(subset, fullreposet):
         return baseset([None])
     return baseset()
 
@@ -3408,11 +3409,6 @@  class fullreposet(spanset):
     def __init__(self, repo):
         super(fullreposet, self).__init__(repo)
 
-    def __contains__(self, rev):
-        # assumes the given rev is valid
-        hidden = self._hiddenrevs
-        return not (hidden and rev in hidden)
-
     def __and__(self, other):
         """As self contains the whole repo, all of the other set should also be
         in self. Therefore `self & other = other`.
diff --git a/tests/test-glog.t b/tests/test-glog.t
--- a/tests/test-glog.t
+++ b/tests/test-glog.t
@@ -2375,4 +2375,12 @@  should not draw line down to null due to
      summary:     add a
   
 
+  $ hg log -G -r 'branch(default)' | tail -6
+  |
+  o  changeset:   0:f8035bb17114
+     user:        test
+     date:        Thu Jan 01 00:00:00 1970 +0000
+     summary:     add a
+  
+
   $ cd ..
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -619,10 +619,10 @@  Test null revision
   $ log 'reverse(null:)' | tail -2
   0
   -1
+BROKEN: should be '-1'
   $ log 'first(null:)'
-  -1
+BROKEN: should be '-1'
   $ log 'min(null:)'
-  -1
   $ log 'tip:null and all()' | tail -2
   1
   0
@@ -630,9 +630,9 @@  Test null revision
 Test working-directory revision
   $ hg debugrevspec 'wdir()'
   None
+BROKEN: should include 'None'
   $ hg debugrevspec 'tip or wdir()'
   9
-  None
   $ hg debugrevspec '0:tip and wdir()'
 
   $ log 'outgoing()'
@@ -1580,6 +1580,38 @@  tests for concatenation of strings/symbo
 
   $ cd ..
 
+prepare repository that has "default" branches of multiple roots
+
+  $ hg init namedbranch
+  $ cd namedbranch
+
+  $ echo default0 >> a
+  $ hg ci -Aqm0
+  $ echo default1 >> a
+  $ hg ci -m1
+
+  $ hg branch -q stable
+  $ echo stable2 >> a
+  $ hg ci -m2
+  $ echo stable3 >> a
+  $ hg ci -m3
+
+  $ hg update -q null
+  $ echo default4 >> a
+  $ hg ci -Aqm4
+  $ echo default5 >> a
+  $ hg ci -m5
+
+"null" revision belongs to "default" branch, but it shouldn't appear in set
+unless explicitly specified (issue4682)
+
+  $ log 'children(branch(default))'
+  1
+  2
+  5
+
+  $ cd ..
+
 test author/desc/keyword in problematic encoding
 # unicode: cp932:
 # u30A2    0x83 0x41(= 'A')