Patchwork D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)

login
register
mail settings
Submitter phabricator
Date June 11, 2018, 11:11 p.m.
Message ID <differential-rev-PHID-DREV-ogw6r362ybbdlpep5q4l-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/32068/
State New
Headers show

Comments

phabricator - June 11, 2018, 11:11 p.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The goal of this commit is to allow namespaces that support multiple
  nodes per name to also have the name resolve to those multiple nodes
  in the revset. For example, the "topics" namespace seems to be a
  natural candidate for this (I don't know if the extension's maintainer
  agrees). I view a topic as having multiple nodes and I would therefore
  expect `hg log -r my-topic` to list all those nodes. Note that the
  topics extension already supports multiple nodes per name, but we
  don't expose that to the user in a consistent way (each namespace has
  to define its own revset).
  
  This commit adds an option to namespaces to indicate that `hg log -r
  <name in the namespace>` and similar should resolve to the nodes that
  the namespace says and not just the highest revnum among them.
  
  Marked (API) because I repurposed singlenode() to nodes().
  
  Note: I think branches should also resolve to multiple nodes (so
  e.g. `hg log -r stable` lists all nodes on stable), but it's obviously
  too late to change that now (and perhaps BC is the reason it even
  behaves the way it does). I also realize that any namespace that were
  to use the new mechanism would be inconsistent with how branches
  work. I think the convenience and intuitiveness outweighs the cost of
  that inconsistency.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3715

AFFECTED FILES
  mercurial/namespaces.py
  mercurial/revset.py
  mercurial/scmutil.py
  tests/paritynamesext.py
  tests/test-revset2.t

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - June 12, 2018, 5:41 p.m.
durin42 added subscribers: lothiraldan, smf, durin42.
durin42 accepted this revision as: durin42.
durin42 added a comment.


  I'm in favor, but feel like I've got enough conflict of interest I shouldn't land the patches.
  
  @smf @lothiraldan this might be of interest to both of you?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3715

To: martinvonz, #hg-reviewers, durin42
Cc: durin42, smf, lothiraldan, mercurial-devel
Sean Farley - June 13, 2018, 12:40 a.m.
durin42 (Augie Fackler) <phabricator@mercurial-scm.org> writes:

> durin42 added subscribers: lothiraldan, smf, durin42.
> durin42 accepted this revision as: durin42.
> durin42 added a comment.
>
>
>   I'm in favor, but feel like I've got enough conflict of interest I shouldn't land the patches.
>
>   @smf @lothiraldan this might be of interest to both of you?

Side note: I keep missing messages that I'm tagged in because I'm not
explicitly mentioned in a CC field. Is it possible to add a CC to each
person tagged in a message?

Side note2: Phabricator emails are really non-trivial to parse and
(worse!) search. The raw emails are not simple, raw text so I'm having
trouble tagging these for higher priority.

Thanks for alerting me of this series! I've had a discussion with Martin
about this on IRC but I'm a bit out of time today to respond (but
definitely do want to respond). (I'm going to try to spend some time in
the mornings to do my email triaging so I can get back on top of this
list.)
phabricator - June 13, 2018, 3:59 p.m.
lothiraldan added a comment.


  In https://phab.mercurial-scm.org/D3715#58383, @durin42 wrote:
  
  > I'm in favor, but feel like I've got enough conflict of interest I shouldn't land the patches.
  >
  > @smf @lothiraldan this might be of interest to both of you?
  
  
  The proposition of making namespaces behave differently than branches, and potentially making two namespaces behave differently (one with `multinode=True` and the other with `multinode=False`) make my spider sense tickle. But I will take time to see the exact implications of that series.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3715

To: martinvonz, #hg-reviewers, durin42
Cc: durin42, smf, lothiraldan, mercurial-devel
phabricator - June 13, 2018, 4:37 p.m.
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D3715#58383, @durin42 wrote:
  
  > I'm in favor, but feel like I've got enough conflict of interest I shouldn't land the patches.
  >
  > @smf @lothiraldan this might be of interest to both of you?
  
  
  
  
  In https://phab.mercurial-scm.org/D3715#58507, @lothiraldan wrote:
  
  > In https://phab.mercurial-scm.org/D3715#58383, @durin42 wrote:
  >
  > > I'm in favor, but feel like I've got enough conflict of interest I shouldn't land the patches.
  > >
  > > @smf @lothiraldan this might be of interest to both of you?
  >
  >
  > The proposition of making namespaces behave differently than branches, and potentially making two namespaces behave differently (one with `multinode=True` and the other with `multinode=False`) make my spider sense tickle. But I will take time to see the exact implications of that series.
  
  
  As I said in the commit message, I think branches should have multinode=True, but we can't do that for BC reasons. Bookmarks and tags clearly identify a single commit per name (at least in my mind).
  
  How about we introduce the multinode option and default it to False for now, but plan to change the default to True later and then let only branches have the old behavior (i.e. multinode=False) and discourage extensions from setting multinode=False? That would result in consistency between namespaces (except for the legacy case of branches) while giving existing extensions some time to adapt (perhaps by making namemap return at most one node if they view their names as identifying a single commit).
  
  Btw, the topics extension was just an example. We have an internal extension that's the reason for this patch. I don't use topics, so I don't care too much if topics decide to have one commit per name or not (but if I did use topics, I think I would definitely view it as having multiple nodes per name).

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3715

