Open
Bug 1122188
Opened 7 years ago
Updated 5 years ago
Template keywords for identifying commits that are Firefox tree heads
Categories
(Developer Services :: Mercurial: firefoxtree, defect)
Developer Services
Mercurial: firefoxtree
Tracking
(Not tracked)
NEW
People
(Reporter: gps, Unassigned)
Details
Attachments
(2 files, 1 obsolete file)
3.50 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
2.58 KB,
patch
|
Details | Diff | Splinter Review |
firefoxtree needs template keywords to display firefox tree heads and nearest firefox tree head.
Reporter | ||
Comment 1•7 years ago
|
||
This patch implements {fxheads} to return a list of string Firefox trees whose heads were last seen on a changeset. The caching logic was split out to facilitate more templates in the future.
Attachment #8550123 -
Flags: review?(ted)
Reporter | ||
Updated•7 years ago
|
Assignee: nobody → gps
Status: NEW → ASSIGNED
Reporter | ||
Comment 2•7 years ago
|
||
I want to change this a bit before it lands. So only asking for feedback.
Attachment #8550131 -
Flags: feedback?(ted)
Comment 3•7 years ago
|
||
Comment on attachment 8550123 [details] [diff] [review] firefoxtree: implement {fxheads} template keyword () Review of attachment 8550123 [details] [diff] [review]: ----------------------------------------------------------------- ::: hgext/firefoxtree/__init__.py @@ +396,5 @@ > + > + res = set() > + for tag, node, tree, uri in labels: > + if node == ctx.node(): > + res.add(tag) Couldn't you do this all in the set constructor with a generator expression? res = set(t for t,n,tr,u in labels if n == ctx.node())
Attachment #8550123 -
Flags: review?(ted) → review+
Comment 4•7 years ago
|
||
Comment on attachment 8550131 [details] [diff] [review] firefoxtree: implement {nearestfxhead} for finding what tree a changeset is on () Review of attachment 8550131 [details] [diff] [review]: ----------------------------------------------------------------- I think this could be okay with a small change. ::: hgext/firefoxtree/__init__.py @@ +406,5 @@ > +def template_nearestfxhead(repo, ctx, templ, cache, revcache, **args): > + """:nearestfxhead: String. Nearest Firefox tree in the ancestry of this > + changeset. > + > + This template only works on unpublished changesets due to performance This seems kind of odd and not the behavior I'd expect. Since the original request here was for something that'd work with the prompt extension, I would expect this to return a consistent result regardless. Otherwise, if you had your prompt set to "{nearestfxhead}", you could wind up with behavior like: luser central:/build/mozilla-central$ hg pull inbound <...> luser central:/build/mozilla-central$ hg up inbound luser :/build/mozilla-central$ # wtf or luser inbound:/build/mozilla-central$ hg push inbound <...> luser :/build/mozilla-central$ # wtf After thinking about it I understand why you're doing this, but I think a minor tweak to your restriction here could fix this issue and still not make it do unbounded DAG walks--simply say that it only works on unpublished changesets *and* the known Firefox tree heads. Then you can just quickly check if ctx.rev() is one of the known labels and fall through to your existing logic. @@ +416,5 @@ > + > + if ctx.phase() == phases.public: > + return None > + > + for rev in repo.changelog.ancestors([ctx.rev()]): I was toying with reproducing this in `hg log` using the following command: hg log -r "first(reverse(fxheads() & ancestors(.)))" Is it possible to write the equivalent of that revset expression here to make it performant? @@ +418,5 @@ > + return None > + > + for rev in repo.changelog.ancestors([ctx.rev()]): > + actx = repo[rev] > + for tag, node, tree, uri in labels: I would probably just make a dict of node->label up above and use that here.
Attachment #8550131 -
Flags: feedback?(ted) → feedback-
Comment 5•7 years ago
|
||
Comment on attachment 8550131 [details] [diff] [review] firefoxtree: implement {nearestfxhead} for finding what tree a changeset is on () Review of attachment 8550131 [details] [diff] [review]: ----------------------------------------------------------------- ::: hgext/firefoxtree/__init__.py @@ +418,5 @@ > + return None > + > + for rev in repo.changelog.ancestors([ctx.rev()]): > + actx = repo[rev] > + for tag, node, tree, uri in labels: Furthering that point, maybe it makes sense to just have _getcachedlabels return that dict?
Reporter | ||
Comment 6•7 years ago
|
||
https://hg.mozilla.org/hgcustom/version-control-tools/rev/0dfbdacd7a45
Reporter | ||
Comment 7•7 years ago
|
||
Your revset suggestion for the template implementation is a pretty good one. Thinking about the "nearest tree" problem, I think there are 2 use cases with the difference being the direction of the DAG walk: 1. Find the nearest tree this changeset is based on (walking towards roots) (useful for a shell prompt, local development, etc) 2. Find the trees a changeset is part of (walking towards heads) (useful for archeology) I think we should implement 2 templates for these different use cases. But I'm not sure what to call them. Naming is hard.
Reporter | ||
Comment 8•7 years ago
|
||
Here is a slightly better implementation of the walk-towards-roots template.
Reporter | ||
Updated•7 years ago
|
Attachment #8550131 -
Attachment is obsolete: true
Comment 9•7 years ago
|
||
I think your {nearestfxhead} naming is fine for use case #1, keeping {fxheads} for #2.
Reporter | ||
Comment 10•7 years ago
|
||
Well, there is a 3rd use case: show me the labels for Firefox tree heads that are on this specific commit. That's what {fxheads} currently does.
Reporter | ||
Comment 11•5 years ago
|
||
I'm not actively working on this.
Assignee: gps → nobody
Status: ASSIGNED → NEW
You need to log in
before you can comment on or make changes to this bug.
Description
•