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.

Download Raw Diff

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

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

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

durin42 created this revision.Aug 9 2017, 10:24 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptAug 9 2017, 10:24 AM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

durin42 added a child revision: D300: python3: whitelist four more passing tests.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	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	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	Ugh, gross. I can't use encoding from pycompat, can I?
mercurial/ui.py
1222	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	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	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 ↗	(On Diff #948)	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 ↗	(On Diff #971)	Fixed as `s/encoding/_sysstr(encoding)/`.

Revision Contents
Changeset List

			Path	Packages
M			hgext/hgk.py (7 lines)
M			mercurial/pycompat.py (18 lines)
M			mercurial/ui.py (2 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	682		Aug 9 2017, 10:24 AM	★	★
Diff 2	801		Aug 11 2017, 3:52 PM	★	★
Diff 3	948		Aug 15 2017, 4:49 PM	★	★
Diff 4	971	rHGd390726b575d9e18d0b5c14ee2e46fac7eb028fb	Jul 24 2017, 2:38 PM	★	★

Status	Author	Revision
Closed	durin42	D279 tests: fix test-notify.t to use $PYTHON
Closed	durin42	D278 tests: make test-highlight code portable to python3
Closed	durin42	D277 dagparser: make print statement in doctest Py3 portable
Closed	durin42	D276 i18n: fix check-translation.py to be less broken on Python 3
Closed	durin42	D275 ui: refactor extractchoices so it doesn't break on Python 3
Closed	durin42	D274 obsutil: defend against succsmarkers() returning None
Closed	durin42	D273 changegroup: more **kwargs
Closed	durin42	D272 bundle2: convert ints to strings using pycompat.bytestring()
Closed	durin42	D353 extensions: don't give AttributeError bytes message on Python 3
Closed	durin42	D302 python3: whitelist another 13 tests that now pass
Closed	durin42	D301 scmutil: use util.shellquote instead of %r
Abandoned	durin42	D300 python3: whitelist four more passing tests
Closed	durin42	D299 py3: introduce a wrapper for __builtins__.{raw_,}input()
Closed	durin42	D298 python3: whitelist four more passing tests
Closed	durin42	D297 tests: update duplicateoptions test to use bytestrings everywhere
Closed	durin42	D296 extensions: if on py3 and propname is a bytestr, convert to sysstr
Closed	durin42	D295 extensions: attempt to use non-deprecated inspect method
Closed	durin42	D294 tests: clean up many print statements to be print functions instead
Closed	durin42	D293 tests: fix simple heredoc print statements to work on Py3
Closed	durin42	D271 obsolete: use bytes() instead of str() so the node is bytes on py3
Closed	durin42	D270 tests: fix up test-run-tests failures on Python 3.6

Diff 682

hgext/hgk.py

	nullid,			nullid,
	nullrev,			nullrev,
	short,			short,
	)			)
	from mercurial import (			from mercurial import (
	commands,			commands,
	obsolete,			obsolete,
	patch,			patch,
				pycompat,
	registrar,			registrar,
	scmutil,			scmutil,
	)			)

	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
	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 = pycompat.bytesinput().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) = pycompat.bytesinput().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) = pycompat.bytesinput().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/pycompat.py

	maplist = lambda args: list(map(args))			maplist = lambda args: list(map(args))

	# TODO: .buffer might not exist if std streams were replaced; we'll need			# TODO: .buffer might not exist if std streams were replaced; we'll need
	# a silly wrapper to make a bytes stream backed by a unicode one.			# a silly wrapper to make a bytes stream backed by a unicode one.
	stdin = sys.stdin.buffer			stdin = sys.stdin.buffer
	stdout = sys.stdout.buffer			stdout = sys.stdout.buffer
	stderr = sys.stderr.buffer			stderr = sys.stderr.buffer

				class noclosetextio(io.TextIOWrapper):
				def __del__(self):
				"""Override __del__ so it doesn't close the underlying stream."""

				def bytesinput(args, *kwargs):
				origs = {}
				try:
				for stream in 'stdin stdout stderr'.split():
				s = getattr(sys, stream)
				origs[stream] = s
				if not isinstance(s, io.TextIOBase):
				setattr(sys, stream, noclosetextio(s))
				return bytestr(input(args, *kwargs))
				yujaUnsubmitted Done 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. yuja: Needs to specify encoding because user input may contain non-ascii characters. Perhaps it…
				durin42AuthorUnsubmitted Done Ugh, gross. I can't use encoding from pycompat, can I? durin42: Ugh, gross. I can't use encoding from pycompat, can I?
				yujaUnsubmitted Done Nah. So this could be `util.bytesinput(fin, fout)` and `encoding.strio()`. yuja: Nah. So this could be `util.bytesinput(fin, fout)` and `encoding.strio()`.
				durin42AuthorUnsubmitted Not Done Sigh. I'm not sure I got the encoding.py bit quite right, take a look? durin42: Sigh. I'm not sure I got the encoding.py bit quite right, take a look?
				finally:
				for stream, restore in origs.items():
				setattr(sys, stream, restore)

	# Since Python 3 converts argv to wchar_t type by Py_DecodeLocale() on Unix,			# Since Python 3 converts argv to wchar_t type by Py_DecodeLocale() on Unix,
	# we can use os.fsencode() to get back bytes argv.			# we can use os.fsencode() to get back bytes argv.
	#			#
	# https://hg.python.org/cpython/file/v3.5.1/Programs/python.c#l55			# https://hg.python.org/cpython/file/v3.5.1/Programs/python.c#l55
	#			#
	# TODO: On Windows, the native argv is wchar_t, so we'll need a different			# TODO: On Windows, the native argv is wchar_t, so we'll need a different
	# workaround to simulate the Python 2 (i.e. ANSI Win32 API) behavior.			# workaround to simulate the Python 2 (i.e. ANSI Win32 API) behavior.
	if getattr(sys, 'argv', None) is not None:			if getattr(sys, 'argv', None) is not None:
	oslinesep = os.linesep			oslinesep = os.linesep
	osname = os.name			osname = os.name
	ospathsep = os.pathsep			ospathsep = os.pathsep
	ossep = os.sep			ossep = os.sep
	osaltsep = os.altsep			osaltsep = os.altsep
	stdin = sys.stdin			stdin = sys.stdin
	stdout = sys.stdout			stdout = sys.stdout
	stderr = sys.stderr			stderr = sys.stderr
				bytesinput = raw_input
	if getattr(sys, 'argv', None) is not None:			if getattr(sys, 'argv', None) is not None:
	sysargv = sys.argv			sysargv = sys.argv
	sysplatform = sys.platform			sysplatform = sys.platform
	getcwd = os.getcwd			getcwd = os.getcwd
	sysexecutable = sys.executable			sysexecutable = sys.executable
	shlexsplit = shlex.split			shlexsplit = shlex.split
	stringio = cStringIO.StringIO			stringio = cStringIO.StringIO
	maplist = map			maplist = map

mercurial/ui.py

	# self.fout) with (sys.stdin, sys.stdout)			# self.fout) with (sys.stdin, sys.stdout)
	oldin = sys.stdin			oldin = sys.stdin
	oldout = sys.stdout			oldout = sys.stdout
	sys.stdin = self.fin			sys.stdin = self.fin
	sys.stdout = self.fout			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 = pycompat.bytesinput(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.stdin = oldin
	sys.stdout = oldout			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