D6044: phabricator: convert conduit response JSON unicode to bytes inside callconduit

Kwan (Ian Moody) phabricator at mercurial-scm.org
Sat Mar 2 18:51:24 UTC 2019


Kwan created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6044

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -60,6 +60,7 @@
     parser,
     patch,
     phases,
+    pycompat,
     registrar,
     scmutil,
     smartset,
@@ -219,7 +220,8 @@
         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(lambda x: encoding.unitolocal(x)
+                             if isinstance(x, unicode) else x, json.loads(body))
     if parsed.get(r'error_code'):
         msg = (_(b'Conduit Error (%s): %s')
                % (parsed[r'error_code'], parsed[r'error_info']))
@@ -251,7 +253,7 @@
                         {b'constraints': {b'callsigns': [callsign]}})
     if len(query[r'data']) == 0:
         return None
-    repophid = encoding.strtolocal(query[r'data'][0][r'phid'])
+    repophid = query[r'data'][0][r'phid']
     repo.ui.setconfig(b'phabricator', b'repophid', repophid)
     return repophid
 
@@ -305,8 +307,8 @@
         drevs = [drev for force, precs, drev in toconfirm.values()]
         alldiffs = callconduit(unfi, b'differential.querydiffs',
                                {b'revisionIDs': drevs})
-        getnode = lambda d: bin(encoding.unitolocal(
-            getdiffmeta(d).get(r'node', b''))) or None
+        getnode = lambda d: bin(
+            getdiffmeta(d).get(r'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]
@@ -582,7 +584,6 @@
                 drevid = drevids[i]
                 drev = [d for d in drevs if int(d[r'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:
@@ -923,7 +924,7 @@
                 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'))],



To: Kwan, #hg-reviewers
Cc: mercurial-devel


More information about the Mercurial-devel mailing list