To: martinvonz, #hg-reviewers, durin42
Cc: durin42, smf, lothiraldan, mercurial-devel
Sean Farley - June 15, 2018, 12:34 a.m.
Sean Farley <sean@farley.io> writes:

> durin42 (Augie Fackler) <phabricator@mercurial-scm.org> writes:
>
>> durin42 added subscribers: lothiraldan, smf, durin42.
>> durin42 accepted this revision as: durin42.
>> durin42 added a comment.
>>
>>
>>   I'm in favor, but feel like I've got enough conflict of interest I shouldn't land the patches.
>>
>>   @smf @lothiraldan this might be of interest to both of you?
>
> Side note: I keep missing messages that I'm tagged in because I'm not
> explicitly mentioned in a CC field. Is it possible to add a CC to each
> person tagged in a message?
>
> Side note2: Phabricator emails are really non-trivial to parse and
> (worse!) search. The raw emails are not simple, raw text so I'm having
> trouble tagging these for higher priority.

I think I got this ironed out now.

> Thanks for alerting me of this series! I've had a discussion with Martin
> about this on IRC but I'm a bit out of time today to respond (but
> definitely do want to respond). (I'm going to try to spend some time in
> the mornings to do my email triaging so I can get back on top of this
> list.)

This patch strikes me as a Seems-Like-A-Good-Idea-But-Could-Blowup type
of thing. So, what this patch does is conditionally change the behavior
of 'log -r' based on the type of object passed in. In terms of revsets,
this means:

hg log -r 'default' -> hg log -r 'branch(default)'

I'm using branches to illustrate the argument in terms of core behavior
but will go back to topics further down.

But what about more complex revsets? 'default & draft()' will now mean
'branch(default) & draft()' which is something completely different. You
could argue that, "Oh, well, we'll only do that if the argument sent to
-r is equal to a single branch name." But what about users that expect
'-r NAME' to work like 'branch(NAME)' in a revset? I would expect they
learn a bad habit of 'default == branch(default)'. And what about a more
complex that will reduce down to 'default'? Will that be
'branch(default)' now?

It's not just about 'hg log,' though. What about 'hg push -r default'?
Currently, this will push the highest rev of default but with default
being replaced in a user's mental model with 'branch(default)' this
should push each head.

For these reasons, I am fairly against having a conditional hack like
this on top of -r.

I'd like to point out that this is why -b exists. 'hg log -b default' is
the command you're looking for. "But what about topics?" you ask. Well,
personally, I think that extension should add -t to log if it wants that
functionality (-t is available to both 'push' and 'log' for those
curious).

P.S. (personal opinion mini-rant) This is why I don't really like the
concept of topics. In reality, they are conceptually sub-branches. What
I really want 99% of the time is a branch (we call them named branches)
that is closed when merged into default / stable. There are many points
I made about this almost a year ago but the biggest to me are: core hg
commands work out of the box, bitbucket knows how to deal with them, and
old clients know what to do with 90% of the cases.

