Merge pull request #3302 from bdarnell/autoreload-updates

autoreload: Various updates

tornado/autoreload.py

@@ -60,8 +60,7 @@
 # may become relative in spite of the future import.
 #
 # 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
+# line 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__.
 
@@ -76,8 +75,9 @@
         del sys.path[0]
 
 import functools
+import importlib.abc
 import os
-import pkgutil  # type: ignore
+import pkgutil
 import sys
 import traceback
 import types
@@ -87,18 +87,13 @@
 from tornado import ioloop
 from tornado.log import gen_log
 from tornado import process
-from tornado.util import exec_in
 
 try:
     import signal
 except ImportError:
     signal = None  # type: ignore
 
-import typing
-from typing import Callable, Dict
-
-if typing.TYPE_CHECKING:
-    from typing import List, Optional, Union  # noqa: F401
+from typing import Callable, Dict, Optional, List, Union
 
 # os.execv is broken on Windows and can't properly parse command line
 # arguments and executable name if they contain whitespaces. subprocess
@@ -108,9 +103,11 @@
 _watched_files = set()
 _reload_hooks = []
 _reload_attempted = False
-_io_loops = weakref.WeakKeyDictionary()  # type: ignore
+_io_loops: "weakref.WeakKeyDictionary[ioloop.IOLoop, bool]" = (
+    weakref.WeakKeyDictionary()
+)
 _autoreload_is_main = False
-_original_argv = None  # type: Optional[List[str]]
+_original_argv: Optional[List[str]] = None
 _original_spec = None
 
 
@@ -126,7 +123,7 @@ def start(check_time: int = 500) -> None:
     _io_loops[io_loop] = True
     if len(_io_loops) > 1:
         gen_log.warning("tornado.autoreload started more than once in the same process")
-    modify_times = {}  # type: Dict[str, float]
+    modify_times: Dict[str, float] = {}
     callback = functools.partial(_reload_on_update, modify_times)
     scheduler = ioloop.PeriodicCallback(callback, check_time)
     scheduler.start()
@@ -214,54 +211,33 @@ def _reload() -> None:
     # 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.
+    # ensure that the new process sees the same path we did.
     if _autoreload_is_main:
         assert _original_argv is not None
         spec = _original_spec
         argv = _original_argv
     else:
         spec = getattr(sys.modules["__main__"], "__spec__", None)
         argv = sys.argv
-    if spec:
+    if spec and spec.name != "__main__":
+        # __spec__ is set in two cases: when running a module, and when running a directory. (when
+        # running a file, there is no spec). In the former case, we must pass -m to maintain the
+        # module-style behavior (setting sys.path), even though python stripped -m from its argv at
+        # startup. If sys.path is exactly __main__, we're running a directory and should fall
+        # through to the non-module behavior.
+        #
+        # Some of this, including the use of exactly __main__ as a spec for directory mode,
+        # is documented at https://docs.python.org/3/library/runpy.html#runpy.run_path
         argv = ["-m", spec.name] + 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] + argv)
         os._exit(0)
     else:
