Patchwork [STABLE] revset: raise RepoLookupError to make present() predicate continue the query

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Jan. 30, 2015, 4:07 p.m.
Message ID <0b8a5ef9a8f3106749c0.1422634033@juju>
Download mbox | patch
Permalink /patch/7578/
State Accepted
Commit eeb5d5ab14a661ad445ac6cb868901f3faa11a70
Headers show

Comments

Katsunori FUJIWARA - Jan. 30, 2015, 4:07 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1422633650 -32400
#      Sat Jan 31 01:00:50 2015 +0900
# Branch stable
# Node ID 0b8a5ef9a8f3106749c0fe3b6300e2358822d376
# Parent  6becb9dbca25057c6186e255a48dd2c2ce5701a5
revset: raise RepoLookupError to make present() predicate continue the query

Before this patch, "bookmark()", "named()" and "tag()" predicates
raise "Abort", when the specified pattern doesn't match against
existing ones.

This prevents "present()" predicate from continuing the query, because
it only catches "RepoLookupError".

This patch raises "RepoLookupError" instead of "Abort", to make
"present()" predicate continue the query, even if "bookmark()",
"named()" or "tag()" in the sub-query of it are aborted.

This patch doesn't contain raising "RepoLookupError" for "re:" pattern
in "tag()", because "tag()" treats it differently from others. Actions
of each predicates at failure of pattern matching can be summarized as
below:

  predicate  "literal:"  "re:"
  ---------- ----------- ------------
  bookmark   abort       abort
  named      abort       abort
  tag        abort       continue (*1)

  branch     abort       continue (*2)
  ---------- ----------- ------------

"tag()" may have to abort in the (*1) case for similarity, but this
change may break backward compatibility of existing revset queries. It
seems to have to be changed on "default" branch (with "BC" ?).

On the other hand, (*2) seems to be reasonable, even though it breaks
similarity, because "branch()" in this case doesn't check exact
existence of branches, but does pick up revisions of which branch
matches against the pattern.

This patch also adds tests for "branch()" to clarify behavior around
"present()" of similar predicates, even though this patch doesn't
change "branch()".
Matt Mackall - Feb. 1, 2015, 8:22 p.m.
On Sat, 2015-01-31 at 01:07 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1422633650 -32400
> #      Sat Jan 31 01:00:50 2015 +0900
> # Branch stable
> # Node ID 0b8a5ef9a8f3106749c0fe3b6300e2358822d376
> # Parent  6becb9dbca25057c6186e255a48dd2c2ce5701a5
> revset: raise RepoLookupError to make present() predicate continue the query

Queued for stable, thanks.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -496,7 +496,8 @@  def bookmark(repo, subset, x):
         if kind == 'literal':
             bmrev = repo._bookmarks.get(pattern, None)
             if not bmrev:
-                raise util.Abort(_("bookmark '%s' does not exist") % bm)
+                raise error.RepoLookupError(_("bookmark '%s' does not exist")
+                                            % bm)
             bms.add(repo[bmrev].rev())
         else:
             matchrevs = set()
@@ -504,8 +505,8 @@  def bookmark(repo, subset, x):
                 if matcher(name):
                     matchrevs.add(bmrev)
             if not matchrevs:
-                raise util.Abort(_("no bookmarks exist that match '%s'")
-                                 % pattern)
+                raise error.RepoLookupError(_("no bookmarks exist"
+                                              " that match '%s'") % pattern)
             for bmrev in matchrevs:
                 bms.add(repo[bmrev].rev())
     else:
@@ -1262,15 +1263,16 @@  def named(repo, subset, x):
     namespaces = set()
     if kind == 'literal':
         if pattern not in repo.names:
-            raise util.Abort(_("namespace '%s' does not exist") % ns)
+            raise error.RepoLookupError(_("namespace '%s' does not exist")
+                                        % ns)
         namespaces.add(repo.names[pattern])
     else:
         for name, ns in repo.names.iteritems():
             if matcher(name):
                 namespaces.add(ns)
         if not namespaces:
-            raise util.Abort(_("no namespace exists that match '%s'")
-                             % pattern)
+            raise error.RepoLookupError(_("no namespace exists"
+                                          " that match '%s'") % pattern)
 
     names = set()
     for ns in namespaces:
@@ -1816,7 +1818,8 @@  def tag(repo, subset, x):
             # avoid resolving all tags
             tn = repo._tagscache.tags.get(pattern, None)
             if tn is None:
-                raise util.Abort(_("tag '%s' does not exist") % pattern)
+                raise error.RepoLookupError(_("tag '%s' does not exist")
+                                            % pattern)
             s = set([repo[tn].rev()])
         else:
             s = set([cl.rev(n) for t, n in repo.tagslist() if matcher(t)])
diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
--- a/tests/test-bookmarks.t
+++ b/tests/test-bookmarks.t
@@ -133,8 +133,13 @@  bookmarks revset
   
 
   $ hg log -r 'bookmark(unknown)'
-  abort: bookmark 'unknown' does not exist
-  [255]
+  abort: bookmark 'unknown' does not exist!
+  [255]
+  $ hg log -r 'bookmark("re:unknown")'
+  abort: no bookmarks exist that match 'unknown'!
+  [255]
+  $ hg log -r 'present(bookmark("literal:unknown"))'
+  $ hg log -r 'present(bookmark("re:unknown"))'
 
   $ hg help revsets | grep 'bookmark('
       "bookmark([name])"
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -709,7 +709,7 @@  check that conversion to only works
 we can use patterns when searching for tags
 
   $ log 'tag("1..*")'
-  abort: tag '1..*' does not exist
+  abort: tag '1..*' does not exist!
   [255]
   $ log 'tag("re:1..*")'
   6
@@ -720,11 +720,17 @@  we can use patterns when searching for t
   $ log 'tag("re:0..*")'
 
   $ log 'tag(unknown)'
-  abort: tag 'unknown' does not exist
-  [255]
+  abort: tag 'unknown' does not exist!
+  [255]
+  $ log 'tag("re:unknown")'
+  $ log 'present(tag("unknown"))'
+  $ log 'present(tag("re:unknown"))'
   $ log 'branch(unknown)'
   abort: unknown revision 'unknown'!
   [255]
+  $ log 'branch("re:unknown")'
+  $ log 'present(branch("unknown"))'
+  $ log 'present(branch("re:unknown"))'
   $ log 'user(bob)'
   2
 
@@ -772,6 +778,15 @@  we can use patterns when searching for t
   3
   1
 
+  $ log 'named("unknown")'
+  abort: namespace 'unknown' does not exist!
+  [255]
+  $ log 'named("re:unknown")'
+  abort: no namespace exists that match 'unknown'!
+  [255]
+  $ log 'present(named("unknown"))'
+  $ log 'present(named("re:unknown"))'
+
 issue2437
 
   $ log '3 and p1(5)'