From c40cdb930b2a9335af0451ef51821d4f2838c61d Mon Sep 17 00:00:00 2001 From: Sven Date: Fri, 9 Jun 2017 16:50:34 +0200 Subject: [PATCH] Fix module autoreload for __main__ with relative imports The fix works for Python >= 3.4, keeping the current workaround for previous versions. Also introduce a first unit test for autoreload. Close #2044. --- tornado/autoreload.py | 36 +++++++++++++++----------- tornado/test/autoreload_test.py | 46 +++++++++++++++++++++++++++++++++ tornado/test/import_test.py | 1 - tornado/test/runtests.py | 1 + 4 files changed, 68 insertions(+), 16 deletions(-) create mode 100644 tornado/test/autoreload_test.py diff --git a/tornado/autoreload.py b/tornado/autoreload.py index 24deb79d..ad6cec36 100644 --- a/tornado/autoreload.py +++ b/tornado/autoreload.py @@ -63,12 +63,11 @@ import sys # file.py gets added to the path, which can cause confusion as imports # may become relative in spite of the future import. # -# We address the former problem by setting the $PYTHONPATH environment -# variable before re-execution so the new process will see the correct -# path. We attempt to address the latter problem when tornado.autoreload -# is run as __main__, although we can't fix the general case because -# we cannot reliably reconstruct the original command line -# (http://bugs.python.org/issue14208). +# We address the former problem by reconstructing the original command +# line (Python >= 3.4) or by setting the $PYTHONPATH environment +# variable (Python < 3.4) before re-execution so the new process will +# see the correct path. We attempt to address the latter problem when +# tornado.autoreload is run as __main__. if __name__ == "__main__": # This sys.path manipulation must come before our imports (as much @@ -209,15 +208,22 @@ def _reload(): # ioloop.set_blocking_log_threshold so it doesn't fire # after the exec. signal.setitimer(signal.ITIMER_REAL, 0, 0) - # sys.path fixes: see comments at top of file. If sys.path[0] is an empty - # string, we were (probably) invoked with -m and the effective path - # is about to change on re-exec. Add the current directory to $PYTHONPATH - # to ensure that the new process sees the same path we did. - path_prefix = '.' + os.pathsep - if (sys.path[0] == '' and - not os.environ.get("PYTHONPATH", "").startswith(path_prefix)): - os.environ["PYTHONPATH"] = (path_prefix + - os.environ.get("PYTHONPATH", "")) + # sys.path fixes: see comments at top of file. If __main__.__spec__ + # exists, we were invoked with -m and the effective path is about to + # change on re-exec. Reconstruct the original command line to + # ensure that the new process sees the same path we did. If + # __spec__ is not available (Python < 3.4), check instead if + # sys.path[0] is an empty string and add the current directory to + # $PYTHONPATH. + spec = getattr(sys.modules['__main__'], '__spec__', None) + if spec: + sys.argv = ['-m', spec.name] + sys.argv[1:] + else: + path_prefix = '.' + os.pathsep + if (sys.path[0] == '' and + not os.environ.get("PYTHONPATH", "").startswith(path_prefix)): + os.environ["PYTHONPATH"] = (path_prefix + + os.environ.get("PYTHONPATH", "")) if not _has_execv: subprocess.Popen([sys.executable] + sys.argv) sys.exit(0) diff --git a/tornado/test/autoreload_test.py b/tornado/test/autoreload_test.py new file mode 100644 index 00000000..b0390a19 --- /dev/null +++ b/tornado/test/autoreload_test.py @@ -0,0 +1,46 @@ +from __future__ import absolute_import, division, print_function +import os +import subprocess +from subprocess import Popen +import sys +from tempfile import mkdtemp + +from tornado.test.util import unittest + + +MAIN = """\ +import os +import sys + +from tornado import autoreload + +# This import will fail if path is not set up correctly +import testapp + +print('Starting') +if 'TESTAPP_STARTED' not in os.environ: + os.environ['TESTAPP_STARTED'] = '1' + sys.stdout.flush() + autoreload._reload() +""" + + +class AutoreloadTest(unittest.TestCase): + def test_reload_module(self): + # Create temporary test application + path = mkdtemp() + os.mkdir(os.path.join(path, 'testapp')) + open(os.path.join(path, 'testapp/__init__.py'), 'w').close() + with open(os.path.join(path, 'testapp/__main__.py'), 'w') as f: + f.write(MAIN) + + # Make sure the tornado module under test is available to the test + # application + pythonpath = os.getcwd() + if 'PYTHONPATH' in os.environ: + pythonpath += os.pathsep + os.environ['PYTHONPATH'] + + p = Popen([sys.executable, '-m', 'testapp'], stdout=subprocess.PIPE, + cwd=path, env=dict(os.environ, PYTHONPATH=pythonpath)) + out = p.communicate()[0].decode() + self.assertEqual(out, 'Starting\nStarting\n') diff --git a/tornado/test/import_test.py b/tornado/test/import_test.py index 88d02e02..e5eaec33 100644 --- a/tornado/test/import_test.py +++ b/tornado/test/import_test.py @@ -9,7 +9,6 @@ class ImportTest(unittest.TestCase): # all (unless they have external dependencies) here to at # least ensure that there are no syntax errors. import tornado.auth - import tornado.autoreload import tornado.concurrent import tornado.escape import tornado.gen diff --git a/tornado/test/runtests.py b/tornado/test/runtests.py index b81c5f22..942ce59b 100644 --- a/tornado/test/runtests.py +++ b/tornado/test/runtests.py @@ -25,6 +25,7 @@ TEST_MODULES = [ 'tornado.util.doctests', 'tornado.test.asyncio_test', 'tornado.test.auth_test', + 'tornado.test.autoreload_test', 'tornado.test.concurrent_test', 'tornado.test.curl_httpclient_test', 'tornado.test.escape_test',