I do think topics have a place in an advanced workflow but not as
feature branches. /endrant
phabricator - June 15, 2018, 5:13 a.m.
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D3715#58720, @smf wrote:
  
  > Sean Farley <sean@farley.io> writes:
  >
  > > durin42 (Augie Fackler) <phabricator@mercurial-scm.org> writes:
  > > 
  > >> durin42 added subscribers: lothiraldan, smf, durin42.
  > >>  durin42 accepted this revision as: durin42.
  > >>  durin42 added a comment.
  > >> 
  > >>   I'm in favor, but feel like I've got enough conflict of interest I shouldn't land the patches.
  > >>   
  > >>   @smf @lothiraldan this might be of interest to both of you?
  > > 
  > > Side note: I keep missing messages that I'm tagged in because I'm not
  > >  explicitly mentioned in a CC field. Is it possible to add a CC to each
  > >  person tagged in a message?
  > > 
  > > Side note2: Phabricator emails are really non-trivial to parse and
  > >  (worse!) search. The raw emails are not simple, raw text so I'm having
  > >  trouble tagging these for higher priority.
  >
  > I think I got this ironed out now.
  >
  > > Thanks for alerting me of this series! I've had a discussion with Martin
  > >  about this on IRC but I'm a bit out of time today to respond (but
  > >  definitely do want to respond). (I'm going to try to spend some time in
  > >  the mornings to do my email triaging so I can get back on top of this
  > >  list.)
  >
  > This patch strikes me as a Seems-Like-A-Good-Idea-But-Could-Blowup type
  >  of thing. So, what this patch does is conditionally change the behavior
  >  of 'log -r' based on the type of object passed in.
  
  
  No, it just allows namespaces to do that. As I said (or tried to say) in the commit message, `hg log -r stable` is protected by BC, so we can't change it. There should be no functional change from this patch.
  
  Unfortunately, I think much of your comments below were based on this incorrect assumption about what this patch does. I've read through it, but it doesn't seem very relevant.
  
  > "But what about topics?" you ask. Well,
  >  personally, I think that extension should add -t to log if it wants that
  >  functionality (-t is available to both 'push' and 'log' for those
  >  curious).
  
  This is closer to the actual point of this patch. As I tried to say in the commit message, this patch allows an extension to indicate that the name->nodes mapping it provides should present in the plain revset symbol. For example, if the topics extension opts in to this behavior, then `hg log -r my-topic` would start including all nodes in the topic. More important to me (I don't use topics) is that our Google-internal extension could opt in (it would kind of lets you do something like `hg log -r D3715` and get current and past versions of https://phab.mercurial-scm.org/D3715). Again, I don't intend to change how `hg log -r stable` behaves.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3715

To: martinvonz, #hg-reviewers, durin42
Cc: durin42, smf, lothiraldan, mercurial-devel
phabricator - June 15, 2018, 5:24 a.m.
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D3715#58726, @martinvonz wrote:
  
  > In https://phab.mercurial-scm.org/D3715#58720, @smf wrote:
  >
  > > Sean Farley <sean@farley.io> writes:
  > >
  > > > durin42 (Augie Fackler) <phabricator@mercurial-scm.org> writes:
  > > > 
  > > >> durin42 added subscribers: lothiraldan, smf, durin42.
  > > >>  durin42 accepted this revision as: durin42.
  > > >>  durin42 added a comment.
  > > >> 
  > > >>   I'm in favor, but feel like I've got enough conflict of interest I shouldn't land the patches.
  > > >>   
  > > >>   @smf @lothiraldan this might be of interest to both of you?
  > > > 
  > > > Side note: I keep missing messages that I'm tagged in because I'm not
  > > >  explicitly mentioned in a CC field. Is it possible to add a CC to each
  > > >  person tagged in a message?
  > > > 
  > > > Side note2: Phabricator emails are really non-trivial to parse and
  > > >  (worse!) search. The raw emails are not simple, raw text so I'm having
  > > >  trouble tagging these for higher priority.
  > >
  > > I think I got this ironed out now.
  > >
  > > > Thanks for alerting me of this series! I've had a discussion with Martin
  > > >  about this on IRC but I'm a bit out of time today to respond (but
  > > >  definitely do want to respond). (I'm going to try to spend some time in
  > > >  the mornings to do my email triaging so I can get back on top of this
  > > >  list.)
  > >
  > > This patch strikes me as a Seems-Like-A-Good-Idea-But-Could-Blowup type
  > >  of thing. So, what this patch does is conditionally change the behavior
  > >  of 'log -r' based on the type of object passed in.
  >
  >
  > No, it just allows namespaces to do that. As I said (or tried to say) in the commit message, `hg log -r stable` is protected by BC, so we can't change it. There should be no functional change from this patch.
  >
  > Unfortunately, I think much of your comments below were based on this incorrect assumption about what this patch does. I've read through it, but it doesn't seem very relevant.
  >
  > > "But what about topics?" you ask. Well,
  > >  personally, I think that extension should add -t to log if it wants that
  > >  functionality (-t is available to both 'push' and 'log' for those
  > >  curious).
  >
  > This is closer to the actual point of this patch. As I tried to say in the commit message, this patch allows an extension to indicate that the name->nodes mapping it provides should present in the plain revset symbol. For example, if the topics extension opts in to this behavior, then `hg log -r my-topic` would start including all nodes in the topic. More important to me (I don't use topics) is that our Google-internal extension could opt in (it would kind of lets you do something like `hg log -r D3715` and get current and past versions of https://phab.mercurial-scm.org/D3715). Again, I don't intend to change how `hg log -r stable` behaves.
  
  
  Also, I said in the commit message that I view a topic as a set of commits. Namespaces allows modeling that (as I'm sure you know). The topics extension takes advantage of that and does model it like that (i.e. by letting namemap() map a topic name to many nodes). It's just that the revset code resolves the symbol to a single value. To help illustrate what I mean, I find it a little silly that the topic extension tests pass with this patch applied:
  
  diff --git a/hgext3rd/topic/__init__.py b/hgext3rd/topic/__init__.py
  
  - a/hgext3rd/topic/__init__.py
  
  +++ b/hgext3rd/topic/__init__.py
  @@ -284,7 +284,7 @@ def _namemap(repo, name):
  
    if name not in repo.topics:
        return []
    node = repo.changelog.node
  
  - return [node(rev) for rev in repo.revs('topic(%s)', name)]
  
  +    return [node(rev) for rev in repo.revs('max(topic(%s))', name)]
  
  def _nodemap(repo, node):
  
    ctx = repo[node]

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3715

To: martinvonz, #hg-reviewers, durin42
Cc: durin42, smf, lothiraldan, mercurial-devel
Sean Farley - June 15, 2018, 5:37 a.m.
--=-=-=
Content-Type: text/plain


martinvonz (Martin von Zweigbergk) <phabricator@mercurial-scm.org> writes:

> martinvonz added a comment.
>
>
>   In https://phab.mercurial-scm.org/D3715#58720, @smf wrote:
>
>   > Sean Farley <sean@farley.io> writes:
>   >
>   > > durin42 (Augie Fackler) <phabricator@mercurial-scm.org> writes:
>   > >
>   > >> durin42 added subscribers: lothiraldan, smf, durin42.
>   > >>  durin42 accepted this revision as: durin42.
>   > >>  durin42 added a comment.
>   > >>
>   > >>   I'm in favor, but feel like I've got enough conflict of interest I shouldn't land the patches.
>   > >>
>   > >>   @smf @lothiraldan this might be of interest to both of you?
>   > >
>   > > Side note: I keep missing messages that I'm tagged in because I'm not
>   > >  explicitly mentioned in a CC field. Is it possible to add a CC to each
>   > >  person tagged in a message?
>   > >
>   > > Side note2: Phabricator emails are really non-trivial to parse and
>   > >  (worse!) search. The raw emails are not simple, raw text so I'm having
>   > >  trouble tagging these for higher priority.
>   >
>   > I think I got this ironed out now.
>   >
>   > > Thanks for alerting me of this series! I've had a discussion with Martin
>   > >  about this on IRC but I'm a bit out of time today to respond (but
>   > >  definitely do want to respond). (I'm going to try to spend some time in
>   > >  the mornings to do my email triaging so I can get back on top of this
>   > >  list.)
>   >
>   > This patch strikes me as a Seems-Like-A-Good-Idea-But-Could-Blowup type
>   >  of thing. So, what this patch does is conditionally change the behavior
>   >  of 'log -r' based on the type of object passed in.
>
>
>   No, it just allows namespaces to do that. As I said (or tried to say) in the commit message, `hg log -r stable` is protected by BC, so we can't change it. There should be no functional change from this patch.
>
>   Unfortunately, I think much of your comments below were based on this incorrect assumption about what this patch does. I've read through it, but it doesn't seem very relevant.

No, I understand what you're trying to do. I was attempting to paint a
picture of what it looks like to me in the sense of "this works for one
type of object but not this other one". Why? Why does it work for topics
and not branches? As a user, and believe me I have years of experience
with angry ones, they do not understand the nuanced differences between
branches and topics. They just lump them all together.

Topics is going out of its way to pretend they are in the same namespace
as branches (that's the motivation of my mini rant in my previous
message). The difference between branches and topics is lost on everyone
outside this thread. When I tried to introduce them onto Bitbucket I
only got headaches: "can I merge between a topic and branch? can I merge
between a topic on branch A and another topic on branch B? if so, what
does that mean? Why is branch B still open? I merged one topic into
another and it closed the three topics it was based on, why?"

It solidified my stance that there should only be one namespace for most
users: branches.

And yes, your patch only applies to topics. My point is the cognitive
load. Why is it '-r topic-foo' for one and not '-r branch-bar' for the
other? To the average hg user, both topics and branches are treated
(semantically) the same. Why one and not the other?

Let's say you and I say a repo. I add a branch (because that's the only
thing I use) and you add a topic. Cool. Another dev comes along. 'hg log
-r martins-work' lists all the commits of his feature work, but, 'hg log
-r seans-work' only lists the tip-most commit of my feature work. Why?
Why should that dev be tripped up by this?

