Skip to content

Commit

Permalink
bpo-29723: Consistently configure sys.path[0] (python#575)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ncoghlan authored Mar 12, 2017
1 parent 6a6d090 commit d2977a3
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 28 deletions.
67 changes: 67 additions & 0 deletions Lib/test/test_cmd_line_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
72 changes: 44 additions & 28 deletions Modules/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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)) &&
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit d2977a3

Please sign in to comment.