This is an archive of the discontinued Mercurial Phabricator instance.

Differential D447

templatekw: choose {latesttag} by len(changes), not date (issue5659)
ClosedPublic

Authored by martinvonz on Aug 18 2017, 4:48 PM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHGfb672eac2702: templatekw: choose {latesttag} by len(changes), not date (issue5659)
Summary

As Augie reported in the bug, the current heuristic of choosing the
best tag of a merge commit by taking the one with newest tag (in terms
of tagging date) currently fails in the Mercurial repo itself. Copying
the example from Yuya:

$ hg glog -T '{node|short} {latesttag}+{latesttagdistance}\n' \
  -r '4.2.3: & (merge() + parents(merge()) + tag())'
o    02a745c20121 4.2.3+5
|\
| o    86aca74a063b 4.2.3+4
| |\
| | o  e6d8ee3c9ec3 4.3-rc+109
| | |
| | ~
o |  a3ce07e2dde5 4.3.1+2
: |
o |  3fee7f7d2da0 4.3.1+0
|/
o    98e990bb7330 4.2.3+3
|\
| ~
o  506d7e48fbe6 4.2.3+2
:
o  943c91326b23 4.2.3+0
|
~

It seems to me like the best choice is the tag with the smallest
number of changes since it (across all paths, not the longest single
path). So that's what this patch does, even though it's
costly. Best-of-5 timings for Yuya's command above shows a slowdown
from 1.293s to 1.610s. We can optimize it later.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.Aug 18 2017, 4:48 PM
yuja requested changes to this revision.Aug 19 2017, 3:11 AM
yuja added a subscriber: yuja.
yuja added inline comments.
mercurial/templatekw.py
222

This should be enabled only if ptags[0][2] != ptags[1][2].

The latesttagdistance is documented to return the longest path to the latest tag.
f04d17912441

This revision now requires changes to proceed.Aug 19 2017, 3:11 AM
martinvonz added inline comments.Aug 19 2017, 11:32 AM
mercurial/templatekw.py
222

This should be enabled only if ptags[0][2] != ptags[1][2].

Good point. Done.

The latesttagdistance is documented to return the longest path to the latest tag.

I'm not changing latesttagdistance (AFAIK), only the definition of latesttag.

f04d17912441

Luckily, that's just the commit message and "hg help templates.latesttag" just says "The global tags on the most recent globally tagged ancestor of this changeset" and doesn't define what "most recent" is.

martinvonz edited edge metadata.Aug 19 2017, 11:32 AM
martinvonz updated this revision to Diff 1088.
yuja added inline comments.Aug 19 2017, 11:47 AM
mercurial/templatekw.py
222

I'm not changing latesttagdistance (AFAIK), only the definition of latesttag.

Try ptags = reversed(...) and run tests. The largest pdist should be selected
so that latesttagdistance returns the longest path.

martinvonz edited the summary of this revision. (Show Details)Aug 19 2017, 12:07 PM
martinvonz added inline comments.Aug 19 2017, 12:09 PM
mercurial/templatekw.py
222

This should be enabled only if ptags[0][2] != ptags[1][2].

Good point. Done.

Great point, actually! See updated timings in commit message.

martinvonz updated this revision to Diff 1089.Aug 19 2017, 12:29 PM
martinvonz added inline comments.Aug 19 2017, 12:29 PM
mercurial/templatekw.py
222

I'm not changing latesttagdistance (AFAIK), only the definition of latesttag.

Try ptags = reversed(...) and run tests. The largest pdist should be selected
so that latesttagdistance returns the longest path.

Ah, right, when the tag is in the common ancestor, we should choose the longest branch. Good catch. See how you like this change.

yuja accepted this revision.Aug 19 2017, 10:51 PM

This one looks good to me, thanks. I leave it to Sean since he said he's queued the previous series.

This revision is now accepted and ready to land.Aug 19 2017, 10:51 PM
martinvonz edited the summary of this revision. (Show Details)Aug 21 2017, 4:37 PM
Closed by commit rHGfb672eac2702: templatekw: choose {latesttag} by len(changes), not date (issue5659) (authored by martinvonz). · Explain WhyAug 22 2017, 1:24 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/templatekw.py (16 lines)
Mtests/test-command-template.t (54 lines)

Diff 1088

mercurial/templatekw.py

