mercurial-scm/hg: hgext/phabricator.py comparison

comparison hgext/phabricator.py @ 41897:c340a8ac7ef3

phabricator: convert conduit response JSON unicode to bytes inside callconduit Previously the byte conversion was happening piecemeal in callers, and in the case of createdifferentialrevision not at all, leading to UnicodeEncodeErrors when trying to phabsend a commit with a description containing characters not representable in ascii. (issue6040) Remove all the scattered encoding.unitolocal calls and perform it once, inside callconduit, on the entire response dict recursively before returning it, in keeping with the strategy of converting at the earliest opportunity. Convert all keys used on returned object to bytes. Modify the phabsend tests to test this by adding a ? to the commit message of alpha. Differential Revision: https://phab.mercurial-scm.org/D6044

author	Ian Moody <moz-ian@perix.co.uk>
date	Sat, 02 Mar 2019 18:48:23 +0000
parents	570e62f1dcf2
children	2bad8f92cebf

comparison

equal deleted inserted replaced

-:94faa2e84094
+:c340a8ac7ef3
 mdiff,
 obsutil,
 parser,
 patch,
 phases,
+pycompat,
 registrar,
 scmutil,
 smartset,
 tags,
 templateutil,
 urlopener = urlmod.opener(repo.ui, authinfo)
 request = util.urlreq.request(url, data=data)
 with contextlib.closing(urlopener.open(request)) as rsp:
 body = rsp.read()
 repo.ui.debug(b'Conduit Response: %s\n' % body)
-parsed = json.loads(body)
+parsed = pycompat.rapply(
-if parsed.get(r'error_code'):
+lambda x: encoding.unitolocal(x) if isinstance(x, pycompat.unicode)
+else x,
+json.loads(body)
+)
+if parsed.get(b'error_code'):
 msg = (_(b'Conduit Error (%s): %s')
-% (parsed[r'error_code'], parsed[r'error_info']))
+% (parsed[b'error_code'], parsed[b'error_info']))
 raise error.Abort(msg)
