Submitter | Jun Wu |
---|---|
Date | July 3, 2017, 3:10 a.m. |
Message ID | <edb97a6c5f1065227180.1499051419@x1c> |
Download | mbox | patch |
Permalink | /patch/21947/ |
State | Accepted |
Headers | show |
Comments
> + """ > + revs = list(revs) + opts.get('rev', []) > + revs = scmutil.revrange(repo, revs) > + I'd add something like this here: if not revs: raise error.Abort(_('must specify at least one changeset with -r')) On Sun, Jul 2, 2017 at 8:10 PM, Jun Wu <quark@fb.com> wrote: > # HG changeset patch > # User Jun Wu <quark@fb.com> > # Date 1499051289 25200 > # Sun Jul 02 20:08:09 2017 -0700 > # Node ID edb97a6c5f1065227180e41010076ad09697b408 > # Parent dde88a5ead698208226301a62a5f2aa2629d7839 > # Available At https://bitbucket.org/quark-zju/hg-draft > # hg pull https://bitbucket.org/quark-zju/hg-draft -r edb97a6c5f10 > phabricator: add phabsend command to send a stack > > The `phabsend` command is intended to provide `hg email`-like experience - > sending a stack, setup dependency information and do not amend existing > changesets. > > It uses differential.createrawdiff and differential.revision.edit Conduit > API to create or update a Differential Revision. > > Local tags like `D123` are written indicating certain changesets were sent > to Phabricator. The `phabsend` command will use obsstore and tags > information to decide whether to update or create Differential Revisions. > > diff --git a/contrib/phabricator.py b/contrib/phabricator.py > --- a/contrib/phabricator.py > +++ b/contrib/phabricator.py > @@ -7,4 +7,11 @@ > """simple Phabricator integration > > +This extension provides a ``phabsend`` command which sends a stack of > +changesets to Phabricator without amending commit messages. > + > +By default, Phabricator requires ``Test Plan`` which might prevent some > +changeset from being sent. The requirement could be disabled by changing > +``differential.require-test-plan-field`` config server side. > + > Config:: > > @@ -15,4 +22,8 @@ Config:: > # API token. Get it from https://$HOST/conduit/login/ > token = cli-xxxxxxxxxxxxxxxxxxxxxxxxxxxx > + > + # Repo callsign. If a repo has a URL https://$HOST/diffusion/FOO, then its > + # callsign is "FOO". > + callsign = FOO > """ > > @@ -20,9 +31,16 @@ from __future__ import absolute_import > > import json > +import re > > from mercurial.i18n import _ > from mercurial import ( > + encoding, > error, > + mdiff, > + obsolete, > + patch, > registrar, > + scmutil, > + tags, > url as urlmod, > util, > @@ -97,2 +115,161 @@ def debugcallconduit(ui, repo, name): > s = json.dumps(result, sort_keys=True, indent=2, separators=(',', ': ')) > ui.write('%s\n' % s) > + > +def getrepophid(repo): > + """given callsign, return repository PHID or None""" > + # developer config: phabricator.repophid > + repophid = repo.ui.config('phabricator', 'repophid') > + if repophid: > + return repophid > + callsign = repo.ui.config('phabricator', 'callsign') > + if not callsign: > + return None > + query = callconduit(repo, 'diffusion.repository.search', > + {'constraints': {'callsigns': [callsign]}}) > + if len(query[r'data']) == 0: > + return None > + repophid = encoding.strtolocal(query[r'data'][0][r'phid']) > + repo.ui.setconfig('phabricator', 'repophid', repophid) > + return repophid > + > +_differentialrevisionre = re.compile('\AD([1-9][0-9]*)\Z') > + > +def getmapping(ctx): > + """return (node, associated Differential Revision ID) or (None, None) > + > + Examines all precursors and their tags. Tags with format like "D1234" are > + considered a match and the node with that tag, and the number after "D" > + (ex. 1234) will be returned. > + """ > + unfi = ctx.repo().unfiltered() > + nodemap = unfi.changelog.nodemap > + for n in obsolete.allprecursors(unfi.obsstore, [ctx.node()]): > + if n in nodemap: > + for tag in unfi.nodetags(n): > + m = _differentialrevisionre.match(tag) > + if m: > + return n, int(m.group(1)) > + return None, None > + > +def getdiff(ctx, diffopts): > + """plain-text diff without header (user, commit message, etc)""" > + output = util.stringio() > + for chunk, _label in patch.diffui(ctx.repo(), ctx.p1().node(), ctx.node(), > + None, opts=diffopts): > + output.write(chunk) > + return output.getvalue() > + > +def creatediff(ctx): > + """create a Differential Diff""" > + repo = ctx.repo() > + repophid = getrepophid(repo) > + # Create a "Differential Diff" via "differential.createrawdiff" API > + params = {'diff': getdiff(ctx, mdiff.diffopts(git=True, context=32767))} > + if repophid: > + params['repositoryPHID'] = repophid > + diff = callconduit(repo, 'differential.createrawdiff', params) > + if not diff: > + raise error.Abort(_('cannot create diff for %s') % ctx) > + return diff > + > +def writediffproperties(ctx, diff): > + """write metadata to diff so patches could be applied losslessly""" > + params = { > + 'diff_id': diff[r'id'], > + 'name': 'hg:meta', > + 'data': json.dumps({ > + 'user': ctx.user(), > + 'date': '%d %d' % ctx.date(), > + }), > + } > + callconduit(ctx.repo(), 'differential.setdiffproperty', params) > + > +def createdifferentialrevision(ctx, revid=None, parentrevid=None): > + """create or update a Differential Revision > + > + If revid is None, create a new Differential Revision, otherwise update > + revid. If parentrevid is not None, set it as a dependency. > + """ > + repo = ctx.repo() > + diff = creatediff(ctx) > + writediffproperties(ctx, diff) > + > + transactions = [{'type': 'update', 'value': diff[r'phid']}] > + > + # Use a temporary summary to set dependency. There might be better ways but > + # I cannot find them for now. But do not do that if we are updating an > + # existing revision (revid is not None) since that introduces visible > + # churns (someone edited "Summary" twice) on the web page. > + if parentrevid and revid is None: > + summary = 'Depends on D%s' % parentrevid > + transactions += [{'type': 'summary', 'value': summary}, > + {'type': 'summary', 'value': ' '}] > + > + # Parse commit message and update related fields. > + desc = ctx.description() > + info = callconduit(repo, 'differential.parsecommitmessage', > + {'corpus': desc}) > + for k, v in info[r'fields'].items(): > + if k in ['title', 'summary', 'testPlan']: > + transactions.append({'type': k, 'value': v}) > + > + params = {'transactions': transactions} > + if revid is not None: > + # Update an existing Differential Revision > + params['objectIdentifier'] = revid > + > + revision = callconduit(repo, 'differential.revision.edit', params) > + if not revision: > + raise error.Abort(_('cannot create revision for %s') % ctx) > + > + return revision > + > +@command('phabsend', > + [('r', 'rev', [], _('revisions to send'), _('REV'))], > + _('REV [OPTIONS]')) > +def phabsend(ui, repo, *revs, **opts): > + """upload changesets to Phabricator > + > + If there are multiple revisions specified, they will be send as a stack > + with a linear dependencies relationship using the order specified by the > + revset. > + > + For the first time uploading changesets, local tags will be created to > + maintain the association. After the first time, phabsend will check > + obsstore and tags information so it can figure out whether to update an > + existing Differential Revision, or create a new one. > + """ > + revs = list(revs) + opts.get('rev', []) > + revs = scmutil.revrange(repo, revs) > + > + # Send patches one by one so we know their Differential Revision IDs and > + # can provide dependency relationship > + lastrevid = None > + for rev in revs: > + ui.debug('sending rev %d\n' % rev) > + ctx = repo[rev] > + > + # Get Differential Revision ID > + oldnode, revid = getmapping(ctx) > + if oldnode != ctx.node(): > + # Create or update Differential Revision > + revision = createdifferentialrevision(ctx, revid, lastrevid) > + newrevid = int(revision[r'object'][r'id']) > + if revid: > + action = _('updated') > + else: > + action = _('created') > + > + # Create a local tag to note the association > + tagname = 'D%d' % newrevid > + tags.tag(repo, tagname, ctx.node(), message=None, user=None, > + date=None, local=True) > + else: > + # Nothing changed. But still set "newrevid" so the next revision > + # could depend on this one. > + newrevid = revid > + action = _('skipped') > + > + ui.write(_('D%s: %s - %s: %s\n') % (newrevid, action, ctx, > + ctx.description().split('\n')[0])) > + lastrevid = newrevid > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Excerpts from Phillip Cohen's message of 2017-07-03 13:38:34 -0700: > > + """ > > + revs = list(revs) + opts.get('rev', []) > > + revs = scmutil.revrange(repo, revs) > > + > > I'd add something like this here: > > if not revs: > raise error.Abort(_('must specify at least one changeset with -r')) Good point. I'll send a follow up.
Patch
diff --git a/contrib/phabricator.py b/contrib/phabricator.py --- a/contrib/phabricator.py +++ b/contrib/phabricator.py @@ -7,4 +7,11 @@ """simple Phabricator integration +This extension provides a ``phabsend`` command which sends a stack of +changesets to Phabricator without amending commit messages. + +By default, Phabricator requires ``Test Plan`` which might prevent some +changeset from being sent. The requirement could be disabled by changing +``differential.require-test-plan-field`` config server side. + Config:: @@ -15,4 +22,8 @@ Config:: # API token. Get it from https://$HOST/conduit/login/ token = cli-xxxxxxxxxxxxxxxxxxxxxxxxxxxx + + # Repo callsign. If a repo has a URL https://$HOST/diffusion/FOO, then its + # callsign is "FOO". + callsign = FOO """ @@ -20,9 +31,16 @@ from __future__ import absolute_import import json +import re from mercurial.i18n import _ from mercurial import ( + encoding, error, + mdiff, + obsolete, + patch, registrar, + scmutil, + tags, url as urlmod, util, @@ -97,2 +115,161 @@ def debugcallconduit(ui, repo, name): s = json.dumps(result, sort_keys=True, indent=2, separators=(',', ': ')) ui.write('%s\n' % s) + +def getrepophid(repo): + """given callsign, return repository PHID or None""" + # developer config: phabricator.repophid + repophid = repo.ui.config('phabricator', 'repophid') + if repophid: + return repophid + callsign = repo.ui.config('phabricator', 'callsign') + if not callsign: + return None + query = callconduit(repo, 'diffusion.repository.search', + {'constraints': {'callsigns': [callsign]}}) + if len(query[r'data']) == 0: + return None + repophid = encoding.strtolocal(query[r'data'][0][r'phid']) + repo.ui.setconfig('phabricator', 'repophid', repophid) + return repophid + +_differentialrevisionre = re.compile('\AD([1-9][0-9]*)\Z') + +def getmapping(ctx): + """return (node, associated Differential Revision ID) or (None, None) + + Examines all precursors and their tags. Tags with format like "D1234" are + considered a match and the node with that tag, and the number after "D" + (ex. 1234) will be returned. + """ + unfi = ctx.repo().unfiltered() + nodemap = unfi.changelog.nodemap + for n in obsolete.allprecursors(unfi.obsstore, [ctx.node()]): + if n in nodemap: + for tag in unfi.nodetags(n): + m = _differentialrevisionre.match(tag) + if m: + return n, int(m.group(1)) + return None, None + +def getdiff(ctx, diffopts): + """plain-text diff without header (user, commit message, etc)""" + output = util.stringio() + for chunk, _label in patch.diffui(ctx.repo(), ctx.p1().node(), ctx.node(), + None, opts=diffopts): + output.write(chunk) + return output.getvalue() + +def creatediff(ctx): + """create a Differential Diff""" + repo = ctx.repo() + repophid = getrepophid(repo) + # Create a "Differential Diff" via "differential.createrawdiff" API + params = {'diff': getdiff(ctx, mdiff.diffopts(git=True, context=32767))} + if repophid: + params['repositoryPHID'] = repophid + diff = callconduit(repo, 'differential.createrawdiff', params) + if not diff: + raise error.Abort(_('cannot create diff for %s') % ctx) + return diff + +def writediffproperties(ctx, diff): + """write metadata to diff so patches could be applied losslessly""" + params = { + 'diff_id': diff[r'id'], + 'name': 'hg:meta', + 'data': json.dumps({ + 'user': ctx.user(), + 'date': '%d %d' % ctx.date(), + }), + } + callconduit(ctx.repo(), 'differential.setdiffproperty', params) + +def createdifferentialrevision(ctx, revid=None, parentrevid=None): + """create or update a Differential Revision + + If revid is None, create a new Differential Revision, otherwise update + revid. If parentrevid is not None, set it as a dependency. + """ + repo = ctx.repo() + diff = creatediff(ctx) + writediffproperties(ctx, diff) + + transactions = [{'type': 'update', 'value': diff[r'phid']}] + + # Use a temporary summary to set dependency. There might be better ways but + # I cannot find them for now. But do not do that if we are updating an + # existing revision (revid is not None) since that introduces visible + # churns (someone edited "Summary" twice) on the web page. + if parentrevid and revid is None: + summary = 'Depends on D%s' % parentrevid + transactions += [{'type': 'summary', 'value': summary}, + {'type': 'summary', 'value': ' '}] + + # Parse commit message and update related fields. + desc = ctx.description() + info = callconduit(repo, 'differential.parsecommitmessage', + {'corpus': desc}) + for k, v in info[r'fields'].items(): + if k in ['title', 'summary', 'testPlan']: + transactions.append({'type': k, 'value': v}) + + params = {'transactions': transactions} + if revid is not None: + # Update an existing Differential Revision + params['objectIdentifier'] = revid + + revision = callconduit(repo, 'differential.revision.edit', params) + if not revision: + raise error.Abort(_('cannot create revision for %s') % ctx) + + return revision + +@command('phabsend', + [('r', 'rev', [], _('revisions to send'), _('REV'))], + _('REV [OPTIONS]')) +def phabsend(ui, repo, *revs, **opts): + """upload changesets to Phabricator + + If there are multiple revisions specified, they will be send as a stack + with a linear dependencies relationship using the order specified by the + revset. + + For the first time uploading changesets, local tags will be created to + maintain the association. After the first time, phabsend will check + obsstore and tags information so it can figure out whether to update an + existing Differential Revision, or create a new one. + """ + revs = list(revs) + opts.get('rev', []) + revs = scmutil.revrange(repo, revs) + + # Send patches one by one so we know their Differential Revision IDs and + # can provide dependency relationship + lastrevid = None + for rev in revs: + ui.debug('sending rev %d\n' % rev) + ctx = repo[rev] + + # Get Differential Revision ID + oldnode, revid = getmapping(ctx) + if oldnode != ctx.node(): + # Create or update Differential Revision + revision = createdifferentialrevision(ctx, revid, lastrevid) + newrevid = int(revision[r'object'][r'id']) + if revid: + action = _('updated') + else: + action = _('created') + + # Create a local tag to note the association + tagname = 'D%d' % newrevid + tags.tag(repo, tagname, ctx.node(), message=None, user=None, + date=None, local=True) + else: + # Nothing changed. But still set "newrevid" so the next revision + # could depend on this one. + newrevid = revid + action = _('skipped') + + ui.write(_('D%s: %s - %s: %s\n') % (newrevid, action, ctx, + ctx.description().split('\n')[0])) + lastrevid = newrevid