1485878 - Make pushlog compatible with Mercurial 4.7

Status: RESOLVED FIXED

(Developer Services :: Mercurial: Pushlog, enhancement)

Description

[Connor Sheehan [:sheehan]]

Attachment: test output for `./run-tests hgext/pushlog --with-hg=venv/mercurials/4.7/bin/hg`

Multiple breakages here, mostly related to templating. Logs are attached separately.

Comment 1

[Connor Sheehan [:sheehan]]

Attachment: pushlog: remove Mercurial 1.0 compatibility code (Bug 1485878) r?gps

"these three functions are in webutil in newer hg, but not in hg 1.0".
Yes, you read that correctly.... 1.0. I think it might be safe
to remove this now.

Comment 2

[Connor Sheehan [:sheehan]]

Attachment: pushlog: rename `id` variables to `pushid` (Bug 1485878) r?gps

`id` is the name of a built-in function in Python. Creating a
variable of this name overrides the existing `id` function and
is considered a bad practice. We already reference this field as
`pushid` in other places (including the core pushlog extension),
so this commit changes the feed extension to do the same.

Comment 3

[Connor Sheehan [:sheehan]]

Attachment: pushlog: use new `dateutil` module under Mercurial 4.7 (Bug 1485878) r?gps

4.7 moved a few functions from `util` to `utils.dateutil`. Use them
from their new module if it is available.

Comment 4

[Connor Sheehan [:sheehan]]

Attachment: pushlog: use `next(iter)` instead of `iter.next()` (Bug 1485878) r?gps

Python 3 removed the instance `next` method, and the builtin
function is the recommended way to do things.

Comment 5

[Connor Sheehan [:sheehan]]

Attachment: pushlog: cat error logs in `test-hgweb-user-queries.t` (Bug 1485878) r?gps

When the test fails, this will spit out the offending internal
server failure for debugging.

Comment 6

[Connor Sheehan [:sheehan]]

Attachment: pushlog: define custom Pushlog template keywords using registrar API (Bug 1485878) r?gps

This is the recommended way to define template keywords. Since
we are only supporting the current and future versions of
Mercurial for the server-side extensions, let's switch to the
new API.

Comment 7

[Connor Sheehan [:sheehan]]

Attachment: pushlog: move `isotime` into standalone anonymous function (Bug 1485878) r?gps

This function is pure and has no requirements to access the
outer function's scope. Moving it to the top level of this
file will allow us to use it elsewhere in later commits.

Comment 8

[Connor Sheehan [:sheehan]]

Attachment: pushlog: create entries in the `pushlog_feed` webcommand with a generator (Bug 1485878) r?gps

This commit changes the `entries` field in the template mapping
to be generated using a `mappinggenerator`. Setting the entries
field to a list as in the current implementation breaks under
Mercurial 4.7, so this change makes us compatible in both versions.

In the new implementation, we use a generator expression instead
of a for loop to iterate through the query entries. To determine
whether a changeset is hidden or not, we use the `isrevsymbol`
function instead of catching a `FilteredRepoLookupError`. We
also use `revsingle` to get the context variable instead of
using `repo[node]`.

In making this change, I noticed that the `test-obsolescence-pushlog.t`
test started to fail. Upon further review, I realized that the test
is expecting the obsolete changesets in the test repo to not appear
in the ATOM output, but in the previous implementation they were
showing up anyways. This is because in the previous implementation
we would check for a hidden changeset, but then `pass` instead of
`continue` in the except block. I have removed the bad output and
the behaviour now meets the specification in the test.

Comment 9

[Connor Sheehan [:sheehan]]

Attachment: pushlog: change `changenav` local to a generator (Bug 1485878) r?gps

This commit changes the `changenav` function into a generator
for use in the template layer under Mercurial 4.7.

Comment 10

[Connor Sheehan [:sheehan]]

Attachment: pushlog: switch from `dict` function to a dict literal (Bug 1485878) r?gps

This is better for Python 3 compatibility, readability (subjective)
and a *tiny* performance boost.

Comment 11

[Connor Sheehan [:sheehan]]

Attachment: pushlog: move pushlog_changenav into standalone function (Bug 1485878) r?gps

This commit moves `pushlog_changenav` into it's own standalone
function.

Comment 12

[Connor Sheehan [:sheehan]]

Attachment: pushlog: implement pushlog's `changelist` as a generator (Bug 1485878) r?gps

This commit makes a variety of changes to the `changelist` function
defined within the `pushloghtml` webcommand.

First, the function is no longer a closure defined within the
`pushloghtml` webcommand, instead becoming a function at the
top-level of the module. Next, the limit variable is changed to
a `tiponly` flag, as the  value was only ever set to 0 (no limit)
or 1 (single entry returned).

