This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7061

convert: don't pass bytes to, or expect bytes from, emailparser
AbandonedPublic

Authored by Kwan on Oct 11 2019, 1:12 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

Kwan created this revision.Oct 11 2019, 1:12 PM
martinvonz added a subscriber: martinvonz.Oct 11 2019, 2:23 PM
martinvonz added inline comments.
hgext/convert/gnuarch.py
301

Is it right to depend on the user's preferred encoding (as I think unifromlocal() does)? Would it make sense to instead initialize self.catlogparser = emailparser.BytesParser()?

Kwan added inline comments.Oct 12 2019, 10:36 AM
hgext/convert/gnuarch.py
301

Hmm, I wasn't aware of that drawback of unifromlocal() if it's true. Is there a canonical "Give me unicode from these mercurial bytes" function? Regardless, BytesParser does sound handy, and is even in 3.5, but isn't present in 2.7. Would doing it conditionally be alright? (and a conditional alias for parsebytes)

self.catlogparser = (
    emailparser.BytesParser()
    if pycompat.ispy3
    else emailparser.Parser()
)
if not pycompat.ispy3:
    self.catlogparser.parsebytes = self.catlogparser.parsestr
-            catlog = self.catlogparser.parsestr(data)
+            catlog = self.catlogparser.parsebytes(data)
durin42 added a subscriber: durin42.Oct 16 2019, 4:43 PM
durin42 added inline comments.
hgext/convert/gnuarch.py
301

It depends which bytes, basically. Some bytes in hg are known to be UTF-8, but anytime we have file contents or filenames we don't know.

I'm not sure of the context here, but maybe the output from tla is in some known encoding?

(I'm also open to the idea of dropping tla convert support as we move to Python 3, as tla has been obsolete for a _long_ time.)

Kwan abandoned this revision.Nov 4 2019, 2:28 PM

Obsoleted by a better fix in cf3bf3b03445.

Revision Contents
Changeset List

PathPackages
Mhgext/convert/gnuarch.py (14 lines)

Diff 17068

hgext/convert/gnuarch.py

def _stripbasepath(self, path): def _stripbasepath(self, path):
if path.startswith(b'./'): if path.startswith(b'./'):
return path[2:] return path[2:]
return path return path
def _parsecatlog(self, data, rev): def _parsecatlog(self, data, rev):
try: try:
catlog = self.catlogparser.parsestr(data) catlog = self.catlogparser.parsestr(encoding.unifromlocal(data))
martinvonzUnsubmitted
Not Done

Is it right to depend on the user's preferred encoding (as I think unifromlocal() does)? Would it make sense to instead initialize self.catlogparser = emailparser.BytesParser()?

martinvonz: Is it right to depend on the user's preferred encoding (as I think `unifromlocal()` does)?
KwanAuthorUnsubmitted
Done

Hmm, I wasn't aware of that drawback of unifromlocal() if it's true. Is there a canonical "Give me unicode from these mercurial bytes" function? Regardless, BytesParser does sound handy, and is even in 3.5, but isn't present in 2.7. Would doing it conditionally be alright? (and a conditional alias for parsebytes)

self.catlogparser = (
    emailparser.BytesParser()
    if pycompat.ispy3
    else emailparser.Parser()
)
if not pycompat.ispy3:
    self.catlogparser.parsebytes = self.catlogparser.parsestr
-            catlog = self.catlogparser.parsestr(data)
+            catlog = self.catlogparser.parsebytes(data)
Kwan: Hmm, I wasn't aware of that drawback of `unifromlocal()` if it's true. Is there a canonical…
durin42Unsubmitted
Not Done

It depends which bytes, basically. Some bytes in hg are known to be UTF-8, but anytime we have file contents or filenames we don't know.

I'm not sure of the context here, but maybe the output from tla is in some known encoding?

(I'm also open to the idea of dropping tla convert support as we move to Python 3, as tla has been obsolete for a _long_ time.)

durin42: It depends which bytes, basically. Some bytes in hg are known to be UTF-8, but anytime we have…
# Commit date # Commit date
self.changes[rev].date = dateutil.datestr( self.changes[rev].date = dateutil.datestr(
dateutil.strdate(catlog[b'Standard-date'], b'%Y-%m-%d %H:%M:%S') dateutil.strdate(catlog['Standard-date'], b'%Y-%m-%d %H:%M:%S')
) )
# Commit author # Commit author
self.changes[rev].author = self.recode(catlog[b'Creator']) self.changes[rev].author = self.recode(catlog['Creator'])
# Commit description # Commit description
self.changes[rev].summary = b'\n\n'.join( self.changes[rev].summary = '\n\n'.join(
(catlog[b'Summary'], catlog.get_payload()) (catlog['Summary'], catlog.get_payload())
) )
self.changes[rev].summary = self.recode(self.changes[rev].summary) self.changes[rev].summary = self.recode(self.changes[rev].summary)
# Commit revision origin when dealing with a branch or tag # Commit revision origin when dealing with a branch or tag
if b'Continuation-of' in catlog: if 'Continuation-of' in catlog:
self.changes[rev].continuationof = self.recode( self.changes[rev].continuationof = self.recode(
catlog[b'Continuation-of'] catlog['Continuation-of']
) )
except Exception: except Exception:
raise error.Abort(_(b'could not parse cat-log of %s') % rev) raise error.Abort(_(b'could not parse cat-log of %s') % rev)
def _parsechangeset(self, data, rev): def _parsechangeset(self, data, rev):
for l in data: for l in data:
l = l.strip() l = l.strip()
# Added file (ignore added directory) # Added file (ignore added directory)