From 17cb7dc659aec4c1290cddbfb03eb3383cf6246f Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Thu, 9 Mar 2017 14:49:48 +1000 Subject: [PATCH 1/3] WIP: bpo-29723: Set sys.path[0] correctly for importer-as-main --- Lib/test/test_cmd_line_script.py | 29 +++++++++++++ Modules/main.c | 72 +++++++++++++++++++------------- 2 files changed, 73 insertions(+), 28 deletions(-) diff --git a/Lib/test/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py index e058ecd086df3c..7274f065ce7e1b 100644 --- a/Lib/test/test_cmd_line_script.py +++ b/Lib/test/test_cmd_line_script.py @@ -572,6 +572,35 @@ def test_syntaxerror_indented_caret_position(self): self.assertNotIn("\f", text) self.assertIn("\n 1 + 1 = 2\n ^", text) + def test_issue29723_consistent_sys_path_for_directory_execution(self): + # This exercises a code path that is common to all components that + # are valid sys.path entries, so it implicitly covers zipfiles as well + 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 site.py to align with the isolated mode check later + p = spawn_python("-S", 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("-S", 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) + + with support.temp_dir() as script_dir: + script_name = _make_test_script(script_dir, '__main__') def test_main(): support.run_unittest(CmdLineTest) diff --git a/Modules/main.c b/Modules/main.c index 2e6a60b1673f88..dd502119d985e9 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; From 6dc2f226608965380ef1173fea048d3acb4cfe22 Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Thu, 9 Mar 2017 15:19:30 +1000 Subject: [PATCH 2/3] Adjust test case to also cover package execution --- Lib/test/test_cmd_line_script.py | 47 ++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py index 7274f065ce7e1b..99b55051730188 100644 --- a/Lib/test/test_cmd_line_script.py +++ b/Lib/test/test_cmd_line_script.py @@ -572,9 +572,13 @@ def test_syntaxerror_indented_caret_position(self): self.assertNotIn("\f", text) self.assertIn("\n 1 + 1 = 2\n ^", text) - def test_issue29723_consistent_sys_path_for_directory_execution(self): - # This exercises a code path that is common to all components that - # are valid sys.path entries, so it implicitly covers zipfiles as well + 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: @@ -599,8 +603,41 @@ def test_issue29723_consistent_sys_path_for_directory_execution(self): out_by_dir_isolated = kill_python(p).decode().splitlines() self.assertEqual(out_by_dir_isolated, out_by_dir, out_by_name) - with support.temp_dir() as script_dir: - script_name = _make_test_script(script_dir, '__main__') + 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 site.py as that's independent of __main__ execution + 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) From 7ad27cb53891d5b4fb751fc227a70a7aa7843fea Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Thu, 9 Mar 2017 16:06:31 +1000 Subject: [PATCH 3/3] Correctly align sys.path with isolated mode --- Lib/test/test_cmd_line_script.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py index 99b55051730188..1587daf8f582dd 100644 --- a/Lib/test/test_cmd_line_script.py +++ b/Lib/test/test_cmd_line_script.py @@ -576,8 +576,8 @@ 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 -s script_dir/__main__.py + # ./python -s script_dir # ./python -I script_dir script = textwrap.dedent("""\ import sys @@ -589,13 +589,13 @@ def test_consistent_sys_path_for_direct_execution(self): 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 site.py to align with the isolated mode check later - p = spawn_python("-S", script_name, cwd=work_dir) + # 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("-S", script_dir, cwd=work_dir) + 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 @@ -606,8 +606,8 @@ def test_consistent_sys_path_for_direct_execution(self): 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 + # ./python -sm script_pkg.__main__ + # ./python -sm script_pkg # # And that this fails as unable to find the package: # ./python -Im script_pkg @@ -623,13 +623,14 @@ def test_consistent_sys_path_for_module_execution(self): os.mkdir(script_dir) script_name = _make_test_script(script_dir, '__main__', script) # Reference output comes from `-m script_pkg.__main__` - # We omit site.py as that's independent of __main__ execution - p = spawn_python("-Sm", "script_pkg.__main__", cwd=work_dir) + # 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) + 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