Patchwork [STABLE,V3] revset: mask specific names for named() predicate

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Feb. 5, 2015, 5:49 a.m.
Message ID <9132a1cbf802d3abbbcd.1423115369@juju>
Download mbox | patch
Permalink /patch/7680/
State Accepted
Headers show

Comments

Katsunori FUJIWARA - Feb. 5, 2015, 5:49 a.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1423115149 -32400
#      Thu Feb 05 14:45:49 2015 +0900
# Branch stable
# Node ID 9132a1cbf802d3abbbcddf0f9ee675c3bc67e108
# Parent  942a5a34b2d0611ab284380fbe45b9bb1897af98
revset: mask specific names for named() predicate

Before this patch, revset predicate "tag()" and "named('tags')" differ
from each other, because the former doesn't include "tip" but the
latter does.

For equivalence, "named('tags')" shouldn't include the revision
corresponded to "tip". But just removing "tip" from the "tags"
namespace causes breaking backward compatibility, even though "tip"
itself is planned to be eliminated, as mentioned below.

    http://selenic.com/pipermail/mercurial-devel/2015-February/066157.html

To mask specific names ("tip" in this case) for "named()" predicate,
this patch introduces "deprecated" into "namespaces", and makes
"named()" predicate examine whether each names are masked by the
namespace, to which they belong.

"named()" will really work correctly after 3.3.1 (see 873eb5db89c8 for
detail), and fixing this on STABLE before 3.3.1 can prevent initial
users of "named()" from expecting "named('tags')" to include "tip".

It is reason why this patch is posted for STABLE, even though problem
itself isn't so serious.

This may have to be flagged as "(BC)", if applied on DEFAULT.
Sean Farley - Feb. 5, 2015, 6:26 a.m.
FUJIWARA Katsunori writes:

> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1423115149 -32400
> #      Thu Feb 05 14:45:49 2015 +0900
> # Branch stable
> # Node ID 9132a1cbf802d3abbbcddf0f9ee675c3bc67e108
> # Parent  942a5a34b2d0611ab284380fbe45b9bb1897af98
> revset: mask specific names for named() predicate
>
> Before this patch, revset predicate "tag()" and "named('tags')" differ
> from each other, because the former doesn't include "tip" but the
> latter does.
>
> For equivalence, "named('tags')" shouldn't include the revision
> corresponded to "tip". But just removing "tip" from the "tags"
> namespace causes breaking backward compatibility, even though "tip"
> itself is planned to be eliminated, as mentioned below.
>
>     http://selenic.com/pipermail/mercurial-devel/2015-February/066157.html
>
> To mask specific names ("tip" in this case) for "named()" predicate,
> this patch introduces "deprecated" into "namespaces", and makes
> "named()" predicate examine whether each names are masked by the
> namespace, to which they belong.
>
> "named()" will really work correctly after 3.3.1 (see 873eb5db89c8 for
> detail), and fixing this on STABLE before 3.3.1 can prevent initial
> users of "named()" from expecting "named('tags')" to include "tip".
>
> It is reason why this patch is posted for STABLE, even though problem
> itself isn't so serious.
>
> This may have to be flagged as "(BC)", if applied on DEFAULT.

Ok, let me try to pause things here. I think we all agree that the
approach to remove 'tip' from namespaces is the way forward. My question
is on implementation. Part of me likes having the 'deprecated' or
'revsetmask' that you propose.

I don't know what the future holds for the namespaces API but it does
some this is a bit of a big step. My suggestion (looking for feedback)
would be: could we split tags into their own namespaces? e.g.

namespaces['localtags']
namespaces['tags']

