From d5d9e02dd3c6df06a8dd9ce75ee9b52976420a8b Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Sun, 25 Mar 2018 23:03:10 +1000 Subject: [PATCH] bpo-33053: -m now adds *starting* directory to sys.path (GH-6231) 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. --- Doc/library/test.rst | 4 +- Doc/whatsnew/3.7.rst | 11 ++ Lib/test/support/script_helper.py | 3 +- Lib/test/test_bdb.py | 3 +- Lib/test/test_cmd_line_script.py | 104 ++++++++---------- Lib/test/test_doctest.py | 16 ++- Lib/test/test_import/__init__.py | 11 +- .../2018-03-25-19-49-06.bpo-33053.V3xlsH.rst | 4 + Python/pathconfig.c | 30 +++-- 9 files changed, 108 insertions(+), 78 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-03-25-19-49-06.bpo-33053.V3xlsH.rst diff --git a/Doc/library/test.rst b/Doc/library/test.rst index 6041f529c16..0746fcfde0a 100644 --- 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 index 0b5ad007f30..e0c19cfa3bd 100644 --- 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 index 5a81697708f..64b25aab800 100644 --- a/Lib/test/support/script_helper.py +++ b/Lib/test/support/script_helper.py @@ -87,6 +87,7 @@ class _PythonRunResult(collections.namedtuple("_PythonRunResult", # 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 index abefe6c4e57..bda74a2be77 100644 --- a/Lib/test/test_bdb.py +++ b/Lib/test/test_bdb.py @@ -524,13 +524,13 @@ def run_test(modules, set_list, skip=None): 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 index 0d0bcd784d2..17620085500 100644 --- 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 @@ class CmdLineTest(unittest.TestCase): 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 @@ class CmdLineTest(unittest.TestCase): 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 @@ class CmdLineTest(unittest.TestCase): 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,10 +319,10 @@ class CmdLineTest(unittest.TestCase): 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: @@ -336,8 +330,7 @@ class CmdLineTest(unittest.TestCase): 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 @@ class CmdLineTest(unittest.TestCase): 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 @@ class CmdLineTest(unittest.TestCase): 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 @@ class CmdLineTest(unittest.TestCase): 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 @@ class CmdLineTest(unittest.TestCase): # 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 index f0eb52881bc..83941c129f4 100644 --- a/Lib/test/test_doctest.py +++ b/Lib/test/test_doctest.py @@ -9,7 +9,7 @@ import os 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 index 606b05784af..c23fac1ecc2 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -118,7 +118,7 @@ class ImportTests(unittest.TestCase): 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 @@ class ImportTests(unittest.TestCase): 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 @@ class PycacheTests(unittest.TestCase): 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 new file mode 100644 index 00000000000..fd32ac150e4 --- /dev/null +++ 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 index 6de5481bc8d..07aa01c0304 100644 --- a/Python/pathconfig.c +++ b/Python/pathconfig.c @@ -3,6 +3,7 @@ #include "Python.h" #include "osdefs.h" #include "internal/pystate.h" +#include #ifdef __cplusplus extern "C" { @@ -255,11 +256,6 @@ Py_GetProgramName(void) return _Py_path_config.program_name; } - -#define _HAVE_SCRIPT_ARGUMENT(argc, argv) \ - (argc > 0 && argv0 != NULL && \ - wcscmp(argv0, L"-c") != 0 && wcscmp(argv0, L"-m") != 0) - /* Compute argv[0] which will be prepended to sys.argv */ PyObject* _PyPathConfig_ComputeArgv0(int argc, wchar_t **argv) @@ -267,6 +263,8 @@ _PyPathConfig_ComputeArgv0(int argc, wchar_t **argv) wchar_t *argv0; wchar_t *p = NULL; Py_ssize_t n = 0; + int have_script_arg = 0; + int have_module_arg = 0; #ifdef HAVE_READLINK wchar_t link[MAXPATHLEN+1]; wchar_t argv0copy[2*MAXPATHLEN+1]; @@ -278,11 +276,25 @@ _PyPathConfig_ComputeArgv0(int argc, wchar_t **argv) wchar_t fullpath[MAX_PATH]; #endif - argv0 = argv[0]; + if (argc > 0 && argv0 != NULL) { + have_module_arg = (wcscmp(argv0, L"-m") == 0); + have_script_arg = !have_module_arg && (wcscmp(argv0, L"-c") != 0); + } + + if (have_module_arg) { + #if defined(HAVE_REALPATH) || defined(MS_WINDOWS) + _Py_wgetcwd(fullpath, Py_ARRAY_LENGTH(fullpath)); + argv0 = fullpath; + n = wcslen(argv0); + #else + argv0 = L"."; + n = 1; + #endif + } #ifdef HAVE_READLINK - if (_HAVE_SCRIPT_ARGUMENT(argc, argv)) + if (have_script_arg) nr = _Py_wreadlink(argv0, link, MAXPATHLEN); if (nr > 0) { /* It's a symlink */ @@ -310,7 +322,7 @@ _PyPathConfig_ComputeArgv0(int argc, wchar_t **argv) #if SEP == '\\' /* Special case for Microsoft filename syntax */ - if (_HAVE_SCRIPT_ARGUMENT(argc, argv)) { + if (have_script_arg) { wchar_t *q; #if defined(MS_WINDOWS) /* Replace the first element in argv with the full path. */ @@ -334,7 +346,7 @@ _PyPathConfig_ComputeArgv0(int argc, wchar_t **argv) } } #else /* All other filename syntaxes */ - if (_HAVE_SCRIPT_ARGUMENT(argc, argv)) { + if (have_script_arg) { #if defined(HAVE_REALPATH) if (_Py_wrealpath(argv0, fullpath, Py_ARRAY_LENGTH(fullpath))) { argv0 = fullpath;