This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6369

templatekw: make {file_*} compare to both merge parents (issue4292)
ClosedPublic

Authored by martinvonz on May 11 2019, 3:18 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG0c72eddb4be5: templatekw: make {file_*} compare to both merge parents (issue4292)
Summary

This redefines the {file_adds}, {file_dels}, {file_mods} template
keywords by getting the lists from the recently introduced context
methods instead of getting them from status compared to p1. As
mentioned before, these are better defined on merge commits. The total
number of files from the three lists now always add up to the number
of files in {files}.

I timed this command:

hg log -r 4.0::5.0 -T '{rev}\n {file_mods}\n {file_adds}\n {file_dels}\n'

It went from 7.6s to 5.6s with this patch. So it's actually faster
than before.

Note that the "files:" field in the bazaar test log output was using
"{file_mods}" (not "{files}" as one might think based on the label).

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.May 11 2019, 3:18 AM
yuja added a subscriber: yuja.May 11 2019, 11:07 PM
This redefines the template keywords by getting the lists from the
recently introduced context methods instead of getting them from
status compared to p1. A mentioned before, these are better defined on
merge commits. The total number of files from the three lists now
always add up to the number of files in {files}.

So this is a BC, and makes log -T status output differ from
status --change REV. Just pointed out.

I'm 0 on this change. I feel the new definition makes more sense, but
the old one seems also okay and is simpler.

martinvonz added a comment.May 12 2019, 1:15 PM

*From: *yuja (Yuya Nishihara) <phabricator@mercurial-scm.org>
*Date: *Sat, May 11, 2019, 20:17
*To: * <mercurial-devel@mercurial-scm.org>

yuja added a comment.

>   This redefines the template keywords by getting the lists from the
>   recently introduced context methods instead of getting them from
>   status compared to p1. A mentioned before, these are better defined

on

>   merge commits. The total number of files from the three lists now
>   always add up to the number of files in {files}.
So this is a BC, and makes `log -T status` output differ from
`status --change REV`. Just pointed out.

Depends on whether you consider the bug valid, right? We don't mark bug
fixes as BC. But I agree with still listing this in the "backwards
compatibility" section of the release notes.

I'm 0 on this change. I feel the new definition makes more sense, but
the old one seems also okay and is simpler.

I agree, but it also seems unlikely that anyone would depend on the old
behavior.

BTW, and this is also related to your comments on the previous patch,
{files} is probably already inconsistent in the working copy when merging
and after committing the merge. I don't think we should change that. But I
think it means that the number of modified, added, and removed files add up
to the files in that keyword in the working copy, even though they're all
compared to just p1. It's just another example of the working copy status
being weird before committing a merge.

REPOSITORY

rHG Mercurial

REVISION DETAIL

https://phab.mercurial-scm.org/D6369

To: martinvonz, hg-reviewers
Cc: yuja, mercurial-devel

Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

martinvonz updated this revision to Diff 15082.May 14 2019, 6:58 PM
martinvonz updated this revision to Diff 15153.May 16 2019, 1:41 PM
martinvonz retitled this revision from templatekw: get file_{adds,mods,dels} directly from context (issue4292) to templatekw: make {file_*} compare to both merge parents (issue4292).May 16 2019, 4:53 PM
martinvonz edited the summary of this revision. (Show Details)
martinvonz updated this revision to Diff 15266.May 26 2019, 1:02 AM
yuja added a comment.May 26 2019, 7:31 PM

Test failures:

--- /home/yuya/work/hghacks/mercurial-review/tests/test-convert-bzr-merges.t
+++ /home/yuya/work/hghacks/mercurial-review/tests/test-convert-bzr-merges.t.err
@@ -59,7 +59,7 @@
   $ glog -R source-hg
   o    5@source "(octopus merge fixup)" files+: [], files-: [], files: [renamed]
   |\
-  | o    4@source "Merged branches" files+: [file-branch1 file-branch2 renamed], files-: [rename_me], files: [file]
+  | o    4@source "Merged branches" files+: [file-branch2 renamed], files-: [rename_me], files: []
   | |\
   o---+  3@source-branch2 "Added brach2 file" files+: [file-branch2 renamed], files-: [rename_me], files: []
    / /
@@ -154,7 +154,7 @@
   $ glog -R hg2hg
   @    5@source "(octopus merge fixup)" files+: [], files-: [], files: []
   |\
-  | o    4@source "Merged branches" files+: [file-branch1 file-branch2 renamed], files-: [rename_me], files: [file]
+  | o    4@source "Merged branches" files+: [file-branch2 renamed], files-: [rename_me], files: []
   | |\
   o---+  3@source-branch2 "Added brach2 file" files+: [file-branch2 renamed], files-: [rename_me], files: []
    / /

