This is an archive of the discontinued Mercurial Phabricator instance.

Differential D299

py3: introduce a wrapper for __builtins__.{raw_,}input()
ClosedPublic

Authored by durin42 on Aug 9 2017, 10:24 AM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG7d5bc0e5b88f: py3: introduce a wrapper for __builtins__.{raw_,}input()
rHGd390726b575d: py3: introduce a wrapper for __builtins__.{raw_,}input()
Summary

In order to make this work, we have to wrap the io streams in a
TextIOWrapper so that __builtins__.input() can do unicode IO on Python

  1. We can't just restore the original (unicode) sys.std* because we

might be running a cmdserver, and if we blindly restore sys.* to the
original values then we end up breaking the cmdserver. Sadly,
TextIOWrapper tries to close the underlying stream during its del,
so we have to make a sublcass to prevent that.

If you see errors like:

TypeError: a bytes-like object is required, not 'str'

On an input() or print() call on Python 3, the substitution of
sys.std* is probably the root cause.

A previous version of this change tried to put the bytesinput() method
in pycompat - it turns out we need to do some encoding handling, so we
have to be in a higher layer that's allowed to use
mercurial.encoding.encoding. As a result, this is in util for now,
with the TextIOWrapper subclass hiding in encoding.py. I'm not sure of
a better place for the time being.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

