1286353 - Add files to Mercurial json-rev

Status: RESOLVED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Dan Minor [:dminor]]

The json-info url (e.g. https://hg.mozilla.org/mozilla-central/json-info/adea33038173 )contains a list of files affected by a revision, but this is not present in json-rev, and json-info is slated to be removed. We should add this info to json-rev.

Comment 1

[Dan Minor [:dminor]]

Attachment: hgserver: Add "files" to json-rev (bug 1286353);

Review commit: https://reviewboard.mozilla.org/r/64054/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64054/

Comment 2

[Gregory Szorc [:gps]]

Comment on attachment 8770622 [details]
hgserver: Add "files" to json-rev (bug 1286353);

https://reviewboard.mozilla.org/r/64054/#review61064

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/ca95dae43fc5
hgserver: Add "files" to json-rev ; r=gps

Comment 4

[Gregory Szorc [:gps]]

I had to back this out. Theorized reason in the commit message.

https://hg.mozilla.org/hgcustom/version-control-tools/rev/b09896b07aa4

Comment 5

[Dan Minor [:dminor]]

Attachment: hgserver: Add "files" to json-rev (bug 1286353);

This adds an "affectedfiles" to the changeset metadata and then renders them
as part of the changeset json template. The previous attempt called this key
"files" which may have caused rendering issues on hgweb. We do need to set
something in the metadata, otherwise, at least in our test cases, the
resulting json will always have an empty files section.

Review commit: https://reviewboard.mozilla.org/r/64232/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64232/

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8770924 [details]
hgserver: Add "files" to json-rev (bug 1286353);

https://reviewboard.mozilla.org/r/64232/#review62098

::: hgext/hgmo/__init__.py:129
(Diff revision 1)
>          d['reviewers'].append({
>              'name': reviewer,
>              'revset': 'reviewer(%s)' % reviewer,
>          })
>  
> +    d['affectedfiles'] = ctx.files()

I don't think you need this. According to my reading of the Mercurial source, the "files" entry is already exposed to the template - it is just a list of dicts instead of a list of strings.

::: hgtemplates/json/map:52
(Diff revision 1)
>    "date": {date|json},
>    "desc": {desc|json},
>    "backedoutby": {if(backedoutbynode, backedoutbynode|json, ""|json)},
>    "branch": {if(branch, branch%changesetbranch, "default"|json)},
>    "bookmarks": [{join(changesetbookmark, ", ")}],
> +  "files": {affectedfiles|json},

Try something like:

changesetfile = '{"file": {file|json}}'
...

"files": [{join(file%changesetfile, ", ")}],

I imagine Mercurial will add the "files" key at some point. When they do, my guess is they'll make it a list of dicts. So we should aim to be compatible.

Perhaps it is better to just make the key something else like "mozfiles" if we're concerned about forward compatibility (which quite frankly I am).

Comment 7

[Dan Minor [:dminor]]

https://reviewboard.mozilla.org/r/64232/#review62098

> Try something like:
> 
> changesetfile = '{"file": {file|json}}'
> ...
> 
> "files": [{join(file%changesetfile, ", ")}],
> 
> I imagine Mercurial will add the "files" key at some point. When they do, my guess is they'll make it a list of dicts. So we should aim to be compatible.
> 
> Perhaps it is better to just make the key something else like "mozfiles" if we're concerned about forward compatibility (which quite frankly I am).

I sunk a couple of hours trying this out this morning, but other than eventually finding that it needs to be:

changesetfile = '\{"file": {file|json}}'

I don't think I'm any closer to getting this working. At this point, I think I'll just cut my losses and scrape the html version of the rev.

Comment 8

[Julien Cristau [:jcristau]]

This was eventually added to mercurial in 5.3 (https://repo.mercurial-scm.org/hg/rev/6feaee05bac5).