-        try:
-            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
-            # re-executing in the current process, start a new one
-            # and cause the current process to exit.  This isn't
-            # ideal since the new process is detached from the parent
-            # terminal and thus cannot easily be killed with ctrl-C,
-            # but it's better than not being able to autoreload at
-            # all.
-            # 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] + argv  # type: ignore
-            )
-            # 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.
-            os._exit(0)
-
-
-_USAGE = """\
-Usage:
+        os.execv(sys.executable, [sys.executable] + argv)
+
+
+_USAGE = """
   python -m tornado.autoreload -m module.to.run [args...]
   python -m tornado.autoreload path/to/script.py [args...]
 """
@@ -283,6 +259,12 @@ def main() -> None:
     # Remember that we were launched with autoreload as main.
     # The main module can be tricky; set the variables both in our globals
     # (which may be __main__) and the real importable version.
+    #
+    # We use optparse instead of the newer argparse because we want to
+    # mimic the python command-line interface which requires stopping
+    # parsing at the first positional argument. optparse supports
+    # this but as far as I can tell argparse does not.
+    import optparse
     import tornado.autoreload
 
     global _autoreload_is_main
@@ -292,46 +274,51 @@ def main() -> None:
     tornado.autoreload._original_argv = _original_argv = original_argv
     original_spec = getattr(sys.modules["__main__"], "__spec__", None)
     tornado.autoreload._original_spec = _original_spec = original_spec
-    sys.argv = sys.argv[:]
-    if len(sys.argv) >= 3 and sys.argv[1] == "-m":
-        mode = "module"
-        module = sys.argv[2]
-        del sys.argv[1:3]
-    elif len(sys.argv) >= 2:
-        mode = "script"
-        script = sys.argv[1]
-        sys.argv = sys.argv[1:]
+
+    parser = optparse.OptionParser(
+        prog="python -m tornado.autoreload",
+        usage=_USAGE,
+        epilog="Either -m or a path must be specified, but not both",
+    )
+    parser.disable_interspersed_args()
+    parser.add_option("-m", dest="module", metavar="module", help="module to run")
+    parser.add_option(
+        "--until-success",
+        action="store_true",
+        help="stop reloading after the program exist successfully (status code 0)",
+    )
+    opts, rest = parser.parse_args()
+    if opts.module is None:
+        if not rest:
+            print("Either -m or a path must be specified", file=sys.stderr)
+            sys.exit(1)
+        path = rest[0]
+        sys.argv = rest[:]
     else:
-        print(_USAGE, file=sys.stderr)
-        sys.exit(1)
+        path = None
+        sys.argv = [sys.argv[0]] + rest
 
+    # SystemExit.code is typed funny: https://github.com/python/typeshed/issues/8513
+    # All we care about is truthiness
+    exit_status: Union[int, str, None] = 1
     try:
-        if mode == "module":
-            import runpy
-
-            runpy.run_module(module, run_name="__main__", alter_sys=True)
-        elif mode == "script":
-            with open(script) as f:
-                # Execute the script in our namespace instead of creating
-                # a new one so that something that tries to import __main__
-                # (e.g. the unittest module) will see names defined in the
-                # script instead of just those defined in this module.
-                global __file__
-                __file__ = script
-                # If __package__ is defined, imports may be incorrectly
-                # interpreted as relative to this module.
-                global __package__
-                del __package__
-                exec_in(f.read(), globals(), globals())
+        import runpy
+
+        if opts.module is not None:
+            runpy.run_module(opts.module, run_name="__main__", alter_sys=True)
+        else:
+            assert path is not None
+            runpy.run_path(path, run_name="__main__")
     except SystemExit as e:
+        exit_status = e.code
         gen_log.info("Script exited with status %s", e.code)
     except Exception as e:
         gen_log.warning("Script exited with uncaught exception", exc_info=True)
         # If an exception occurred at import time, the file with the error
         # never made it into sys.modules and so we won't know to watch it.
         # Just to make sure we've covered everything, walk the stack trace
         # from the exception and watch every file.
-        for (filename, lineno, name, line) in traceback.extract_tb(sys.exc_info()[2]):
+        for filename, lineno, name, line in traceback.extract_tb(sys.exc_info()[2]):
             watch(filename)
         if isinstance(e, SyntaxError):
             # SyntaxErrors are special:  their innermost stack frame is fake
@@ -340,17 +327,22 @@ def main() -> None:
             if e.filename is not None:
                 watch(e.filename)
     else:
+        exit_status = 0
         gen_log.info("Script exited normally")
     # restore sys.argv so subsequent executions will include autoreload
     sys.argv = original_argv
 
-    if mode == "module":
+    if opts.module is not None:
+        assert opts.module is not None
         # runpy did a fake import of the module as __main__, but now it's
         # no longer in sys.modules.  Figure out where it is and watch it.
-        loader = pkgutil.get_loader(module)
-        if loader is not None:
+        loader = pkgutil.get_loader(opts.module)
+        if loader is not None and isinstance(loader, importlib.abc.FileLoader):
+            # TODO: fix when we update typeshed
             watch(loader.get_filename())  # type: ignore
 
+    if opts.until_success and not exit_status:
+        return
     wait()