-return parsed[r'result']
+return parsed[b'result']
 @vcrcommand(b'debugcallconduit', [], _(b'METHOD'))
 def debugcallconduit(ui, repo, name):
 """call Conduit API
 callsign = repo.ui.config(b'phabricator', b'callsign')
 if not callsign:
 return None
 query = callconduit(repo, b'diffusion.repository.search',
 {b'constraints': {b'callsigns': [callsign]}})
-if len(query[r'data']) == 0:
+if len(query[b'data']) == 0:
 return None
-repophid = encoding.strtolocal(query[r'data'][0][r'phid'])
+repophid = query[b'data'][0][b'phid']
 repo.ui.setconfig(b'phabricator', b'repophid', repophid)
 return repophid
 _differentialrevisiontagre = re.compile(br'\AD([1-9][0-9]*)\Z')
 _differentialrevisiondescre = re.compile(
 # Phabricator, and expect precursors overlap with it.
 if toconfirm:
 drevs = [drev for force, precs, drev in toconfirm.values()]
 alldiffs = callconduit(unfi, b'differential.querydiffs',
 {b'revisionIDs': drevs})
-getnode = lambda d: bin(encoding.unitolocal(
+getnode = lambda d: bin(
-getdiffmeta(d).get(r'node', b''))) or None
+getdiffmeta(d).get(b'node', b'')) or None
 for newnode, (force, precset, drev) in toconfirm.items():
 diffs = [d for d in alldiffs.values()
-if int(d[r'revisionID']) == drev]
+if int(d[b'revisionID']) == drev]
 # "precursors" as known by Phabricator
 phprecset = set(getnode(d) for d in diffs)
 # Ignore if precursors (Phabricator and local repo) do not overlap,
 # Find the last node using Phabricator metadata, and make sure it
 # exists in the repo
 oldnode = lastdiff = None
 if diffs:
-lastdiff = max(diffs, key=lambda d: int(d[r'id']))
+lastdiff = max(diffs, key=lambda d: int(d[b'id']))
 oldnode = getnode(lastdiff)
 if oldnode and oldnode not in nodemap:
 oldnode = None
 result[newnode] = (oldnode, lastdiff, drev)
 return diff
 def writediffproperties(ctx, diff):
 """write metadata to diff so patches could be applied losslessly"""
 params = {
-b'diff_id': diff[r'id'],
+b'diff_id': diff[b'id'],
 b'name': b'hg:meta',
 b'data': json.dumps({
 b'user': ctx.user(),
 b'date': b'%d %d' % ctx.date(),
 b'node': ctx.hex(),
 }),
 }
 callconduit(ctx.repo(), b'differential.setdiffproperty', params)
 params = {
-b'diff_id': diff[r'id'],
+b'diff_id': diff[b'id'],
 b'name': b'local:commits',
 b'data': json.dumps({
 ctx.hex(): {
 b'author': stringutil.person(ctx.user()),
 b'authorEmail': stringutil.email(ctx.user()),
 neednewdiff = True
 transactions = []
 if neednewdiff:
 diff = creatediff(ctx)
-transactions.append({b'type': b'update', b'value': diff[r'phid']})
+transactions.append({b'type': b'update', b'value': diff[b'phid']})
 else:
 # 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
 # pushers could know the correct node metadata.
 assert olddiff
 # Parse commit message and update related fields.
 desc = ctx.description()
 info = callconduit(repo, b'differential.parsecommitmessage',
 {b'corpus': desc})
-for k, v in info[r'fields'].items():
+for k, v in info[b'fields'].items():
 if k in [b'title', b'summary', b'testPlan']:
 transactions.append({b'type': k, b'value': v})
 params = {b'transactions': transactions}
 if revid is not None:
 names = [name.lower() for name in names]
 query = {b'constraints': {b'usernames': names}}
 result = callconduit(repo, b'user.search', query)
 # username not found is not an error of the API. So check if we have missed
 # some names here.
-data = result[r'data']
+data = result[b'data']
-resolved = set(entry[r'fields'][r'username'].lower() for entry in data)
+resolved = set(entry[b'fields'][b'username'].lower() for entry in data)
 unresolved = set(names) - resolved
 if unresolved:
 raise error.Abort(_(b'unknown username: %s')
 % b' '.join(sorted(unresolved)))
-return [entry[r'phid'] for entry in data]
+return [entry[b'phid'] for entry in data]
 @vcrcommand(b'phabsend',
 [(b'r', b'rev', [], _(b'revisions to send'), _(b'REV')),
 (b'', b'amend', True, _(b'update commit messages')),
 (b'', b'reviewer', [], _(b'specify reviewers')),
 if oldnode != ctx.node() or opts.get(b'amend'):
 # Create or update Differential Revision
 revision, diff = createdifferentialrevision(
 ctx, revid, lastrevid, oldnode, olddiff, actions)
 diffmap[ctx.node()] = diff
-newrevid = int(revision[r'object'][r'id'])
+newrevid = int(revision[b'object'][b'id'])
 if revid:
 action = b'updated'
 else:
 action = b'created'
 wnode = unfi[b'.'].node()
 mapping = {} # {oldnode: [newnode]}
 for i, rev in enumerate(revs):
 old = unfi[rev]
 drevid = drevids[i]
-drev = [d for d in drevs if int(d[r'id']) == drevid][0]
+drev = [d for d in drevs if int(d[b'id']) == drevid][0]
 newdesc = getdescfromdrev(drev)
-newdesc = encoding.unitolocal(newdesc)
 # Make sure commit message contain "Differential Revision"
 if old.description() != newdesc:
 if old.phase() == phases.public:
 ui.warn(_("warning: not updating public commit %s\n")
 % scmutil.formatchangeid(old))
 if wnode in mapping:
 unfi.setparents(mapping[wnode][0])
 # Map from "hg:meta" keys to header understood by "hg import". The order is
 # consistent with "hg export" output.
-_metanamemap = util.sortdict([(r'user', b'User'), (r'date', b'Date'),
+_metanamemap = util.sortdict([(b'user', b'User'), (b'date', b'Date'),
-(r'node', b'Node ID'), (r'parent', b'Parent ')])
+(b'node', b'Node ID'), (b'parent', b'Parent ')])
 def _confirmbeforesend(repo, revs, oldmap):
 url, token = readurltoken(repo)
 ui = repo.ui
 for rev in revs:
 _knownstatusnames = {b'accepted', b'needsreview', b'needsrevision', b'closed',
 b'abandoned'}
 def _getstatusname(drev):
 """get normalized status name from a Differential Revision"""
-return drev[r'statusName'].replace(b' ', b'').lower()
+return drev[b'statusName'].replace(b' ', b'').lower()
 # Small language to specify differential revisions. Support symbols: (), :X,
 # +, and -.
 _elements = {
 if key in prefetched:
 return prefetched[key]
 drevs = callconduit(repo, b'differential.query', params)
 # Fill prefetched with the result
 for drev in drevs:
-prefetched[drev[r'phid']] = drev
+prefetched[drev[b'phid']] = drev
-prefetched[int(drev[r'id'])] = drev
+prefetched[int(drev[b'id'])] = drev
 if key not in prefetched:
 raise error.Abort(_(b'cannot get Differential Revision %r')
 % params)
 return prefetched[key]
 result = []
 queue = [{r'ids': [i]} for i in topdrevids]
 while queue:
 params = queue.pop()
 drev = fetch(params)
-if drev[r'id'] in visited:
+if drev[b'id'] in visited:
 continue
-visited.add(drev[r'id'])
+visited.add(drev[b'id'])
-result.append(int(drev[r'id']))
+result.append(int(drev[b'id']))
-auxiliary = drev.get(r'auxiliary', {})
+auxiliary = drev.get(b'auxiliary', {})
-depends = auxiliary.get(r'phabricator:depends-on', [])
+depends = auxiliary.get(b'phabricator:depends-on', [])
 for phid in depends:
 queue.append({b'phids': [phid]})
 result.reverse()
 return smartset.baseset(result)
 # Prefetch Differential Revisions in batch
 tofetch = set(drevs)
 for r in ancestordrevs:
 tofetch.update(range(max(1, r - batchsize), r + 1))
 if drevs:
-fetch({r'ids': list(tofetch)})
+fetch({b'ids': list(tofetch)})
 validids = sorted(set(getstack(list(ancestordrevs))) | set(drevs))
 # Walk through the tree, return smartsets
 def walk(tree):
 op = tree[0]
 """get description (commit message) from "Differential Revision"
 This is similar to differential.getcommitmessage API. But we only care
 about limited fields: title, summary, test plan, and URL.
 """
-title = drev[r'title']
+title = drev[b'title']
-summary = drev[r'summary'].rstrip()
+summary = drev[b'summary'].rstrip()
-testplan = drev[r'testPlan'].rstrip()
+testplan = drev[b'testPlan'].rstrip()
 if testplan:
 testplan = b'Test Plan:\n%s' % testplan
-uri = b'Differential Revision: %s' % drev[r'uri']
+uri = b'Differential Revision: %s' % drev[b'uri']
 return b'\n\n'.join(filter(None, [title, summary, testplan, uri]))
 def getdiffmeta(diff):
 """get commit metadata (date, node, user, p1) from a diff object
 }
 Note: metadata extracted from "local:commits" will lose time zone
 information.
 """
-props = diff.get(r'properties') or {}
+props = diff.get(b'properties') or {}
-meta = props.get(r'hg:meta')
+meta = props.get(b'hg:meta')
-if not meta and props.get(r'local:commits'):
+if not meta and props.get(b'local:commits'):
-commit = sorted(props[r'local:commits'].values())[0]
+commit = sorted(props[b'local:commits'].values())[0]
 meta = {
-r'date': r'%d 0' % commit[r'time'],
+b'date': b'%d 0' % commit[b'time'],
-r'node': commit[r'rev'],
+b'node': commit[b'rev'],
-r'user': r'%s <%s>' % (commit[r'author'], commit[r'authorEmail']),
+b'user': b'%s <%s>' % (commit[b'author'], commit[b'authorEmail']),
 }
-if len(commit.get(r'parents', ())) >= 1:
+if len(commit.get(b'parents', ())) >= 1:
-meta[r'parent'] = commit[r'parents'][0]
+meta[b'parent'] = commit[b'parents'][0]
 return meta or {}
 def readpatch(repo, drevs, write):
 """generate plain-text patch readable by 'hg import'
 write is usually ui.write. drevs is what "querydrev" returns, results of
 "differential.query".
 """
 # Prefetch hg:meta property for all diffs
-diffids = sorted(set(max(int(v) for v in drev[r'diffs']) for drev in drevs))
+diffids = sorted(set(max(int(v) for v in drev[b'diffs']) for drev in drevs))
 diffs = callconduit(repo, b'differential.querydiffs', {b'ids': diffids})
 # Generate patch for each drev
 for drev in drevs:
-repo.ui.note(_(b'reading D%s\n') % drev[r'id'])
+repo.ui.note(_(b'reading D%s\n') % drev[b'id'])
-diffid = max(int(v) for v in drev[r'diffs'])
+diffid = max(int(v) for v in drev[b'diffs'])
 body = callconduit(repo, b'differential.getrawdiff',
 {b'diffID': diffid})
 desc = getdescfromdrev(drev)
 header = b'# HG changeset patch\n'
 for k in _metanamemap.keys():
 if k in meta:
 header += b'# %s %s\n' % (_metanamemap[k], meta[k])
 content = b'%s%s\n%s' % (header, desc, body)
-write(encoding.unitolocal(content))
+write(content)
 @vcrcommand(b'phabread',
 [(b'', b'stack', False, _(b'read dependencies'))],
 _(b'DREVSPEC [OPTIONS]'),
 helpcategory=command.CATEGORY_IMPORT_EXPORT)
 drevs = querydrev(repo, spec)
 for i, drev in enumerate(drevs):
 if i + 1 == len(drevs) and opts.get(b'comment'):
 actions.append({b'type': b'comment', b'value': opts[b'comment']})
 if actions:
-params = {b'objectIdentifier': drev[r'phid'],
+params = {b'objectIdentifier': drev[b'phid'],
 b'transactions': actions}
 callconduit(repo, b'differential.revision.edit', params)
 templatekeyword = registrar.templatekeyword()

Mercurial > public > mercurial-scm > hg

comparison hgext/phabricator.py @ 41897:c340a8ac7ef3