( )⚙ D8308 phabricator: record all local commits used to create a Differential revision

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8308

phabricator: record all local commits used to create a Differential revision
ClosedPublic

Authored by mharbison72 on Mar 20 2020, 5:24 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG0437959de6f5: phabricator: record all local commits used to create a Differential revision
Summary

Arcanist records all of the commits that it squashes into a single review, and
that info will be helpful when adding similar functionality. This info is used
when submitting an updated review, so that the extension can recalculate the old
diff and see if a new one is necessary, or if it is just a property update. It
also shows on the commits tab in the Revision Contents section.

When submitting in the usual 1:1 commit to review mode, the wire protocol is
unchanged.

The content of hg:meta is a bit odd, but such is the problem when folding
several commits. The choice for the parent node is obvious, but the node
value uses the tip commit because that seems more natural, and is used elsewhere
to look up the previous diff when updating. The rest of the attributes follow
from there.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

mharbison72 created this revision.Mar 20 2020, 5:24 PM
Alphare accepted this revision.Mar 25 2020, 2:18 PM
mharbison72 added a commit: rHG0437959de6f5: phabricator: record all local commits used to create a Differential revision.Apr 1 2020, 11:23 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG0437959de6f5: phabricator: record all local commits used to create a Differential revision (authored by mharbison72). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/phabricator.py (55 lines)

Diff 20848

hgext/phabricator.py

if basectx != ctx: if basectx != ctx:
msg = _(b'cannot create diff for %s::%s') % (basectx, ctx) msg = _(b'cannot create diff for %s::%s') % (basectx, ctx)
else: else:
msg = _(b'cannot create diff for %s') % ctx msg = _(b'cannot create diff for %s') % ctx
raise error.Abort(msg) raise error.Abort(msg)
return diff return diff
def writediffproperties(ctx, diff): def writediffproperties(ctxs, diff):
"""write metadata to diff so patches could be applied losslessly""" """write metadata to diff so patches could be applied losslessly
``ctxs`` is the list of commits that created the diff, in ascending order.
The list is generally a single commit, but may be several when using
``phabsend --fold``.
"""
# creatediff returns with a diffid but query returns with an id # creatediff returns with a diffid but query returns with an id
diffid = diff.get(b'diffid', diff.get(b'id')) diffid = diff.get(b'diffid', diff.get(b'id'))
basectx = ctxs[0]
tipctx = ctxs[-1]
params = { params = {
b'diff_id': diffid, b'diff_id': diffid,
b'name': b'hg:meta', b'name': b'hg:meta',
b'data': templatefilters.json( b'data': templatefilters.json(
{ {
b'user': ctx.user(), b'user': tipctx.user(),
b'date': b'%d %d' % ctx.date(), b'date': b'%d %d' % tipctx.date(),
b'branch': ctx.branch(), b'branch': tipctx.branch(),
b'node': ctx.hex(), b'node': tipctx.hex(),
b'parent': ctx.p1().hex(), b'parent': basectx.p1().hex(),
} }
), ),
} }
callconduit(ctx.repo().ui, b'differential.setdiffproperty', params) callconduit(basectx.repo().ui, b'differential.setdiffproperty', params)
params = { commits = {}
b'diff_id': diffid, for ctx in ctxs:
b'name': b'local:commits', commits[ctx.hex()] = {
b'data': templatefilters.json(
{
ctx.hex(): {
b'author': stringutil.person(ctx.user()), b'author': stringutil.person(ctx.user()),
b'authorEmail': stringutil.email(ctx.user()), b'authorEmail': stringutil.email(ctx.user()),
b'time': int(ctx.date()[0]), b'time': int(ctx.date()[0]),
b'commit': ctx.hex(), b'commit': ctx.hex(),
b'parents': [ctx.p1().hex()], b'parents': [ctx.p1().hex()],
b'branch': ctx.branch(), b'branch': ctx.branch(),
},
} }
), params = {
b'diff_id': diffid,
b'name': b'local:commits',
b'data': templatefilters.json(commits),
} }
callconduit(ctx.repo().ui, b'differential.setdiffproperty', params) callconduit(basectx.repo().ui, b'differential.setdiffproperty', params)
def createdifferentialrevision( def createdifferentialrevision(
ctx, ctx,
revid=None, revid=None,
parentrevphid=None, parentrevphid=None,
oldnode=None, oldnode=None,
olddiff=None, olddiff=None,
if comment: if comment:
transactions.append({b'type': b'comment', b'value': comment}) transactions.append({b'type': b'comment', b'value': comment})
else: else:
# Even if we don't need to upload a new diff because the patch content # Even if we don't need to upload a new diff because the patch content
# does not change. We might still need to update its metadata so # does not change. We might still need to update its metadata so
# pushers could know the correct node metadata. # pushers could know the correct node metadata.
assert olddiff assert olddiff
diff = olddiff diff = olddiff
writediffproperties(ctx, diff) writediffproperties([ctx], diff)
# Set the parent Revision every time, so commit re-ordering is picked-up # Set the parent Revision every time, so commit re-ordering is picked-up
if parentrevphid: if parentrevphid:
transactions.append( transactions.append(
{b'type': b'parents.set', b'value': [parentrevphid]} {b'type': b'parents.set', b'value': [parentrevphid]}
) )
if actions: if actions:
newnode = new.commit() newnode = new.commit()
mapping[old.node()] = [newnode] mapping[old.node()] = [newnode]
# Update diff property # Update diff property
# If it fails just warn and keep going, otherwise the DREV # If it fails just warn and keep going, otherwise the DREV
# associations will be lost # associations will be lost
try: try:
writediffproperties(unfi[newnode], diffmap[old.node()]) writediffproperties(
[unfi[newnode]], diffmap[old.node()]
)
except util.urlerr.urlerror: except util.urlerr.urlerror:
ui.warnnoi18n( ui.warnnoi18n(
b'Failed to update metadata for D%d\n' % drevid b'Failed to update metadata for D%d\n' % drevid
) )
# Remove local tags since it's no longer necessary # Remove local tags since it's no longer necessary
tagname = b'D%d' % drevid tagname = b'D%d' % drevid
if tagname in repo.tags(): if tagname in repo.tags():
tags.tag( tags.tag(