ERROR: test-convert-bzr-merges.t output changed
!......................................ss..........
--- /home/yuya/work/hghacks/mercurial-review/tests/test-convert-bzr.t
+++ /home/yuya/work/hghacks/mercurial-review/tests/test-convert-bzr.t.err
@@ -147,7 +147,7 @@
   1 Editing b
   0 Merged improve branch
   $ glog -R source-hg
-  o    3@source "Merged improve branch" files+: [], files-: [], files: [b]
+  o    3@source "Merged improve branch" files+: [], files-: [], files: []
   |\
   | o  2@source-improve "Editing b" files+: [], files-: [], files: [b]
   | |

ERROR: test-convert-bzr.t output changed
martinvonz added a comment.May 28 2019, 12:42 PM
In D6369#93709, @yuja wrote:

Test failures:

Thanks for letting me know. I've installed bzr now and I've fixed the test cases (the files: field in the output was from file_mods).

martinvonz edited the summary of this revision. (Show Details)May 28 2019, 12:43 PM
martinvonz added a comment.May 31 2019, 1:04 PM

Yuya, are you okay with this patch? My other patches don't depend on it, so I don't care much from that perspective, but it really feels like a bug fix to me.

Closed by commit rHG0c72eddb4be5: templatekw: make {file_*} compare to both merge parents (issue4292) (authored by martinvonz). · Explain WhyJun 1 2019, 7:36 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/templatekw.py (17 lines)
Mrelnotes/next (14 lines)
Mtests/test-template-keywords.t (4 lines)

Diff 15266

mercurial/templatekw.py

revcache = context.resource(mapping, 'revcache') revcache = context.resource(mapping, 'revcache')
if 'filestatusmap' not in revcache or revcache['filestatusall'] < listall: if 'filestatusmap' not in revcache or revcache['filestatusall'] < listall:
stat = _getfilestatus(context, mapping, listall=listall) stat = _getfilestatus(context, mapping, listall=listall)
revcache['filestatusmap'] = statmap = {} revcache['filestatusmap'] = statmap = {}
for char, files in zip(pycompat.iterbytestr('MAR!?IC'), stat): for char, files in zip(pycompat.iterbytestr('MAR!?IC'), stat):
statmap.update((f, char) for f in files) statmap.update((f, char) for f in files)
return revcache['filestatusmap'] # {path: statchar} return revcache['filestatusmap'] # {path: statchar}
def _showfilesbystat(context, mapping, name, index):
stat = _getfilestatus(context, mapping)
files = stat[index]
return templateutil.compatfileslist(context, mapping, name, files)
@templatekeyword('file_copies', @templatekeyword('file_copies',
requires={'repo', 'ctx', 'cache', 'revcache'}) requires={'repo', 'ctx', 'cache', 'revcache'})
def showfilecopies(context, mapping): def showfilecopies(context, mapping):
"""List of strings. Files copied in this changeset with """List of strings. Files copied in this changeset with
their sources. their sources.
""" """
repo = context.resource(mapping, 'repo') repo = context.resource(mapping, 'repo')
ctx = context.resource(mapping, 'ctx') ctx = context.resource(mapping, 'ctx')
""" """
copies = context.resource(mapping, 'revcache').get('copies') or [] copies = context.resource(mapping, 'revcache').get('copies') or []
return templateutil.compatfilecopiesdict(context, mapping, 'file_copy', return templateutil.compatfilecopiesdict(context, mapping, 'file_copy',
copies) copies)
@templatekeyword('file_adds', requires={'ctx', 'revcache'}) @templatekeyword('file_adds', requires={'ctx', 'revcache'})
def showfileadds(context, mapping): def showfileadds(context, mapping):
"""List of strings. Files added by this changeset.""" """List of strings. Files added by this changeset."""
return _showfilesbystat(context, mapping, 'file_add', 1) ctx = context.resource(mapping, 'ctx')
return templateutil.compatfileslist(context, mapping, 'file_add',
ctx.filesadded())
@templatekeyword('file_dels', requires={'ctx', 'revcache'}) @templatekeyword('file_dels', requires={'ctx', 'revcache'})
def showfiledels(context, mapping): def showfiledels(context, mapping):
"""List of strings. Files removed by this changeset.""" """List of strings. Files removed by this changeset."""
return _showfilesbystat(context, mapping, 'file_del', 2) ctx = context.resource(mapping, 'ctx')
return templateutil.compatfileslist(context, mapping, 'file_del',
ctx.filesremoved())
@templatekeyword('file_mods', requires={'ctx', 'revcache'}) @templatekeyword('file_mods', requires={'ctx', 'revcache'})
def showfilemods(context, mapping): def showfilemods(context, mapping):
"""List of strings. Files modified by this changeset.""" """List of strings. Files modified by this changeset."""
return _showfilesbystat(context, mapping, 'file_mod', 0) ctx = context.resource(mapping, 'ctx')
return templateutil.compatfileslist(context, mapping, 'file_mod',
ctx.filesmodified())
@templatekeyword('files', requires={'ctx'}) @templatekeyword('files', requires={'ctx'})
def showfiles(context, mapping): def showfiles(context, mapping):
"""List of strings. All files modified, added, or removed by this """List of strings. All files modified, added, or removed by this
changeset. changeset.
""" """
ctx = context.resource(mapping, 'ctx') ctx = context.resource(mapping, 'ctx')
return templateutil.compatfileslist(context, mapping, 'file', ctx.files()) return templateutil.compatfileslist(context, mapping, 'file', ctx.files())