and then filter 'tip' from those specially?
Katsunori FUJIWARA - Feb. 7, 2015, 11:50 a.m.
At Wed, 04 Feb 2015 22:26:58 -0800,
Sean Farley wrote:
> 
> 
> FUJIWARA Katsunori writes:
> 
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1423115149 -32400
> > #      Thu Feb 05 14:45:49 2015 +0900
> > # Branch stable
> > # Node ID 9132a1cbf802d3abbbcddf0f9ee675c3bc67e108
> > # Parent  942a5a34b2d0611ab284380fbe45b9bb1897af98
> > revset: mask specific names for named() predicate
> >
> > Before this patch, revset predicate "tag()" and "named('tags')" differ
> > from each other, because the former doesn't include "tip" but the
> > latter does.
> >
> > For equivalence, "named('tags')" shouldn't include the revision
> > corresponded to "tip". But just removing "tip" from the "tags"
> > namespace causes breaking backward compatibility, even though "tip"
> > itself is planned to be eliminated, as mentioned below.
> >
> >     http://selenic.com/pipermail/mercurial-devel/2015-February/066157.html
> >
> > To mask specific names ("tip" in this case) for "named()" predicate,
> > this patch introduces "deprecated" into "namespaces", and makes
> > "named()" predicate examine whether each names are masked by the
> > namespace, to which they belong.
> >
> > "named()" will really work correctly after 3.3.1 (see 873eb5db89c8 for
> > detail), and fixing this on STABLE before 3.3.1 can prevent initial
> > users of "named()" from expecting "named('tags')" to include "tip".
> >
> > It is reason why this patch is posted for STABLE, even though problem
> > itself isn't so serious.
> >
> > This may have to be flagged as "(BC)", if applied on DEFAULT.
> 
> Ok, let me try to pause things here. I think we all agree that the
> approach to remove 'tip' from namespaces is the way forward. My question
> is on implementation. Part of me likes having the 'deprecated' or
> 'revsetmask' that you propose.
> 
> I don't know what the future holds for the namespaces API but it does
> some this is a bit of a big step. My suggestion (looking for feedback)
> would be: could we split tags into their own namespaces? e.g.
> 
> namespaces['localtags']
> namespaces['tags']
> 
> and then filter 'tip' from those specially?
> 

Which would you mean that "localtags" are just ones stored in
".hg/localtags", or ones NOT stored in ".hgtags" (including "tip" and
ones from MQ patches) ?

If the latter, what about separation below ?

  - "publictags":  ones stored in ".hgtags"
  - "privatetags": others ("tip", ones from ".hg/localtags" and so on)
  - "tags":        combination of them

And I roughly made a tentative plan to move "tip" tag away:

  1. remove "tip" from result of "named('tags')" revset for equivalence

     by this patch :-)

  2. introduce "publictags" and "privatetags" namespaces

     "tip" is still visible via "tags" and "privatetags" (but removed
     from result of "named('tags')" and "named('privatetags')").

  3. replace "repo.tagslist()" for referring tags by "repo.names['tags']"

     Some code paths may use "repo.names['publictags']" instead of
     explicit filtering private tag.

  4. remove "tip" from "repo.tagslist" and "privatetags" namespace

     "tip" is also removed from "tags" namespace, log-like output, web
     UI and "hg tags" output.

     Now, "tip" is recognized as just one of reserved symbols like as
     "null" and ".".

     "namespace.deprecated" is also removed at this point.

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Sean Farley - Feb. 12, 2015, 8:34 p.m.
FUJIWARA Katsunori writes:

> At Wed, 04 Feb 2015 22:26:58 -0800,
> Sean Farley wrote:
>> 
>> 
>> FUJIWARA Katsunori writes:
>> 
>> > # HG changeset patch
>> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
>> > # Date 1423115149 -32400
>> > #      Thu Feb 05 14:45:49 2015 +0900
>> > # Branch stable
>> > # Node ID 9132a1cbf802d3abbbcddf0f9ee675c3bc67e108
>> > # Parent  942a5a34b2d0611ab284380fbe45b9bb1897af98
>> > revset: mask specific names for named() predicate
>> >
>> > Before this patch, revset predicate "tag()" and "named('tags')" differ
>> > from each other, because the former doesn't include "tip" but the
>> > latter does.
>> >
>> > For equivalence, "named('tags')" shouldn't include the revision
>> > corresponded to "tip". But just removing "tip" from the "tags"
>> > namespace causes breaking backward compatibility, even though "tip"
>> > itself is planned to be eliminated, as mentioned below.
>> >
>> >     http://selenic.com/pipermail/mercurial-devel/2015-February/066157.html
>> >
>> > To mask specific names ("tip" in this case) for "named()" predicate,
>> > this patch introduces "deprecated" into "namespaces", and makes
>> > "named()" predicate examine whether each names are masked by the
>> > namespace, to which they belong.
>> >
>> > "named()" will really work correctly after 3.3.1 (see 873eb5db89c8 for
>> > detail), and fixing this on STABLE before 3.3.1 can prevent initial
>> > users of "named()" from expecting "named('tags')" to include "tip".
>> >
>> > It is reason why this patch is posted for STABLE, even though problem
>> > itself isn't so serious.
>> >
>> > This may have to be flagged as "(BC)", if applied on DEFAULT.
>> 
>> Ok, let me try to pause things here. I think we all agree that the
>> approach to remove 'tip' from namespaces is the way forward. My question
>> is on implementation. Part of me likes having the 'deprecated' or
>> 'revsetmask' that you propose.
>> 
>> I don't know what the future holds for the namespaces API but it does
>> some this is a bit of a big step. My suggestion (looking for feedback)
>> would be: could we split tags into their own namespaces? e.g.
>> 
>> namespaces['localtags']
>> namespaces['tags']
>> 
>> and then filter 'tip' from those specially?
>> 
>
> Which would you mean that "localtags" are just ones stored in
> ".hg/localtags", or ones NOT stored in ".hgtags" (including "tip" and
> ones from MQ patches) ?
>
> If the latter, what about separation below ?
>
>   - "publictags":  ones stored in ".hgtags"
>   - "privatetags": others ("tip", ones from ".hg/localtags" and so on)
>   - "tags":        combination of them
>
> And I roughly made a tentative plan to move "tip" tag away:
>
>   1. remove "tip" from result of "named('tags')" revset for equivalence
>
>      by this patch :-)
>
>   2. introduce "publictags" and "privatetags" namespaces
>
>      "tip" is still visible via "tags" and "privatetags" (but removed
>      from result of "named('tags')" and "named('privatetags')").
>
>   3. replace "repo.tagslist()" for referring tags by "repo.names['tags']"
>
>      Some code paths may use "repo.names['publictags']" instead of
>      explicit filtering private tag.
>
>   4. remove "tip" from "repo.tagslist" and "privatetags" namespace
>
>      "tip" is also removed from "tags" namespace, log-like output, web
>      UI and "hg tags" output.
>
>      Now, "tip" is recognized as just one of reserved symbols like as
>      "null" and ".".
>
>      "namespace.deprecated" is also removed at this point.

