bpo-33053: -m now adds *starting* directory to sys.path (GH-6231)

ncoghlan · web-flow · commit d5d9e02dd3c6 · 2018-03-25T23:03:10.000+10:00
Historically, -m added the empty string as sys.path
zero, meaning it resolved imports against the current
working directory, the same way -c and the interactive
prompt do.

This changes the sys.path initialisation to add the
*starting* working directory as sys.path[0] instead,
such that changes to the working directory while the
program is running will have no effect on imports
when using the -m switch.
diff --git a/Doc/library/test.rst b/Doc/library/test.rst
@@ -1332,8 +1332,8 @@ script execution tests.
 .. function:: run_python_until_end(*args, **env_vars)
 
    Set up the environment based on *env_vars* for running the interpreter
-   in a subprocess.  The values can include ``__isolated``, ``__cleavenv``,
-   and ``TERM``.
+   in a subprocess.  The values can include ``__isolated``, ``__cleanenv``,
+   ``__cwd``, and ``TERM``.
 
 
 .. function:: assert_python_ok(*args, **env_vars)
diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst
@@ -421,6 +421,12 @@ Other Language Changes
   writable.
   (Contributed by Nathaniel J. Smith in :issue:`30579`.)
 
+* When using the :option:`-m` switch, ``sys.path[0]`` is now eagerly expanded
+  to the full starting directory path, rather than being left as the empty
+  directory (which allows imports from the *current* working directory at the
+  time when an import occurs)
+  (Contributed by Nick Coghlan in :issue:`33053`.)
+
 
 New Modules
 ===========
@@ -1138,6 +1144,11 @@ Changes in Python behavior
   parentheses can be omitted only on calls.
   (Contributed by Serhiy Storchaka in :issue:`32012` and :issue:`32023`.)
 
+* When using the ``-m`` switch, the starting directory is now added to sys.path,
+  rather than the current working directory. Any programs that are found to be
+  relying on the previous behaviour will need to be updated to manipulate
+  :data:`sys.path` appropriately.
+
 
 Changes in the Python API
 -------------------------
diff --git a/Lib/test/support/script_helper.py b/Lib/test/support/script_helper.py
@@ -87,6 +87,7 @@ def fail(self, cmd_line):
 # Executing the interpreter in a subprocess
 def run_python_until_end(*args, **env_vars):
     env_required = interpreter_requires_environment()
+    cwd = env_vars.pop('__cwd', None)
     if '__isolated' in env_vars:
         isolated = env_vars.pop('__isolated')
     else:
@@ -125,7 +126,7 @@ def run_python_until_end(*args, **env_vars):
     cmd_line.extend(args)
     proc = subprocess.Popen(cmd_line, stdin=subprocess.PIPE,
                          stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                         env=env)
+                         env=env, cwd=cwd)
     with proc:
         try:
             out, err = proc.communicate()
diff --git a/Lib/test/test_bdb.py b/Lib/test/test_bdb.py
@@ -524,13 +524,13 @@ def gen(a, b):
     test.id = lambda : None
     test.expect_set = list(gen(repeat(()), iter(sl)))
     with create_modules(modules):
-        sys.path.append(os.getcwd())
         with TracerRun(test, skip=skip) as tracer:
             tracer.runcall(tfunc_import)
 
 @contextmanager
 def create_modules(modules):
     with test.support.temp_cwd():
+        sys.path.append(os.getcwd())
         try:
             for m in modules:
                 fname = m + '.py'
@@ -542,6 +542,7 @@ def create_modules(modules):
         finally:
             for m in modules:
                 test.support.forget(m)
+            sys.path.pop()
 
 def break_in_func(funcname, fname=__file__, temporary=False, cond=None):
     return 'break', (fname, None, temporary, cond, funcname)