ctx = repo[rev] ctx = repo[rev]
tags = [t for t in ctx.tags() tags = [t for t in ctx.tags()
if (repo.tagtype(t) and repo.tagtype(t) != 'local' if (repo.tagtype(t) and repo.tagtype(t) != 'local'
and match(t))] and match(t))]
if tags: if tags:
latesttags[rev] = ctx.date()[0], 0, [t for t in sorted(tags)] latesttags[rev] = ctx.date()[0], 0, [t for t in sorted(tags)]
continue continue
try: try:
# The tuples are laid out so the right one can be found by def key(x):
# comparison. changessincetag = len(repo.revs('only(%d, %s)',
pdate, pdist, ptag = max( ctx.rev(), x[2][0]))
latesttags[p.rev()] for p in ctx.parents()) # Smallest number of changes since tag wins. Date is used
# as tiebreaker.
return [-changessincetag, x[0]]
ptags = [latesttags[p.rev()] for p in ctx.parents()]
if len(ptags) > 1 and ptags[0][2] != ptags[1][2]:
pdate, pdist, ptag = max(ptags, key=key)
else:
# Don't call expensive key function if not necessary
pdate, pdist, ptag = ptags[0]
yujaUnsubmitted
Not Done

This should be enabled only if ptags[0][2] != ptags[1][2].

The latesttagdistance is documented to return the longest path to the latest tag.
f04d17912441

yuja: This should be enabled only if `ptags[0][2] != ptags[1][2]`. The latesttagdistance is…
martinvonzAuthorUnsubmitted
Not Done

This should be enabled only if ptags[0][2] != ptags[1][2].

Good point. Done.

The latesttagdistance is documented to return the longest path to the latest tag.

I'm not changing latesttagdistance (AFAIK), only the definition of latesttag.

f04d17912441

Luckily, that's just the commit message and "hg help templates.latesttag" just says "The global tags on the most recent globally tagged ancestor of this changeset" and doesn't define what "most recent" is.

martinvonz: > This should be enabled only if ptags[0][2] != ptags[1][2]. Good point. Done. > The…
yujaUnsubmitted
Not Done

I'm not changing latesttagdistance (AFAIK), only the definition of latesttag.

Try ptags = reversed(...) and run tests. The largest pdist should be selected
so that latesttagdistance returns the longest path.

yuja: > I'm not changing latesttagdistance (AFAIK), only the definition of latesttag. Try `ptags =…
martinvonzAuthorUnsubmitted
Not Done

I'm not changing latesttagdistance (AFAIK), only the definition of latesttag.

Try ptags = reversed(...) and run tests. The largest pdist should be selected
so that latesttagdistance returns the longest path.

Ah, right, when the tag is in the common ancestor, we should choose the longest branch. Good catch. See how you like this change.

martinvonz: >> I'm not changing latesttagdistance (AFAIK), only the definition of latesttag. >Try ptags =…
martinvonzAuthorUnsubmitted
Not Done

This should be enabled only if ptags[0][2] != ptags[1][2].

Good point. Done.

Great point, actually! See updated timings in commit message.

martinvonz: >> This should be enabled only if ptags[0][2] != ptags[1][2]. > Good point. Done. Great point…
except KeyError: except KeyError:
# Cache miss - recurse # Cache miss - recurse
todo.append(rev) todo.append(rev)
todo.extend(p.rev() for p in ctx.parents()) todo.extend(p.rev() for p in ctx.parents())
continue continue
latesttags[rev] = pdate, pdist + 1, ptag latesttags[rev] = pdate, pdist + 1, ptag
return latesttags[rev] return latesttags[rev]

tests/test-command-template.t

