This is an archive of the discontinued Mercurial Phabricator instance.

phabricator: handle local:commits time being string or int
ClosedPublic

Authored by Kwan on Jul 16 2019, 2:23 PM.

Details

Summary

When setting local:commits arcanist has different behaviour depending on
whether the repo is git or hg. With hg it sets the time as a number, since it
calls PHP's strtotime on the value, but with git it sets it as a string.
Normally this wouldn't be an issue since phabread wouldn't be interacting with
Phabricator Revisions for git repos, but Mozilla has a secondary workflow for
git users that uses the git-cinnabar tool to interact with their hg repos. When
a git-cinnabar user uses the moz-phab tool to submit patches for mozilla-central
it makes use of Mozilla's fork of arcanist, which works with their local git
version of m-c, and thus sets the local:commit time as a string, and then
translates the commit hashes.

Currently when encountering such DREVS phabread dies with "TypeError: %d format:
a number is required, not str".

phabsend also used to set it as a string but wouldn't have encountered the
issue with its own DREVs since it would read hg:meta first.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Kwan created this revision.Jul 16 2019, 2:23 PM

I'm not sure if the test is actually needed, so feel free to drop it/ask me to.

Kwan added inline comments.Jul 16 2019, 2:35 PM
hgext/phabricator.py
953

I went for int() over switching to %s because if time is ever a float we'd end up with # Date 1562019844.0 0 in the patch header.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.