Furthermore, what about the revset language? Should 'topic-foo' become
'topic(topic-foo)' in our AST? I was hoping the abstraction I made
between branches and topics was clear but I guess I am too terse
sometimes, so I apologize.

>   > "But what about topics?" you ask. Well,
>   >  personally, I think that extension should add -t to log if it wants that
>   >  functionality (-t is available to both 'push' and 'log' for those
>   >  curious).
>
>   This is closer to the actual point of this patch. As I tried to say in the commit message, this patch allows an extension to indicate that the name->nodes mapping it provides should present in the plain revset symbol. For example, if the topics extension opts in to this behavior, then `hg log -r my-topic` would start including all nodes in the topic. More important to me (I don't use topics) is that our Google-internal extension could opt in (it would kind of lets you do something like `hg log -r D3715` and get current and past versions of https://phab.mercurial-scm.org/D3715). Again, I don't intend to change how `hg log -r stable` behaves.

Right, but as I tried to explain in my previous email I believe your
extension should add another custom flag of '--phab D3715' to signify
that "I want all the commits of D3715" (e.g. --stack I think in
phabread).

And what about push? Should 'hg push -r D3715' push those other diffs as
well? I know it sounds silly but that's what being an AST means (at
least to me).

Hopefully this message was a bit more clear but if not, then I can try
to discuss in person if that's better.

--=-=-=
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEXQX2nmDejz5RkkDD/9jcOZ+fO9YFAlsjUK4ACgkQ/9jcOZ+f
O9ZoZA//f5RD6iA9HnwYhXdt126V1cB5xmgkGaZoDNY54B+hkrmeBQ+/6w1PfQij
pSZToUwhANrrh3L4vnwQOL3al6HII+sftTSmd+sh8/ch1r59+L0f7RYSEPXqapuI
nEE8otm8V18KKvHu48smOl69EoB3UDASX3C2b7E27VF0hJQjPOZV8ai9KOhRZpti
5Tftwhkh7PKdyOdk7JzNkNKZKcfQO1294YIpOFtuhrXa0GH5GPfbVXDT5gIcsIX2
vT9uZ9UWSpQANT7UfIoCh0nMH90uLnG4Jb68Q6nL8F6ZPnmmDn7dO6LV5SdtZUe3
svNBqPU5lSvZKokBpRdhqjYoxCtZ1KBFSGHO/N5km44HckshpS2FhyiXpNJrSxK/
K6lAc0VKTFmj5WwL1VYQiv5pr2GMPppRi5TSkVlsXtDio79qKd31ER9Nkc/ZibEF
A+wn6hLjtgPZAl9BCvGNSAciOPlfC4C3BJ2k7BGcqMGxYir0Kyp1+sl7R68dZ5sm
DEPLLlCmKuuHCOQ+NgEmsavLpEvTO/uypCW3cOBNy8FfLxgLvJZkeXG0daHJJkA1
Jb4dNiNXXPo6c/EL+xpCIUgKJdPmGab/qs7z9WlyZhUUmzpcZgzPFuCImnebgeUW
ABUAk9yBB/thUrunM9QmMKu2QZ8ggUoPInlt5blMbRPUxeMPbJs=
=O0Nu
-----END PGP SIGNATURE-----
--=-=-=--
phabricator - June 15, 2018, 6:16 a.m.
smf added a comment.


  It seems I'm having email sending trouble ... going to attempt to send again

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3715