| | | |
o | 2: null+3 o | 2: null+3
|/ |/
o 1: null+2 o 1: null+2
| |
o 0: null+1 o 0: null+1
One common tag: longest path wins: One common tag: longest path wins for {latesttagdistance}:
$ hg tag -r 1 -m t1 -d '6 0' t1 $ hg tag -r 1 -m t1 -d '6 0' t1
$ hg log -G --template '{rev}: {latesttag}+{latesttagdistance}\n' $ hg log -G --template '{rev}: {latesttag}+{latesttagdistance}\n'
@ 6: t1+4 @ 6: t1+4
| |
o 5: t1+3 o 5: t1+3
|\ |\
| o 4: t1+2 | o 4: t1+2
| | | |
| o 3: t1+1 | o 3: t1+1
| | | |
o | 2: t1+1 o | 2: t1+1
|/ |/
o 1: t1+0 o 1: t1+0
| |
o 0: null+1 o 0: null+1
One ancestor tag: more recent wins: One ancestor tag: closest wins:
$ hg tag -r 2 -m t2 -d '7 0' t2 $ hg tag -r 2 -m t2 -d '7 0' t2
$ hg log -G --template '{rev}: {latesttag}+{latesttagdistance}\n' $ hg log -G --template '{rev}: {latesttag}+{latesttagdistance}\n'
@ 7: t2+3 @ 7: t2+3
| |
o 6: t2+2 o 6: t2+2
| |
o 5: t2+1 o 5: t2+1
|\ |\
| o 4: t1+2 | o 4: t1+2
| | | |
| o 3: t1+1 | o 3: t1+1
| | | |
o | 2: t2+0 o | 2: t2+0
|/ |/
o 1: t1+0 o 1: t1+0
| |
o 0: null+1 o 0: null+1
Two branch tags: more recent wins: Two branch tags: more recent wins if same number of changes:
$ hg tag -r 3 -m t3 -d '8 0' t3 $ hg tag -r 3 -m t3 -d '8 0' t3
$ hg log -G --template '{rev}: {latesttag}+{latesttagdistance}\n' $ hg log -G --template '{rev}: {latesttag}+{latesttagdistance}\n'
@ 8: t3+5 @ 8: t3+5
| |
o 7: t3+4 o 7: t3+4
| |
o 6: t3+3 o 6: t3+3
| |
o 5: t3+2 o 5: t3+2
|\ |\
| o 4: t3+1 | o 4: t3+1
| | | |
| o 3: t3+0 | o 3: t3+0
| | | |
o | 2: t2+0 o | 2: t2+0
|/ |/
o 1: t1+0 o 1: t1+0
| |
o 0: null+1 o 0: null+1
Two branch tags: fewest changes wins:
$ hg tag -r 4 -m t4 -d '4 0' t4 # older than t2, but should not matter
$ hg log -G --template "{rev}: {latesttag % '{tag}+{distance},{changes} '}\n"
@ 9: t4+5,6
|
o 8: t4+4,5
|
o 7: t4+3,4
|
o 6: t4+2,3
|
o 5: t4+1,2
|\
| o 4: t4+0,0
| |
| o 3: t3+0,0
| |
o | 2: t2+0,0
|/
o 1: t1+0,0
|
o 0: null+1,1
Merged tag overrides: Merged tag overrides:
$ hg tag -r 5 -m t5 -d '9 0' t5 $ hg tag -r 5 -m t5 -d '9 0' t5
$ hg tag -r 3 -m at3 -d '10 0' at3 $ hg tag -r 3 -m at3 -d '10 0' at3
$ hg log -G --template '{rev}: {latesttag}+{latesttagdistance}\n' $ hg log -G --template '{rev}: {latesttag}+{latesttagdistance}\n'
@ 10: t5+5 @ 11: t5+6
|
o 10: t5+5
| |
o 9: t5+4 o 9: t5+4
| |
o 8: t5+3 o 8: t5+3
| |
o 7: t5+2 o 7: t5+2
| |
o 6: t5+1 o 6: t5+1
| |
o 5: t5+0 o 5: t5+0
|\ |\
| o 4: at3:t3+1 | o 4: t4+0
| | | |
| o 3: at3:t3+0 | o 3: at3:t3+0
| | | |
o | 2: t2+0 o | 2: t2+0
|/ |/
o 1: t1+0 o 1: t1+0
| |
o 0: null+1 o 0: null+1
$ hg log -G --template "{rev}: {latesttag % '{tag}+{distance},{changes} '}\n" $ hg log -G --template "{rev}: {latesttag % '{tag}+{distance},{changes} '}\n"
@ 10: t5+5,5 @ 11: t5+6,6
|
o 10: t5+5,5
| |
o 9: t5+4,4 o 9: t5+4,4
| |
o 8: t5+3,3 o 8: t5+3,3
| |
o 7: t5+2,2 o 7: t5+2,2
| |
o 6: t5+1,1 o 6: t5+1,1
| |
o 5: t5+0,0 o 5: t5+0,0
|\ |\
| o 4: at3+1,1 t3+1,1 | o 4: t4+0,0
| | | |
| o 3: at3+0,0 t3+0,0 | o 3: at3+0,0 t3+0,0
| | | |
o | 2: t2+0,0 o | 2: t2+0,0
|/ |/
o 1: t1+0,0 o 1: t1+0,0
| |
o 0: null+1,1 o 0: null+1,1
$ hg log -G --template "{rev}: {latesttag('re:^t[13]$') % '{tag}, C: {changes}, D: {distance}'}\n" $ hg log -G --template "{rev}: {latesttag('re:^t[13]$') % '{tag}, C: {changes}, D: {distance}'}\n"
@ 10: t3, C: 8, D: 7 @ 11: t3, C: 9, D: 8
|
o 10: t3, C: 8, D: 7
| |
o 9: t3, C: 7, D: 6 o 9: t3, C: 7, D: 6
| |
o 8: t3, C: 6, D: 5 o 8: t3, C: 6, D: 5
| |
o 7: t3, C: 5, D: 4 o 7: t3, C: 5, D: 4
| |
o 6: t3, C: 4, D: 3 o 6: t3, C: 4, D: 3
$ HOME=`pwd`/home; export HOME $ HOME=`pwd`/home; export HOME
$ cat > latesttag/.hg/hgrc <<EOF $ cat > latesttag/.hg/hgrc <<EOF
> [ui] > [ui]
> style = ~/styles/teststyle > style = ~/styles/teststyle
> EOF > EOF
$ hg -R latesttag tip $ hg -R latesttag tip
test 10:9b4a630e5f5f test 11:97e5943b523a
Test recursive showlist template (issue1989): Test recursive showlist template (issue1989):
$ cat > style1989 <<EOF $ cat > style1989 <<EOF
> changeset = '{file_mods}{manifest}{extras}' > changeset = '{file_mods}{manifest}{extras}'
> file_mod = 'M|{author|person}\n' > file_mod = 'M|{author|person}\n'
> manifest = '{rev},{author}\n' > manifest = '{rev},{author}\n'
> extra = '{key}: {author}\n' > extra = '{key}: {author}\n'
> EOF > EOF
$ hg -R latesttag log -r tip --style=style1989 $ hg -R latesttag log -r tip --style=style1989
M|test M|test
10,test 11,test
branch: test branch: test
Test new-style inline templating: Test new-style inline templating:
$ hg log -R latesttag -r tip --template 'modified files: {file_mods % " {file}\n"}\n' $ hg log -R latesttag -r tip --template 'modified files: {file_mods % " {file}\n"}\n'
modified files: .hgtags modified files: .hgtags
hg: parse error: sub got an invalid replacement: \1 hg: parse error: sub got an invalid replacement: \1
[255] [255]
Test the strip function with chars specified: Test the strip function with chars specified:
$ hg log -R latesttag --template '{desc}\n' $ hg log -R latesttag --template '{desc}\n'
at3 at3
t5 t5
t4
t3 t3
t2 t2
t1 t1
merge merge
h2e h2e
h2d h2d
h1c h1c
b b
a a
$ hg log -R latesttag --template '{strip(desc, "te")}\n' $ hg log -R latesttag --template '{strip(desc, "te")}\n'
at3 at3
5 5
4
3 3
2 2
1 1
merg merg
h2 h2
h2d h2d
h1c h1c
b b
a a
Test date format: Test date format:
$ hg log -R latesttag --template 'date: {date(date, "%y %m %d %S %z")}\n' $ hg log -R latesttag --template 'date: {date(date, "%y %m %d %S %z")}\n'
date: 70 01 01 10 +0000 date: 70 01 01 10 +0000
date: 70 01 01 09 +0000 date: 70 01 01 09 +0000
date: 70 01 01 04 +0000
date: 70 01 01 08 +0000 date: 70 01 01 08 +0000
date: 70 01 01 07 +0000 date: 70 01 01 07 +0000
date: 70 01 01 06 +0000 date: 70 01 01 06 +0000
date: 70 01 01 05 +0100 date: 70 01 01 05 +0100
date: 70 01 01 04 +0000 date: 70 01 01 04 +0000
date: 70 01 01 03 +0000 date: 70 01 01 03 +0000
date: 70 01 01 02 +0000 date: 70 01 01 02 +0000
date: 70 01 01 01 +0000 date: 70 01 01 01 +0000