diff --git a/Lib/test/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py
@@ -87,31 +87,11 @@ def _make_test_zip_pkg(zip_dir, zip_basename, pkg_name, script_basename,
     importlib.invalidate_caches()
     return to_return
 
-# There's no easy way to pass the script directory in to get
-# -m to work (avoiding that is the whole point of making
-# directories and zipfiles executable!)
-# So we fake it for testing purposes with a custom launch script
-launch_source = """\
-import sys, os.path, runpy
-sys.path.insert(0, %s)
-runpy._run_module_as_main(%r)
-"""
-
-def _make_launch_script(script_dir, script_basename, module_name, path=None):
-    if path is None:
-        path = "os.path.dirname(__file__)"
-    else:
-        path = repr(path)
-    source = launch_source % (path, module_name)
-    to_return = make_script(script_dir, script_basename, source)
-    importlib.invalidate_caches()
-    return to_return
-
 class CmdLineTest(unittest.TestCase):
     def _check_output(self, script_name, exit_code, data,
                              expected_file, expected_argv0,
                              expected_path0, expected_package,
-                             expected_loader):
+                             expected_loader, expected_cwd=None):
         if verbose > 1:
             print("Output from test script %r:" % script_name)
             print(repr(data))
@@ -121,7 +101,9 @@ def _check_output(self, script_name, exit_code, data,
         printed_package = '__package__==%r' % expected_package
         printed_argv0 = 'sys.argv[0]==%a' % expected_argv0
         printed_path0 = 'sys.path[0]==%a' % expected_path0
-        printed_cwd = 'cwd==%a' % os.getcwd()
+        if expected_cwd is None:
+            expected_cwd = os.getcwd()
+        printed_cwd = 'cwd==%a' % expected_cwd
         if verbose > 1:
             print('Expected output:')
             print(printed_file)
@@ -135,23 +117,35 @@ def _check_output(self, script_name, exit_code, data,
         self.assertIn(printed_path0.encode('utf-8'), data)
         self.assertIn(printed_cwd.encode('utf-8'), data)
 
-    def _check_script(self, script_name, expected_file,
+    def _check_script(self, script_exec_args, expected_file,
                             expected_argv0, expected_path0,
                             expected_package, expected_loader,
-                            *cmd_line_switches):
+                            *cmd_line_switches, cwd=None, **env_vars):
+        if isinstance(script_exec_args, str):
+            script_exec_args = [script_exec_args]
         run_args = [*support.optim_args_from_interpreter_flags(),
-                    *cmd_line_switches, script_name, *example_args]
-        rc, out, err = assert_python_ok(*run_args, __isolated=False)
-        self._check_output(script_name, rc, out + err, expected_file,
+                    *cmd_line_switches, *script_exec_args, *example_args]
+        if env_vars:
+            print(env_vars)
+        rc, out, err = assert_python_ok(
+            *run_args, __isolated=False, __cwd=cwd, **env_vars
+        )
+        self._check_output(script_exec_args, rc, out + err, expected_file,
                            expected_argv0, expected_path0,
-                           expected_package, expected_loader)
+                           expected_package, expected_loader, cwd)
 
-    def _check_import_error(self, script_name, expected_msg,
-                            *cmd_line_switches):
-        run_args = cmd_line_switches + (script_name,)
-        rc, out, err = assert_python_failure(*run_args)
+    def _check_import_error(self, script_exec_args, expected_msg,
+                            *cmd_line_switches, cwd=None, **env_vars):
+        if isinstance(script_exec_args, str):
+            script_exec_args = (script_exec_args,)
+        else:
+            script_exec_args = tuple(script_exec_args)
+        run_args = cmd_line_switches + script_exec_args
+        rc, out, err = assert_python_failure(
+            *run_args, __isolated=False, __cwd=cwd, **env_vars
+        )
         if verbose > 1:
-            print('Output from test script %r:' % script_name)
+            print('Output from test script %r:' % script_exec_args)
             print(repr(err))
             print('Expected output: %r' % expected_msg)
         self.assertIn(expected_msg.encode('utf-8'), err)
@@ -287,35 +281,35 @@ def test_module_in_package(self):
             pkg_dir = os.path.join(script_dir, 'test_pkg')
             make_pkg(pkg_dir)
             script_name = _make_test_script(pkg_dir, 'script')
-            launch_name = _make_launch_script(script_dir, 'launch', 'test_pkg.script')
-            self._check_script(launch_name, script_name, script_name,
+            self._check_script(["-m", "test_pkg.script"], script_name, script_name,
                                script_dir, 'test_pkg',
-                               importlib.machinery.SourceFileLoader)
+                               importlib.machinery.SourceFileLoader,
+                               cwd=script_dir)
 
     def test_module_in_package_in_zipfile(self):
         with support.temp_dir() as script_dir:
             zip_name, run_name = _make_test_zip_pkg(script_dir, 'test_zip', 'test_pkg', 'script')
-            launch_name = _make_launch_script(script_dir, 'launch', 'test_pkg.script', zip_name)
-            self._check_script(launch_name, run_name, run_name,
-                               zip_name, 'test_pkg', zipimport.zipimporter)
+            self._check_script(["-m", "test_pkg.script"], run_name, run_name,
+                               script_dir, 'test_pkg', zipimport.zipimporter,
+                               PYTHONPATH=zip_name, cwd=script_dir)
 
     def test_module_in_subpackage_in_zipfile(self):
         with support.temp_dir() as script_dir:
             zip_name, run_name = _make_test_zip_pkg(script_dir, 'test_zip', 'test_pkg', 'script', depth=2)
-            launch_name = _make_launch_script(script_dir, 'launch', 'test_pkg.test_pkg.script', zip_name)
-            self._check_script(launch_name, run_name, run_name,
-                               zip_name, 'test_pkg.test_pkg',
-                               zipimport.zipimporter)
+            self._check_script(["-m", "test_pkg.test_pkg.script"], run_name, run_name,
+                               script_dir, 'test_pkg.test_pkg',
+                               zipimport.zipimporter,
+                               PYTHONPATH=zip_name, cwd=script_dir)
 
     def test_package(self):
         with support.temp_dir() as script_dir:
             pkg_dir = os.path.join(script_dir, 'test_pkg')
             make_pkg(pkg_dir)
             script_name = _make_test_script(pkg_dir, '__main__')
-            launch_name = _make_launch_script(script_dir, 'launch', 'test_pkg')
-            self._check_script(launch_name, script_name,
+            self._check_script(["-m", "test_pkg"], script_name,
                                script_name, script_dir, 'test_pkg',
-                               importlib.machinery.SourceFileLoader)
+                               importlib.machinery.SourceFileLoader,
+                               cwd=script_dir)
 
     def test_package_compiled(self):
         with support.temp_dir() as script_dir:
@@ -325,19 +319,18 @@ def test_package_compiled(self):
             compiled_name = py_compile.compile(script_name, doraise=True)
             os.remove(script_name)
             pyc_file = support.make_legacy_pyc(script_name)
-            launch_name = _make_launch_script(script_dir, 'launch', 'test_pkg')
-            self._check_script(launch_name, pyc_file,
+            self._check_script(["-m", "test_pkg"], pyc_file,
                                pyc_file, script_dir, 'test_pkg',
-                               importlib.machinery.SourcelessFileLoader)
+                               importlib.machinery.SourcelessFileLoader,
+                               cwd=script_dir)
 
     def test_package_error(self):
         with support.temp_dir() as script_dir:
             pkg_dir = os.path.join(script_dir, 'test_pkg')
             make_pkg(pkg_dir)
             msg = ("'test_pkg' is a package and cannot "
                    "be directly executed")
-            launch_name = _make_launch_script(script_dir, 'launch', 'test_pkg')
-            self._check_import_error(launch_name, msg)
+            self._check_import_error(["-m", "test_pkg"], msg, cwd=script_dir)
 
     def test_package_recursion(self):
         with support.temp_dir() as script_dir:
@@ -348,8 +341,7 @@ def test_package_recursion(self):
             msg = ("Cannot use package as __main__ module; "
                    "'test_pkg' is a package and cannot "
                    "be directly executed")
-            launch_name = _make_launch_script(script_dir, 'launch', 'test_pkg')
-            self._check_import_error(launch_name, msg)
+            self._check_import_error(["-m", "test_pkg"], msg, cwd=script_dir)
 
     def test_issue8202(self):
         # Make sure package __init__ modules see "-m" in sys.argv0 while
@@ -365,7 +357,7 @@ def test_issue8202(self):
                 expected = "init_argv0==%r" % '-m'
                 self.assertIn(expected.encode('utf-8'), out)
                 self._check_output(script_name, rc, out,
-                                   script_name, script_name, '', 'test_pkg',
+                                   script_name, script_name, script_dir, 'test_pkg',
                                    importlib.machinery.SourceFileLoader)
 
     def test_issue8202_dash_c_file_ignored(self):
@@ -394,7 +386,7 @@ def test_issue8202_dash_m_file_ignored(self):
                     rc, out, err = assert_python_ok('-m', 'other', *example_args,
                                                     __isolated=False)
                     self._check_output(script_name, rc, out,
-                                      script_name, script_name, '', '',
+                                      script_name, script_name, script_dir, '',
                                       importlib.machinery.SourceFileLoader)
 
     @contextlib.contextmanager
@@ -627,7 +619,7 @@ def test_consistent_sys_path_for_module_execution(self):
             # direct execution test cases
             p = spawn_python("-sm", "script_pkg.__main__", cwd=work_dir)
             out_by_module = kill_python(p).decode().splitlines()
-            self.assertEqual(out_by_module[0], '')
+            self.assertEqual(out_by_module[0], work_dir)
             self.assertNotIn(script_dir, out_by_module)
             # Package execution should give the same output
             p = spawn_python("-sm", "script_pkg", cwd=work_dir)
diff --git a/Lib/test/test_doctest.py b/Lib/test/test_doctest.py
@@ -9,7 +9,7 @@
 import sys
 import importlib
 import unittest
-
+import tempfile
 
 # NOTE: There are some additional tests relating to interaction with
 #       zipimport in the test_zipimport_support test module.
@@ -688,10 +688,16 @@ class TestDocTestFinder(unittest.TestCase):
 
     def test_empty_namespace_package(self):
         pkg_name = 'doctest_empty_pkg'
-        os.mkdir(pkg_name)
-        mod = importlib.import_module(pkg_name)
-        assert doctest.DocTestFinder().find(mod) == []
-        os.rmdir(pkg_name)
+        with tempfile.TemporaryDirectory() as parent_dir:
+            pkg_dir = os.path.join(parent_dir, pkg_name)
+            os.mkdir(pkg_dir)
+            sys.path.append(parent_dir)
+            try:
+                mod = importlib.import_module(pkg_name)
+            finally:
+                support.forget(pkg_name)
+                sys.path.pop()
+            assert doctest.DocTestFinder().find(mod) == []
 
 
 def test_DocTestParser(): r"""
diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py
@@ -118,7 +118,7 @@ def test_from_import_star_invalid_type(self):
                 f.write("__all__ = [b'invalid_type']")
             globals = {}
             with self.assertRaisesRegex(
-                TypeError, f"{re.escape(name)}\.__all__ must be str"
+                TypeError, f"{re.escape(name)}\\.__all__ must be str"
             ):
                 exec(f"from {name} import *", globals)
             self.assertNotIn(b"invalid_type", globals)
@@ -127,7 +127,7 @@ def test_from_import_star_invalid_type(self):
                 f.write("globals()[b'invalid_type'] = object()")
             globals = {}
             with self.assertRaisesRegex(
-                TypeError, f"{re.escape(name)}\.__dict__ must be str"
+                TypeError, f"{re.escape(name)}\\.__dict__ must be str"
             ):
                 exec(f"from {name} import *", globals)
             self.assertNotIn(b"invalid_type", globals)
@@ -847,8 +847,11 @@ def test_missing_source_legacy(self):
         unload(TESTFN)
         importlib.invalidate_caches()
         m = __import__(TESTFN)
-        self.assertEqual(m.__file__,
-                         os.path.join(os.curdir, os.path.relpath(pyc_file)))
+        try:
+            self.assertEqual(m.__file__,
+                             os.path.join(os.curdir, os.path.relpath(pyc_file)))
+        finally:
+            os.remove(pyc_file)
 
     def test___cached__(self):
         # Modules now also have an __cached__ that points to the pyc file.
diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-03-25-19-49-06.bpo-33053.V3xlsH.rst b/Misc/NEWS.d/next/Core and Builtins/2018-03-25-19-49-06.bpo-33053.V3xlsH.rst
@@ -0,0 +1,4 @@
+When using the -m switch, sys.path[0] is now explicitly expanded as the
+*starting* working directory, rather than being left as the empty path
+(which allows imports from the current working directory at the time of the
+import)
diff --git a/Python/pathconfig.c b/Python/pathconfig.c