relnotes/next

topological sorting. This is especially useful for aliases since it topological sorting. This is especially useful for aliases since it
lets the alias accept an `-r` option while still using topological lets the alias accept an `-r` option while still using topological
sorting with or without the `-r` (unlike if you use the `sort(..., sorting with or without the `-r` (unlike if you use the `sort(...,
topo)` revset). topo)` revset).
== Bug Fixes == == Bug Fixes ==
* issue4292: "hg log and {files} {file_adds} {file_mods} {file_dels}
in template show wrong files on merged revision". See details in
"Backwards Compatibility Changes".
== Backwards Compatibility Changes == == Backwards Compatibility Changes ==
* Removed (experimental) support for log graph lines mixing * Removed (experimental) support for log graph lines mixing
parent/grandparent styles. Setting parent/grandparent styles. Setting
e.g. `experimental.graphstyle.parent = !` and e.g. `experimental.graphstyle.parent = !` and
`experimental.graphstyle.grandparent = 3.` would use `!` for the `experimental.graphstyle.grandparent = 3.` would use `!` for the
first three lines of the graph and then `.`. This is no longer first three lines of the graph and then `.`. This is no longer
supported. supported.
* If `ui.origbackuppath` had been (incorrectly) configured to point * If `ui.origbackuppath` had been (incorrectly) configured to point
to a file, we will now replace that file by a directory and put to a file, we will now replace that file by a directory and put
backups in that directory. This is similar to how we would backups in that directory. This is similar to how we would
previously replace files *in* the configured directory by previously replace files *in* the configured directory by
subdirectories. subdirectories.
* Template keyword `{file_mods}`, `{file_adds}`, and `{file_dels}`
have changed behavior on merge commits. They used to be relative to
the first parent, but they now consider both parents. `{file_adds}`
shows files that exists in the commit but did not exist in either
parent. `{file_dels}` shows files that do not exist in the commit
but existed in either parent. `{file_mods}` show the remaining
files from `{files}` that were not in the other two
sets.
== Internal API Changes == == Internal API Changes ==
* Matchers are no longer iterable. Use `match.files()` instead. * Matchers are no longer iterable. Use `match.files()` instead.
* `match.visitdir()` and `match.visitchildrenset()` now expect the * `match.visitdir()` and `match.visitchildrenset()` now expect the
empty string instead of '.' to indicate the root directory. empty string instead of '.' to indicate the root directory.
* `util.dirs()` and `util.finddirs()` now include an entry for the * `util.dirs()` and `util.finddirs()` now include an entry for the
root directory (empty string). root directory (empty string).

tests/test-template-keywords.t

$ touch c $ touch c
$ hg add c $ hg add c
$ hg ci -qm 'add file' $ hg ci -qm 'add file'
$ hg merge 10 -q $ hg merge 10 -q
$ hg ci -m 'merge' $ hg ci -m 'merge'
$ hg log -l1 -T '{files}\n' $ hg log -l1 -T '{files}\n'
a fourth a fourth
$ hg log -l1 -T '{file_mods}\n' $ hg log -l1 -T '{file_mods}\n'
third
$ hg log -l1 -T '{file_adds}\n' $ hg log -l1 -T '{file_adds}\n'
b fifth
$ hg log -l1 -T '{file_dels}\n' $ hg log -l1 -T '{file_dels}\n'
a fourth a fourth
Test file copies dict: Test file copies dict:
$ hg log -r8 -T '{join(file_copies, " ")}\n' $ hg log -r8 -T '{join(file_copies, " ")}\n'
fourth (second) fourth (second)
$ hg log -r8 -T '{file_copies % "{name} <- {source}\n"}' $ hg log -r8 -T '{file_copies % "{name} <- {source}\n"}'