durin42 created this revision.Aug 9 2017, 10:24 AM
yuja added a subscriber: yuja.Aug 9 2017, 11:41 AM
yuja added inline comments.
mercurial/pycompat.py
84 ↗(On Diff #682)

Needs to specify encoding because user input may contain
non-ascii characters.

Perhaps it should be noclosetextio(s, encoding=encoding.encoding) and
encoding.strtolocal(input(...)).
Alternatively, forcing latin-1 might work, but I'm not sure.

mercurial/ui.py
1222–1223

I think it's better to specify input and output explicitly.

bytesinput(..., self.fin, self.fout)

Setting BytesIO to sys.stdin/stdout is just plain wrong on Python 3.

durin42 marked an inline comment as done.Aug 11 2017, 3:52 PM
durin42 added inline comments.
mercurial/pycompat.py
84 ↗(On Diff #682)

Ugh, gross. I can't use encoding from pycompat, can I?

mercurial/ui.py
1222–1223

Yeah, it's pretty clowny.

durin42 marked an inline comment as done.Aug 11 2017, 3:52 PM
durin42 updated this revision to Diff 801.
yuja added inline comments.Aug 12 2017, 6:31 AM
hgext/hgk.py
100

Perhaps hgk should use ui.fin and ui.fout.

mercurial/pycompat.py
84 ↗(On Diff #682)

Nah. So this could be util.bytesinput(fin, fout) and encoding.strio().

durin42 edited the summary of this revision. (Show Details)Aug 15 2017, 4:49 PM
durin42 retitled this revision from pycompat: introduce a wrapper for __builtins__.{raw_,}input() to py3: introduce a wrapper for __builtins__.{raw_,}input().
durin42 updated this revision to Diff 948.
durin42 marked 5 inline comments as done.Aug 15 2017, 4:50 PM
durin42 added inline comments.
mercurial/pycompat.py
84 ↗(On Diff #682)

Sigh. I'm not sure I got the encoding.py bit quite right, take a look?

yuja accepted this revision.Aug 15 2017, 11:34 PM
yuja added inline comments.
mercurial/util.py
180

s/pycompat.bytestr/encoding.strtolocal/ and queued, thanks.

This revision is now accepted and ready to land.Aug 15 2017, 11:34 PM
durin42 marked an inline comment as done.Aug 15 2017, 11:47 PM
Closed by commit rHGd390726b575d: py3: introduce a wrapper for __builtins__.{raw_,}input() (authored by durin42). · Explain Why
This revision was automatically updated to reflect the committed changes.
yuja added inline comments.Aug 16 2017, 12:41 AM
mercurial/encoding.py
585

Fixed as s/encoding/_sysstr(encoding)/.

Revision Contents
Changeset List

PathPackages
Mhgext/hgk.py (7 lines)
Mmercurial/encoding.py (14 lines)
Mmercurial/ui.py (10 lines)
Mmercurial/util.py (12 lines)

Diff 971

hgext/hgk.py

short, short,
) )
from mercurial import ( from mercurial import (
commands, commands,
obsolete, obsolete,
patch, patch,
registrar, registrar,
scmutil, scmutil,
util,
) )
cmdtable = {} cmdtable = {}
command = registrar.command(cmdtable) command = registrar.command(cmdtable)
# Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' for # Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' for
# extensions which SHIP WITH MERCURIAL. Non-mainline extensions should # extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
# be specifying the version(s) of Mercurial they are tested with, or # be specifying the version(s) of Mercurial they are tested with, or
# leave the attribute unspecified. # leave the attribute unspecified.
for f in removed: for f in removed:
ui.write((":100664 000000 %s %s D\t%s\t%s\n") % ui.write((":100664 000000 %s %s D\t%s\t%s\n") %
(short(mmap[f]), empty, f, f)) (short(mmap[f]), empty, f, f))
## ##
while True: while True:
if opts['stdin']: if opts['stdin']:
try: try:
line = raw_input().split(' ') line = util.bytesinput(ui.fin, ui.fout).split(' ')
yujaUnsubmitted
Done

Perhaps hgk should use ui.fin and ui.fout.

yuja: Perhaps hgk should use ui.fin and ui.fout.
node1 = line[0] node1 = line[0]
if len(line) > 1: if len(line) > 1:
node2 = line[1] node2 = line[1]
else: else:
node2 = None node2 = None
except EOFError: except EOFError:
break break
node1 = repo.lookup(node1) node1 = repo.lookup(node1)
"""cat a specific revision""" """cat a specific revision"""
# in stdin mode, every line except the commit is prefixed with two # in stdin mode, every line except the commit is prefixed with two
# spaces. This way the our caller can find the commit without magic # spaces. This way the our caller can find the commit without magic
# strings # strings
# #
prefix = "" prefix = ""
if opts['stdin']: if opts['stdin']:
try: try:
(type, r) = raw_input().split(' ') (type, r) = util.bytesinput(ui.fin, ui.fout).split(' ')
prefix = " " prefix = " "
except EOFError: except EOFError:
return return
else: else:
if not type or not r: if not type or not r:
ui.warn(_("cat-file: type or revision not supplied\n")) ui.warn(_("cat-file: type or revision not supplied\n"))
commands.help_(ui, 'cat-file') commands.help_(ui, 'cat-file')
while r: while r:
if type != "commit": if type != "commit":
ui.warn(_("aborting hg cat-file only understands commits\n")) ui.warn(_("aborting hg cat-file only understands commits\n"))
return 1 return 1
n = repo.lookup(r) n = repo.lookup(r)
catcommit(ui, repo, n, prefix) catcommit(ui, repo, n, prefix)
if opts['stdin']: if opts['stdin']:
try: try:
(type, r) = raw_input().split(' ') (type, r) = util.bytesinput(ui.fin, ui.fout).split(' ')
except EOFError: except EOFError:
break break
else: else:
break break
# git rev-tree is a confusing thing. You can supply a number of # git rev-tree is a confusing thing. You can supply a number of
# commit sha1s on the command line, and it walks the commit history # commit sha1s on the command line, and it walks the commit history
# telling you which commits are reachable from the supplied ones via # telling you which commits are reachable from the supplied ones via

mercurial/encoding.py

# encoding.py - character transcoding support for Mercurial # encoding.py - character transcoding support for Mercurial
# #
# Copyright 2005-2009 Matt Mackall <mpm@selenic.com> and others # Copyright 2005-2009 Matt Mackall <mpm@selenic.com> and others
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
import array import array
import io
import locale import locale
import os import os
import unicodedata import unicodedata
from . import ( from . import (
error, error,
policy, policy,
pycompat, pycompat,
while pos < l: while pos < l:
c = getutf8char(s, pos) c = getutf8char(s, pos)
pos += len(c) pos += len(c)
# unescape U+DCxx characters # unescape U+DCxx characters
if "\xed\xb0\x80" <= c <= "\xed\xb3\xbf": if "\xed\xb0\x80" <= c <= "\xed\xb3\xbf":
c = chr(ord(c.decode("utf-8")) & 0xff) c = chr(ord(c.decode("utf-8")) & 0xff)
r += c r += c
return r return r
class strio(io.TextIOWrapper):
"""Wrapper around TextIOWrapper that respects hg's encoding assumptions.
Also works around Python closing streams.
"""
def __init__(self, buffer, **kwargs):
kwargs[r'encoding'] = encoding
yujaUnsubmitted
Not Done

Fixed as s/encoding/_sysstr(encoding)/.

yuja: Fixed as `s/encoding/_sysstr(encoding)/`.
super(strio, self).__init__(buffer, **kwargs)
def __del__(self):
"""Override __del__ so it doesn't close the underlying stream."""

mercurial/ui.py

except Exception: except Exception:
pass pass
# call write() so output goes through subclassed implementation # call write() so output goes through subclassed implementation
# e.g. color extension on Windows # e.g. color extension on Windows
self.write(prompt, prompt=True) self.write(prompt, prompt=True)
self.flush() self.flush()
# instead of trying to emulate raw_input, swap (self.fin,
# self.fout) with (sys.stdin, sys.stdout)
oldin = sys.stdin
oldout = sys.stdout
sys.stdin = self.fin
sys.stdout = self.fout
# prompt ' ' must exist; otherwise readline may delete entire line # prompt ' ' must exist; otherwise readline may delete entire line
# - http://bugs.python.org/issue12833 # - http://bugs.python.org/issue12833
with self.timeblockedsection('stdio'): with self.timeblockedsection('stdio'):
line = raw_input(' ') line = util.bytesinput(self.fin, self.fout, r' ')
yujaUnsubmitted
Done

I think it's better to specify input and output explicitly.

bytesinput(..., self.fin, self.fout)

Setting BytesIO to sys.stdin/stdout is just plain wrong on Python 3.

yuja: I think it's better to specify input and output explicitly. `bytesinput(..., self.fin, self.
durin42AuthorUnsubmitted
Done

Yeah, it's pretty clowny.

durin42: Yeah, it's pretty clowny.
sys.stdin = oldin
sys.stdout = oldout
# When stdin is in binary mode on Windows, it can cause # When stdin is in binary mode on Windows, it can cause
# raw_input() to emit an extra trailing carriage return # raw_input() to emit an extra trailing carriage return
if pycompat.oslinesep == '\r\n' and line and line[-1] == '\r': if pycompat.oslinesep == '\r\n' and line and line[-1] == '\r':
line = line[:-1] line = line[:-1]
return line return line
def prompt(self, msg, default="y"): def prompt(self, msg, default="y"):

mercurial/util.py

# disable Python's problematic floating point timestamps (issue4836) # disable Python's problematic floating point timestamps (issue4836)
# (Python hypocritically says you shouldn't change this behavior in # (Python hypocritically says you shouldn't change this behavior in
# libraries, and sure enough Mercurial is not a library.) # libraries, and sure enough Mercurial is not a library.)
os.stat_float_times(False) os.stat_float_times(False)
def safehasattr(thing, attr): def safehasattr(thing, attr):
return getattr(thing, attr, _notset) is not _notset return getattr(thing, attr, _notset) is not _notset
def bytesinput(fin, fout, *args, **kwargs):
sin, sout = sys.stdin, sys.stdout
try:
if pycompat.ispy3:
sys.stdin, sys.stdout = encoding.strio(fin), encoding.strio(fout)
return encoding.strtolocal(input(*args, **kwargs))
yujaUnsubmitted
Not Done

s/pycompat.bytestr/encoding.strtolocal/ and queued, thanks.

yuja: s/pycompat.bytestr/encoding.strtolocal/ and queued, thanks.
else:
sys.stdin, sys.stdout = fin, fout
return raw_input(*args, **kwargs)
finally:
sys.stdin, sys.stdout = sin, sout
def bitsfrom(container): def bitsfrom(container):
bits = 0 bits = 0
for bit in container: for bit in container:
bits |= bit bits |= bit
return bits return bits
# python 2.6 still have deprecation warning enabled by default. We do not want # python 2.6 still have deprecation warning enabled by default. We do not want
# to display anything to standard user so detect if we are running test and # to display anything to standard user so detect if we are running test and