Repository: tornadoweb/tornado (python)

Issue:
Autoreload fails for __main__ module with relative imports
Let's assume we have a Tornado app with the following structure

```
testapp/
  __init__.py
  __main__.py
  foo.py
  ...
```

where `__main__` contains a relative import like `from .foo import Bar`.

Running the app with `python -m testapp` will work as expected. If autoreload is enabled, a reload will however fail with the following exception:

```
SystemError: Parent module '' not loaded, cannot perform relative import
```

As explained in the source of autoreload, this happens because the command line arguments are transformed by Python from `-m testapp` to `testapp/__main__.py`. On reload, `__main__` will be run as a script, preventing relative imports.

According to http://bugs.python.org/msg289874 it is now possible to detect if `__main__` has been invoked with `-m` and reconstruct the original command line for reloading.

PS: Thanks for your awesome work on Tornado! :+1: 

Pull request:
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.

Patch:
diff --git a/tornado/autoreload.py b/tornado/autoreload.py
index 24deb79d..da1b100d 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,21 +208,29 @@ 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:
+        argv = ['-m', spec.name] + sys.argv[1:]
+    else:
+        argv = sys.argv
+        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)
+        subprocess.Popen([sys.executable] + argv)
         sys.exit(0)
     else:
         try:
-            os.execv(sys.executable, [sys.executable] + sys.argv)
+            os.execv(sys.executable, [sys.executable] + argv)
         except OSError:
             # Mac OS X versions prior to 10.6 do not support execv in
             # a process that contains multiple threads.  Instead of
@@ -236,8 +243,7 @@ def _reload():
             # Unfortunately the errno returned in this case does not
             # appear to be consistent, so we can't easily check for
             # this error specifically.
-            os.spawnv(os.P_NOWAIT, sys.executable,
-                      [sys.executable] + sys.argv)
+            os.spawnv(os.P_NOWAIT, sys.executable, [sys.executable] + argv)
             # At this point the IOLoop has been closed and finally
             # blocks will experience errors if we allow the stack to
             # unwind, so just exit uncleanly.


Test patch:
diff --git a/tornado/test/autoreload_test.py b/tornado/test/autoreload_test.py
new file mode 100644
index 00000000..6a9729db
--- /dev/null
+++ b/tornado/test/autoreload_test.py
@@ -0,0 +1,48 @@
+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),
+            universal_newlines=True)
+        out = p.communicate()[0]
+        self.assertEqual(out, 'Starting\nStarting\n')
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',