Just to keep this going, the current status is that I like your
suggestions but need time with mpm to hash this out. Let's bring this up
in two weeks or so.
Matt Mackall - Feb. 28, 2015, 7:19 p.m.
On Thu, 2015-02-12 at 12:34 -0800, Sean Farley wrote:
> FUJIWARA Katsunori writes:
> 
> > At Wed, 04 Feb 2015 22:26:58 -0800,
> > Sean Farley wrote:
> >> 
> >> 
> >> FUJIWARA Katsunori writes:
> >> 
> >> > # HG changeset patch
> >> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> >> > # Date 1423115149 -32400
> >> > #      Thu Feb 05 14:45:49 2015 +0900
> >> > # Branch stable
> >> > # Node ID 9132a1cbf802d3abbbcddf0f9ee675c3bc67e108
> >> > # Parent  942a5a34b2d0611ab284380fbe45b9bb1897af98
> >> > revset: mask specific names for named() predicate
> >> >
> >> > Before this patch, revset predicate "tag()" and "named('tags')" differ
> >> > from each other, because the former doesn't include "tip" but the
> >> > latter does.
> >> >
> >> > For equivalence, "named('tags')" shouldn't include the revision
> >> > corresponded to "tip". But just removing "tip" from the "tags"
> >> > namespace causes breaking backward compatibility, even though "tip"
> >> > itself is planned to be eliminated, as mentioned below.
> >> >
> >> >     http://selenic.com/pipermail/mercurial-devel/2015-February/066157.html
> >> >
> >> > To mask specific names ("tip" in this case) for "named()" predicate,
> >> > this patch introduces "deprecated" into "namespaces", and makes
> >> > "named()" predicate examine whether each names are masked by the
> >> > namespace, to which they belong.
> >> >
> >> > "named()" will really work correctly after 3.3.1 (see 873eb5db89c8 for
> >> > detail), and fixing this on STABLE before 3.3.1 can prevent initial
> >> > users of "named()" from expecting "named('tags')" to include "tip".
> >> >
> >> > It is reason why this patch is posted for STABLE, even though problem
> >> > itself isn't so serious.
> >> >
> >> > This may have to be flagged as "(BC)", if applied on DEFAULT.
> >> 
> >> Ok, let me try to pause things here. I think we all agree that the
> >> approach to remove 'tip' from namespaces is the way forward. My question
> >> is on implementation. Part of me likes having the 'deprecated' or
> >> 'revsetmask' that you propose.
> >> 
> >> I don't know what the future holds for the namespaces API but it does
> >> some this is a bit of a big step. My suggestion (looking for feedback)
> >> would be: could we split tags into their own namespaces? e.g.
> >> 
> >> namespaces['localtags']
> >> namespaces['tags']
> >> 
> >> and then filter 'tip' from those specially?
> >> 
> >
> > Which would you mean that "localtags" are just ones stored in
> > ".hg/localtags", or ones NOT stored in ".hgtags" (including "tip" and
> > ones from MQ patches) ?
> >
> > If the latter, what about separation below ?
> >
> >   - "publictags":  ones stored in ".hgtags"
> >   - "privatetags": others ("tip", ones from ".hg/localtags" and so on)
> >   - "tags":        combination of them
> >
> > And I roughly made a tentative plan to move "tip" tag away:
> >
> >   1. remove "tip" from result of "named('tags')" revset for equivalence
> >
> >      by this patch :-)
> >
> >   2. introduce "publictags" and "privatetags" namespaces
> >
> >      "tip" is still visible via "tags" and "privatetags" (but removed
> >      from result of "named('tags')" and "named('privatetags')").
> >
> >   3. replace "repo.tagslist()" for referring tags by "repo.names['tags']"
> >
> >      Some code paths may use "repo.names['publictags']" instead of
> >      explicit filtering private tag.
> >
> >   4. remove "tip" from "repo.tagslist" and "privatetags" namespace
> >
> >      "tip" is also removed from "tags" namespace, log-like output, web
> >      UI and "hg tags" output.
> >
> >      Now, "tip" is recognized as just one of reserved symbols like as
> >      "null" and ".".
> >
> >      "namespace.deprecated" is also removed at this point.
> 
> Just to keep this going, the current status is that I like your
> suggestions but need time with mpm to hash this out. Let's bring this up
> in two weeks or so.