To: martinvonz, #hg-reviewers, durin42
Cc: durin42, smf, lothiraldan, mercurial-devel
phabricator - June 15, 2018, 6:55 a.m.
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D3715#58729, @smf wrote:
  
  > --=-=-=
  >  Content-Type: text/plain
  >
  > martinvonz (Martin von Zweigbergk) <phabricator@mercurial-scm.org> writes:
  >
  > > martinvonz added a comment.
  > > 
  > >   In https://phab.mercurial-scm.org/D3715#58720, @smf wrote:
  > >   
  > >   > Sean Farley <sean@farley.io> writes:
  > >   >
  > >   > This patch strikes me as a Seems-Like-A-Good-Idea-But-Could-Blowup type
  > >   >  of thing. So, what this patch does is conditionally change the behavior
  > >   >  of 'log -r' based on the type of object passed in.
  > >   
  > >   
  > >   No, it just allows namespaces to do that. As I said (or tried to say) in the commit message, `hg log -r stable` is protected by BC, so we can't change it. There should be no functional change from this patch.
  > >   
  > >   Unfortunately, I think much of your comments below were based on this incorrect assumption about what this patch does. I've read through it, but it doesn't seem very relevant.
  >
  > No, I understand what you're trying to do. I was attempting to paint a
  >  picture of what it looks like to me in the sense of "this works for one
  >  type of object but not this other one". Why? Why does it work for topics
  >  and not branches? As a user, and believe me I have years of experience
  >  with angry ones, they do not understand the nuanced differences between
  >  branches and topics. They just lump them all together.
  
  
  As I said in the commit message, I'm aware of this argument, I just disagree. I think most users have never even tried to do `hg log -r default` and that's the only shipped-with-core namespace that behaves funny (IMO).
  
  > Topics is going out of its way to pretend they are in the same namespace
  >  as branches (that's the motivation of my mini rant in my previous
  >  message). The difference between branches and topics is lost on everyone
  >  outside this thread. When I tried to introduce them onto Bitbucket I
  >  only got headaches: "can I merge between a topic and branch? can I merge
  >  between a topic on branch A and another topic on branch B? if so, what
  >  does that mean? Why is branch B still open? I merged one topic into
  >  another and it closed the three topics it was based on, why?"
  > 
  > It solidified my stance that there should only be one namespace for most
  >  users: branches.
  
  Sounds more like a criticism of topics.
  
  > And yes, your patch only applies to topics.
  
  *Maybe* to topics. That will be up to that extensions developers.
  
  > My point is the cognitive
  >  load. Why is it '-r topic-foo' for one and not '-r branch-bar' for the
  >  other? To the average hg user, both topics and branches are treated
  >  (semantically) the same. Why one and not the other?
  
  For historical reasons (`hg log -r branch-bar` is protected by BC). Again, I think few users have even tried that command. If they did, perhaps they did it hoping that it would list all the commits on the branch?
  
  > Let's say you and I say a repo. I add a branch (because that's the only
  >  thing I use) and you add a topic. Cool. Another dev comes along. 'hg log
  >  -r martins-work' lists all the commits of his feature work, but, 'hg log
  >  -r seans-work' only lists the tip-most commit of my feature work. Why?
  >  Why should that dev be tripped up by this?
  
  See my commit message.
  
  > Furthermore, what about the revset language? Should 'topic-foo' become
  >  'topic(topic-foo)' in our AST?
  
  No. The way I did it in this patch was to make the symbol "topic-foo" itself resolve to multiple revisions.
  
  > I was hoping the abstraction I made
  >  between branches and topics was clear but I guess I am too terse
  >  sometimes, so I apologize.
  
  I think I see the similarities between them (and I did before your message too :)).
  
  > 
  > 
  >>   > "But what about topics?" you ask. Well,
  >>   >  personally, I think that extension should add -t to log if it wants that
  >>   >  functionality (-t is available to both 'push' and 'log' for those
  >>   >  curious).
  >>   
  >>   This is closer to the actual point of this patch. As I tried to say in the commit message, this patch allows an extension to indicate that the name->nodes mapping it provides should present in the plain revset symbol. For example, if the topics extension opts in to this behavior, then `hg log -r my-topic` would start including all nodes in the topic. More important to me (I don't use topics) is that our Google-internal extension could opt in (it would kind of lets you do something like `hg log -r D3715` and get current and past versions of https://phab.mercurial-scm.org/D3715). Again, I don't intend to change how `hg log -r stable` behaves.
  > 
  > Right, but as I tried to explain in my previous email I believe your
  >  extension should add another custom flag of '--phab https://phab.mercurial-scm.org/D3715' to signify
  >  that "I want all the commits of https://phab.mercurial-scm.org/D3715" (e.g. --stack I think in
  >  phabread).
  
  Revsets were invented to prevent that kind of things, no? What if I want `hg log -r 'only(D3715,@)::'`? We obviously don't want a flag for that.
  
  > And what about push? Should 'hg push -r https://phab.mercurial-scm.org/D3715' push those other diffs as
  >  well?
  
  Yes. Of course, things like `hg co D3715` will still need to resolve the revset (which "https://phab.mercurial-scm.org/D3715" is, and which "default" also is) to a single node. No change there.
  
  > I know it sounds silly but that's what being an AST means (at
  >  least to me).
  
  I'm not sure what that means, but I hope my replies clarified anyway.
  
  > Hopefully this message was a bit more clear but if not, then I can try
  >  to discuss in person if that's better.
  
  Sure, let me know if my replied don't clarify enough.
  
  Again, there should be no user-visible change in this patch. We could safely queue this patch and then let the topics people and the Google people experiment with the feature (or not) and ask them later if it was helpful or harmful.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3715