This commit then makes changes to the way `changelist` yields entries
to the template layer. Formerly, we would process every entry in
the pushlog query and aggregate them in a list, then use a slice of
that list and yield entries. Thus, we were processing every push in
a given query and then only returning a single entry (in the worst
case). This commit instead makes the generator yield entries as they
are ready, avoiding the extra unnecessary work.

In the previous implementation, we would iterate over every push in
the query and create a template mapping for that entry. Along the
way, we would make several logical decisions that influence which
fields are populated and how. The first entry for each pushid has
a "push" field with the user and time of the push. If the push was
a merge, we also add a "mergerollup" field which displays the number
of changesets added by this merge to the first entry for that push.
To populate the number, we would create a reference to the "push"
mapping for that entry and increment by 1 for each successive entry
which had the same pushid.

The new implementation separates the logic into easier to understand
chunks. `pushlog_changelist` now contains the logic to iterate over
different pushes and make decisions about how to display each set
of entries for a push. We iterate over all the entries and aggregate
pushes into a queue until the current pushid changes, at which point
we use `handle_entries_for_push` to yield entries with the correct
information.

`handle_entries_for_push` yields entries given the gathered context
for a single push. The "mergerollup" count field is populated as the
length of the queue of entries for that push. We also determine whether
a push is a merge and whether to hide the merge fields in this section
of code.

`create_entry` takes the contexts gathered in `handle_entries_for_push`
and returns a template mapping for that entry, which is then yielded
successively down the line to the template layer.

Comment 13

[Connor Sheehan [:sheehan]]

Forgot to assign myself to this bug.

Comment 14

[Gregory Szorc [:gps]]

Comment on attachment 9010387 [details]
pushlog: remove Mercurial 1.0 compatibility code (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 15

[Gregory Szorc [:gps]]

Comment on attachment 9010388 [details]
pushlog: rename `id` variables to `pushid` (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 16

[Gregory Szorc [:gps]]

Comment on attachment 9010389 [details]
pushlog: use new `dateutil` module under Mercurial 4.7 (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 17

[Gregory Szorc [:gps]]

Comment on attachment 9010390 [details]
pushlog: use `next(iter)` instead of `iter.next()` (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 18

[Gregory Szorc [:gps]]

Comment on attachment 9010391 [details]
pushlog: cat error logs in `test-hgweb-user-queries.t` (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 19

[Gregory Szorc [:gps]]

Comment on attachment 9010392 [details]
pushlog: define custom Pushlog template keywords using registrar API (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 20

[Gregory Szorc [:gps]]

Comment on attachment 9010393 [details]
pushlog: move `isotime` into standalone anonymous function (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 21

[Gregory Szorc [:gps]]

Comment on attachment 9010394 [details]
pushlog: create entries in the `pushlog_feed` webcommand with a generator (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 22

[Gregory Szorc [:gps]]

Comment on attachment 9010396 [details]
pushlog: change `changenav` local to a generator (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 23

[Gregory Szorc [:gps]]

Comment on attachment 9010397 [details]
pushlog: switch from `dict` function to a dict literal (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 24

[Gregory Szorc [:gps]]

Comment on attachment 9010398 [details]
pushlog: move pushlog_changenav into standalone function (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/5b7f1fe69a6f
pushlog: remove Mercurial 1.0 compatibility code r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/6e2d5b357805
pushlog: rename `id` variables to `pushid` r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/6603436926c8
pushlog: use new `dateutil` module under Mercurial 4.7 r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/08e0a1c9c73d
pushlog: use `next(iter)` instead of `iter.next()` r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/8f9302cd7d6c
pushlog: cat error logs in `test-hgweb-user-queries.t` r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/6724fad155d4
pushlog: define custom Pushlog template keywords using registrar API r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/b6adfac0a882
pushlog: move `isotime` into standalone anonymous function r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/591ad2f1402a
pushlog: create entries in the `pushlog_feed` webcommand with a generator r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/91a3cac03614
pushlog: change `changenav` local to a generator r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/700fb60d4c1d
pushlog: switch from `dict` function to a dict literal r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/58109e0c81cf
pushlog: move pushlog_changenav into standalone function r=gps

Comment 26

[Gregory Szorc [:gps]]

Comment on attachment 9010399 [details]
pushlog: implement pushlog's `changelist` as a generator (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/a746fe28ba4f
pushlog: implement pushlog's `changelist` as a generator r=gps

Comment 28

[Connor Sheehan [:sheehan]]

Attachment: pushlog: mark as compatible with Mercurial 4.7 (Bug 1485878) r?gps

The final failing test for pushlog was failing due to a dependency
on the `hgmo` extension. Now that `hgmo` is compatible with 4.7,
we can mark this extension compatible as well.

Comment 29

[Gregory Szorc [:gps]]

Comment on attachment 9011653 [details]
pushlog: mark as compatible with Mercurial 4.7 (Bug 1485878) r?gps

Gregory Szorc [:gps] has approved the revision.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/2e670681dbd9
pushlog: mark as compatible with Mercurial 4.7 r=gps