Ok, I'm just going to take this as is even though I'm not happy with the
state of the discussion.

Two notes here. First, this is the _stable_ branch. Thus, it's not a
good place to put patches that trigger design discussions. Just do the
simplest thing that fixes the issue. A one-line hack in revset.py is
perfectly acceptable, especially since it greatly narrows the scope for
accidentally introducing other new bugs by accident. We can do it
"right" in default.

Second, this is a time-sensitive issue. The next release is tomorrow and
this needs to be fixed before it gets frozen in the API. Which again
means getting it done is more important than getting it done prettily.
Since I've got a patch in front of me that only has prettiness
objections, I'm gonna take it and we can fix it in default after the
release.

Patch

diff --git a/mercurial/namespaces.py b/mercurial/namespaces.py
--- a/mercurial/namespaces.py
+++ b/mercurial/namespaces.py
@@ -41,7 +41,8 @@  class namespaces(object):
                       # i18n: column positioning for "hg log"
                       logfmt=_("tag:         %s\n"),
                       listnames=tagnames,
-                      namemap=tagnamemap, nodemap=tagnodemap)
+                      namemap=tagnamemap, nodemap=tagnodemap,
+                      deprecated=set(['tip']))
         self.addnamespace(n)
 
         bnames = lambda repo: repo.branchmap().keys()
@@ -126,11 +127,13 @@  class namespace(object):
                    dictionary)
       'namemap': function that takes a name and returns a list of nodes
       'nodemap': function that takes a node and returns a list of names
+      'deprecated': set of names to be masked for ordinary use
 
     """
 
     def __init__(self, name, templatename=None, logname=None, colorname=None,
-                 logfmt=None, listnames=None, namemap=None, nodemap=None):
+                 logfmt=None, listnames=None, namemap=None, nodemap=None,
+                 deprecated=None):
         """create a namespace
 
         name: the namespace to be registered (in plural form)
@@ -144,6 +147,7 @@  class namespace(object):
         listnames: function to list all names
         namemap: function that inputs a node, output name(s)
         nodemap: function that inputs a name, output node(s)
+        deprecated: set of names to be masked for ordinary use
 
         """
         self.name = name
@@ -168,6 +172,11 @@  class namespace(object):
             # i18n: column positioning for "hg log"
             self.logfmt = ("%s:" % self.logname).ljust(13) + "%s\n"
 
+        if deprecated is None:
+            self.deprecated = set()
+        else:
+            self.deprecated = deprecated
+
     def names(self, repo, node):
         """method that returns a (sorted) list of names in a namespace that
         match a given node"""
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1277,7 +1277,8 @@  def named(repo, subset, x):
     names = set()
     for ns in namespaces:
         for name in ns.listnames(repo):
-            names.update(repo[n].rev() for n in ns.nodes(repo, name))
+            if name not in ns.deprecated:
+                names.update(repo[n].rev() for n in ns.nodes(repo, name))
 
     names -= set([node.nullrev])
     return subset & names
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -791,7 +791,6 @@  we can use patterns when searching for t
   6
   $ log 'named("tags")'
   6
-  9
 
 issue2437