To: martinvonz, #hg-reviewers, durin42
Cc: durin42, smf, lothiraldan, mercurial-devel
Sean Farley - June 15, 2018, 7:19 a.m.
martinvonz (Martin von Zweigbergk) <phabricator@mercurial-scm.org> writes:

> martinvonz added a comment.
>
>
>   In https://phab.mercurial-scm.org/D3715#58729, @smf wrote:
>   
>   > --=-=-=
>   >  Content-Type: text/plain
>   >
>   > martinvonz (Martin von Zweigbergk) <phabricator@mercurial-scm.org> writes:
>   >
>   > > martinvonz added a comment.
>   > > 
>   > >   In https://phab.mercurial-scm.org/D3715#58720, @smf wrote:
>   > >   
>   > >   > Sean Farley <sean@farley.io> writes:
>   > >   >
>   > >   > This patch strikes me as a Seems-Like-A-Good-Idea-But-Could-Blowup type
>   > >   >  of thing. So, what this patch does is conditionally change the behavior
>   > >   >  of 'log -r' based on the type of object passed in.
>   > >   
>   > >   
>   > >   No, it just allows namespaces to do that. As I said (or tried to say) in the commit message, `hg log -r stable` is protected by BC, so we can't change it. There should be no functional change from this patch.
>   > >   
>   > >   Unfortunately, I think much of your comments below were based on this incorrect assumption about what this patch does. I've read through it, but it doesn't seem very relevant.
>   >
>   > No, I understand what you're trying to do. I was attempting to paint a
>   >  picture of what it looks like to me in the sense of "this works for one
>   >  type of object but not this other one". Why? Why does it work for topics
>   >  and not branches? As a user, and believe me I have years of experience
>   >  with angry ones, they do not understand the nuanced differences between
>   >  branches and topics. They just lump them all together.
>   
>   
>   As I said in the commit message, I'm aware of this argument, I just disagree. I think most users have never even tried to do `hg log -r default` and that's the only shipped-with-core namespace that behaves funny (IMO).
>   
>   > Topics is going out of its way to pretend they are in the same namespace
>   >  as branches (that's the motivation of my mini rant in my previous
>   >  message). The difference between branches and topics is lost on everyone
>   >  outside this thread. When I tried to introduce them onto Bitbucket I
>   >  only got headaches: "can I merge between a topic and branch? can I merge
>   >  between a topic on branch A and another topic on branch B? if so, what
>   >  does that mean? Why is branch B still open? I merged one topic into
>   >  another and it closed the three topics it was based on, why?"
>   > 
>   > It solidified my stance that there should only be one namespace for most
>   >  users: branches.
>   
>   Sounds more like a criticism of topics.
>   
>   > And yes, your patch only applies to topics.
>   
>   *Maybe* to topics. That will be up to that extensions developers.
>   
>   > My point is the cognitive
>   >  load. Why is it '-r topic-foo' for one and not '-r branch-bar' for the
>   >  other? To the average hg user, both topics and branches are treated
>   >  (semantically) the same. Why one and not the other?
>   
>   For historical reasons (`hg log -r branch-bar` is protected by BC). Again, I think few users have even tried that command. If they did, perhaps they did it hoping that it would list all the commits on the branch?
>   
>   > Let's say you and I say a repo. I add a branch (because that's the only
>   >  thing I use) and you add a topic. Cool. Another dev comes along. 'hg log
>   >  -r martins-work' lists all the commits of his feature work, but, 'hg log
>   >  -r seans-work' only lists the tip-most commit of my feature work. Why?
>   >  Why should that dev be tripped up by this?
>   
>   See my commit message.
>   
>   > Furthermore, what about the revset language? Should 'topic-foo' become
>   >  'topic(topic-foo)' in our AST?
>   
>   No. The way I did it in this patch was to make the symbol "topic-foo" itself resolve to multiple revisions.
>   
>   > I was hoping the abstraction I made
>   >  between branches and topics was clear but I guess I am too terse
>   >  sometimes, so I apologize.
>   
>   I think I see the similarities between them (and I did before your message too :)).
>   
>   > 
>   > 
>   >>   > "But what about topics?" you ask. Well,
>   >>   >  personally, I think that extension should add -t to log if it wants that
>   >>   >  functionality (-t is available to both 'push' and 'log' for those
>   >>   >  curious).
>   >>   
>   >>   This is closer to the actual point of this patch. As I tried to say in the commit message, this patch allows an extension to indicate that the name->nodes mapping it provides should present in the plain revset symbol. For example, if the topics extension opts in to this behavior, then `hg log -r my-topic` would start including all nodes in the topic. More important to me (I don't use topics) is that our Google-internal extension could opt in (it would kind of lets you do something like `hg log -r D3715` and get current and past versions of https://phab.mercurial-scm.org/D3715). Again, I don't intend to change how `hg log -r stable` behaves.
>   > 
>   > Right, but as I tried to explain in my previous email I believe your
>   >  extension should add another custom flag of '--phab https://phab.mercurial-scm.org/D3715' to signify
>   >  that "I want all the commits of https://phab.mercurial-scm.org/D3715" (e.g. --stack I think in
>   >  phabread).
>   
>   Revsets were invented to prevent that kind of things, no? What if I want `hg log -r 'only(D3715,@)::'`? We obviously don't want a flag for that.
>   
>   > And what about push? Should 'hg push -r https://phab.mercurial-scm.org/D3715' push those other diffs as
>   >  well?
>   
>   Yes. Of course, things like `hg co D3715` will still need to resolve the revset (which "https://phab.mercurial-scm.org/D3715" is, and which "default" also is) to a single node. No change there.
>   
>   > I know it sounds silly but that's what being an AST means (at
>   >  least to me).
>   
>   I'm not sure what that means, but I hope my replies clarified anyway.
>   
>   > Hopefully this message was a bit more clear but if not, then I can try
>   >  to discuss in person if that's better.
>   
>   Sure, let me know if my replied don't clarify enough.
>   
>   Again, there should be no user-visible change in this patch. We could safely queue this patch and then let the topics people and the Google people experiment with the feature (or not) and ask them later if it was helpful or harmful.

