From a0f9c21af8572383b2b56702f107f49f4d3db3cb Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Thu, 9 Mar 2017 16:18:07 +1000 Subject: [PATCH] bpo-29723: Consistently configure sys.path[0] Directory and zipfile execution previously added the parent directory of the directory or zipfile as sys.path[0] and then subsequently overwrote it with the directory or zipfile itself. This caused problems in isolated mode, as it overwrote the "stdlib as a zip archive" entry in sys.path, as the parent directory was never added. The attempted fix to that issue in bpo-29319 created the opposite problem in *non*-isolated mode, by potentially leaving the parent directory on sys.path instead of overwriting it. This change fixes the root cause of the problem by removing the whole "add-and-overwrite" dance for sys.path[0], and instead simply never adds the parent directory to sys.path in the first place. --- Lib/test/test_cmd_line_script.py | 67 +++++++++++++++++++++++++++++ Modules/main.c | 72 +++++++++++++++++++------------- 2 files changed, 111 insertions(+), 28 deletions(-) diff --git a/Lib/test/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py index e058ecd086df3c..1587daf8f582dd 100644 --- a/Lib/test/test_cmd_line_script.py +++ b/Lib/test/test_cmd_line_script.py @@ -572,6 +572,73 @@ def test_syntaxerror_indented_caret_position(self): self.assertNotIn("\f", text) self.assertIn("\n 1 + 1 = 2\n ^", text) + def test_consistent_sys_path_for_direct_execution(self): + # This test case ensures that the following all give the same + # sys.path configuration: + # + # ./python -s script_dir/__main__.py + # ./python -s script_dir + # ./python -I script_dir + script = textwrap.dedent("""\ + import sys + for entry in sys.path: + print(entry) + """) + # Always show full path diffs on errors + self.maxDiff = None + with support.temp_dir() as work_dir, support.temp_dir() as script_dir: + script_name = _make_test_script(script_dir, '__main__', script) + # Reference output comes from directly executing __main__.py + # We omit PYTHONPATH and user site to align with isolated mode + p = spawn_python("-Es", script_name, cwd=work_dir) + out_by_name = kill_python(p).decode().splitlines() + self.assertEqual(out_by_name[0], script_dir) + self.assertNotIn(work_dir, out_by_name) + # Directory execution should give the same output + p = spawn_python("-Es", script_dir, cwd=work_dir) + out_by_dir = kill_python(p).decode().splitlines() + self.assertEqual(out_by_dir, out_by_name) + # As should directory execution in isolated mode + p = spawn_python("-I", script_dir, cwd=work_dir) + out_by_dir_isolated = kill_python(p).decode().splitlines() + self.assertEqual(out_by_dir_isolated, out_by_dir, out_by_name) + + def test_consistent_sys_path_for_module_execution(self): + # This test case ensures that the following both give the same + # sys.path configuration: + # ./python -sm script_pkg.__main__ + # ./python -sm script_pkg + # + # And that this fails as unable to find the package: + # ./python -Im script_pkg + script = textwrap.dedent("""\ + import sys + for entry in sys.path: + print(entry) + """) + # Always show full path diffs on errors + self.maxDiff = None + with support.temp_dir() as work_dir: + script_dir = os.path.join(work_dir, "script_pkg") + os.mkdir(script_dir) + script_name = _make_test_script(script_dir, '__main__', script) + # Reference output comes from `-m script_pkg.__main__` + # We omit PYTHONPATH and user site to better align with the + # 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.assertNotIn(script_dir, out_by_module) + # Package execution should give the same output + p = spawn_python("-sm", "script_pkg", cwd=work_dir) + out_by_package = kill_python(p).decode().splitlines() + self.assertEqual(out_by_package, out_by_module) + # Isolated mode should fail with an import error + exitcode, stdout, stderr = assert_python_failure( + "-Im", "script_pkg", cwd=work_dir + ) + traceback_lines = stderr.decode().splitlines() + self.assertIn("No module named script_pkg", traceback_lines[-1]) def test_main(): support.run_unittest(CmdLineTest) diff --git a/Modules/main.c b/Modules/main.c index 633e7576910b84..34928e4b5f37eb 100644 --- a/Modules/main.c +++ b/Modules/main.c @@ -225,55 +225,60 @@ static int RunModule(wchar_t *modname, int set_argv0) return 0; } -static int -RunMainFromImporter(wchar_t *filename) +static PyObject * +AsImportPathEntry(wchar_t *filename) { - PyObject *argv0 = NULL, *importer, *sys_path, *sys_path0; - int sts; + PyObject *sys_path0 = NULL, *importer; - argv0 = PyUnicode_FromWideChar(filename, wcslen(filename)); - if (argv0 == NULL) + sys_path0 = PyUnicode_FromWideChar(filename, wcslen(filename)); + if (sys_path0 == NULL) goto error; - importer = PyImport_GetImporter(argv0); + importer = PyImport_GetImporter(sys_path0); if (importer == NULL) goto error; if (importer == Py_None) { - Py_DECREF(argv0); + Py_DECREF(sys_path0); Py_DECREF(importer); - return -1; + return NULL; } Py_DECREF(importer); + return sys_path0; + +error: + Py_XDECREF(sys_path0); + PySys_WriteStderr("Failed checking if argv[0] is an import path entry\n"); + PyErr_Print(); + PyErr_Clear(); + return NULL; +} + + +static int +RunMainFromImporter(PyObject *sys_path0) +{ + PyObject *sys_path; + int sts; - /* argv0 is usable as an import source, so put it in sys.path[0] - and import __main__ */ + /* Assume sys_path0 has already been checked by AsImportPathEntry, + * so put it in sys.path[0] and import __main__ */ sys_path = PySys_GetObject("path"); if (sys_path == NULL) { PyErr_SetString(PyExc_RuntimeError, "unable to get sys.path"); goto error; } - sys_path0 = PyList_GetItem(sys_path, 0); - sts = 0; - if (!sys_path0) { - PyErr_Clear(); - sts = PyList_Append(sys_path, argv0); - } else if (PyObject_IsTrue(sys_path0)) { - sts = PyList_Insert(sys_path, 0, argv0); - } else { - sts = PyList_SetItem(sys_path, 0, argv0); - } + sts = PyList_Insert(sys_path, 0, sys_path0); if (sts) { - argv0 = NULL; + sys_path0 = NULL; goto error; } - Py_INCREF(argv0); sts = RunModule(L"__main__", 0); return sts != 0; error: - Py_XDECREF(argv0); + Py_XDECREF(sys_path0); PyErr_Print(); return 1; } @@ -358,6 +363,7 @@ Py_Main(int argc, wchar_t **argv) int saw_unbuffered_flag = 0; char *opt; PyCompilerFlags cf; + PyObject *main_importer_path = NULL; PyObject *warning_option = NULL; PyObject *warning_options = NULL; @@ -714,7 +720,17 @@ Py_Main(int argc, wchar_t **argv) argv[_PyOS_optind] = L"-m"; } - PySys_SetArgv(argc-_PyOS_optind, argv+_PyOS_optind); + if (filename != NULL) { + main_importer_path = AsImportPathEntry(filename); + } + + if (main_importer_path != NULL) { + /* Let RunMainFromImporter adjust sys.path[0] later */ + PySys_SetArgvEx(argc-_PyOS_optind, argv+_PyOS_optind, 0); + } else { + /* Use config settings to decide whether or not to update sys.path[0] */ + PySys_SetArgv(argc-_PyOS_optind, argv+_PyOS_optind); + } if ((Py_InspectFlag || (command == NULL && filename == NULL && module == NULL)) && isatty(fileno(stdin)) && @@ -744,11 +760,11 @@ Py_Main(int argc, wchar_t **argv) sts = -1; /* keep track of whether we've already run __main__ */ - if (filename != NULL) { - sts = RunMainFromImporter(filename); + if (main_importer_path != NULL) { + sts = RunMainFromImporter(main_importer_path); } - if (sts==-1 && filename!=NULL) { + if (sts==-1 && filename != NULL) { fp = _Py_wfopen(filename, L"r"); if (fp == NULL) { char *cfilename_buffer;