It's not that this is a user-visible change, it's that we're opening the
floodgates to confusing users. You haven't really addressed my questions
about -r branch and -r topic/google/whatever in the sense that you
haven't addressed the cognitive overload of a user. My point is that
this is dangerous and more harmful to users than the benefits.
Yuya Nishihara - June 15, 2018, 12:05 p.m.
>   > This patch strikes me as a Seems-Like-A-Good-Idea-But-Could-Blowup type
>   >  of thing. So, what this patch does is conditionally change the behavior
>   >  of 'log -r' based on the type of object passed in.
>   
>   
>   No, it just allows namespaces to do that. As I said (or tried to say) in the commit message, `hg log -r stable` is protected by BC, so we can't change it. There should be no functional change from this patch.

How about adding syntax to resolve multinode namespace symbols?
`stable~`, for example, will be resolved to all revisions in the stable
branch.

Implementation wise, we can get rid of the `revsymbol()` hack, and extra
`repo[n]` lookup won't be needed. A possible drawback (other than the '~'
suffix itself) is we'll soon run out of shell-safe symbols.
phabricator - June 15, 2018, 12:06 p.m.
yuja added a comment.


  >   > This patch strikes me as a Seems-Like-A-Good-Idea-But-Could-Blowup type
  >   >  of thing. So, what this patch does is conditionally change the behavior
  >   >  of 'log -r' based on the type of object passed in.
  >   
  >   
  >   No, it just allows namespaces to do that. As I said (or tried to say) in the commit message, `hg log -r stable` is protected by BC, so we can't change it. There should be no functional change from this patch.
  
  How about adding syntax to resolve multinode namespace symbols?
  `stable~`, for example, will be resolved to all revisions in the stable
  branch.
  
  Implementation wise, we can get rid of the `revsymbol()` hack, and extra
  `repo[n]` lookup won't be needed. A possible drawback (other than the '~'
  suffix itself) is we'll soon run out of shell-safe symbols.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3715

To: martinvonz, #hg-reviewers, durin42
Cc: yuja, durin42, smf, lothiraldan, mercurial-devel

Patch

diff --git a/tests/test-revset2.t b/tests/test-revset2.t
--- a/tests/test-revset2.t
+++ b/tests/test-revset2.t
@@ -675,6 +675,12 @@ 
   6
   $ log 'named("tags")'
   6
+  $ hg --config extensions.revnamesext=$TESTDIR/paritynamesext.py log --template '{rev}\n' -r even
+  0
+  2
+  4
+  6
+  8
 
 issue2437
 
diff --git a/tests/paritynamesext.py b/tests/paritynamesext.py
new file mode 100644
--- /dev/null
+++ b/tests/paritynamesext.py
@@ -0,0 +1,26 @@ 
+# Defines a namespace with names 'even' and 'odd'
+
+from __future__ import absolute_import
+
+from mercurial import (
+    namespaces,
+)
+
+def reposetup(ui, repo):
+    def namemap(repo, name):
+        if name == 'even':
+            parity = 0
+        elif name == 'odd':
+            parity = 1
+        else:
+            return []
+        return [repo[rev].node() for rev in repo if rev % 2 == parity]
+    def nodemap(repo, node):
+        return [repo[node].rev() % 2]
+
+    ns = namespaces.namespace(b'parity', templatename=b'parity',
+                              logname=b'parity',
+                              listnames=lambda r: ['even', 'odd'],
+                              namemap=namemap, nodemap=nodemap,
+                              multinode=True)
+    repo.names.addnamespace(ns)
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -488,7 +488,7 @@ 
     except error.RepoLookupError:
         return False
 
-def _revsymbol(repo, symbol):
+def _revsymbol(repo, symbol, allowmultiple):
     if not isinstance(symbol, bytes):
         msg = ("symbol (%s of type %s) was not a string, did you mean "
                "repo[symbol]?" % (symbol, type(symbol)))
@@ -524,9 +524,9 @@ 
 
         # look up bookmarks through the name interface
         try:
-            node = repo.names.singlenode(repo, symbol)
-            rev = repo.changelog.rev(node)
-            return [repo[rev]]
+            nodes = repo.names.nodes(repo, symbol, allowmultiple)
+            revs = [repo.changelog.rev(n) for n in nodes]
+            return [repo[r2] for r2 in revs] # "r2" to keep pyflakes happy
         except KeyError:
             pass
 
@@ -550,10 +550,19 @@ 
     i.e. things like ".", "tip", "1234", "deadbeef", "my-bookmark" work, but
     not "max(public())".
     """
-    ctxs = _revsymbol(repo, symbol)
+    ctxs = _revsymbol(repo, symbol, allowmultiple=False)
     assert len(ctxs) == 1
     return ctxs[0]
 
+def revsymbolmultiple(repo, symbol):
+    """Returns all contexts given a single revision symbol (as string).
+
+    This is similar to revsymbol(), but if "symbol" is a namespace symbol,
+    then this may return many context (if the namespace maps the symbol to
+    many revisions).
+    """
+    return _revsymbol(repo, symbol, allowmultiple=True)
+
 def _filterederror(repo, changeid):
     """build an exception to be raised about a filtered changeid
 
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -118,11 +118,14 @@ 
 def stringset(repo, subset, x, order):
     if not x:
         raise error.ParseError(_("empty string is not a valid revision"))
-    x = scmutil.intrev(scmutil.revsymbol(repo, x))
-    if (x in subset
-        or x == node.nullrev and isinstance(subset, fullreposet)):
-        return baseset([x])
-    return baseset()
+    ctxs = scmutil.revsymbolmultiple(repo, x)
+    if len(ctxs) == 1:
+        x = scmutil.intrev(ctxs[0])
+        if (x in subset
+            or x == node.nullrev and isinstance(subset, fullreposet)):
+            return baseset([x])
+        return baseset()
+    return subset & baseset(sorted(ctx.rev() for ctx in ctxs))
 
 def rangeset(repo, subset, x, y, order):
     m = getset(repo, fullreposet(repo), x)
diff --git a/mercurial/namespaces.py b/mercurial/namespaces.py
--- a/mercurial/namespaces.py
+++ b/mercurial/namespaces.py
@@ -93,7 +93,7 @@ 
             def generatekw(context, mapping):
                 return templatekw.shownames(context, mapping, namespace.name)
 
-    def singlenode(self, repo, name):
+    def nodes(self, repo, name, allowmultiple=False):
         """
         Return the 'best' node for the given name. Best means the first node
         in the first nonempty list returned by a name-to-nodes mapping function
@@ -104,12 +104,12 @@ 
         for ns, v in self._names.iteritems():
             n = v.namemap(repo, name)
             if n:
+                if (allowmultiple and v.multinode) or len(n) == 1:
+                    return n
                 # return max revision number
-                if len(n) > 1:
-                    cl = repo.changelog
-                    maxrev = max(cl.rev(node) for node in n)
-                    return cl.node(maxrev)
-                return n[0]
+                cl = repo.changelog
+                maxrev = max(cl.rev(node) for node in n)
+                return [cl.node(maxrev)]
         raise KeyError(_('no such name: %s') % name)
 
 class namespace(object):
@@ -142,7 +142,7 @@ 
 
     def __init__(self, name, templatename=None, logname=None, colorname=None,
                  logfmt=None, listnames=None, namemap=None, nodemap=None,
-                 deprecated=None, builtin=False):
+                 deprecated=None, builtin=False, multinode=False):
         """create a namespace
 
         name: the namespace to be registered (in plural form)
@@ -158,6 +158,9 @@ 
         nodemap: function that inputs a node, output name(s)
         deprecated: set of names to be masked for ordinary use
         builtin: whether namespace is implemented by core Mercurial
+        multinode: whether namespace can have multiple nodes associated with
+                   one name (`hg log -r that-name` will then list multiple
+                   nodes)
         """
         self.name = name
         self.templatename = templatename
@@ -187,6 +190,7 @@ 
             self.deprecated = deprecated
 
         self.builtin = builtin
+        self.multinode = multinode
 
     def names(self, repo, node):
         """method that returns a (sorted) list of names in a namespace that