gh-109853: Fix sys.path[0] For Subinterpreters (gh-109994)

ericsnowcurrently · web-flow · commit a040a32ea2f1 · 2023-10-02T19:59:05.000Z
This change makes sure sys.path[0] is set properly for subinterpreters. Before, it wasn't getting set at all. This PR does not address the broader concerns from gh-109853.
diff --git a/Include/cpython/initconfig.h b/Include/cpython/initconfig.h
@@ -204,6 +204,9 @@ typedef struct PyConfig {
     wchar_t *run_module;
     wchar_t *run_filename;
 
+    /* --- Set by Py_Main() -------------------------- */
+    wchar_t *sys_path_0;
+
     /* --- Private fields ---------------------------- */
 
     // Install importlib? If equals to 0, importlib is not initialized at all.
diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py
@@ -505,6 +505,7 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
         'run_command': None,
         'run_module': None,
         'run_filename': None,
+        'sys_path_0': None,
 
         '_install_importlib': 1,
         'check_hash_pycs_mode': 'default',
@@ -1132,6 +1133,7 @@ def test_init_run_main(self):
             'program_name': './python3',
             'run_command': code + '\n',
             'parse_argv': 2,
+            'sys_path_0': '',
         }
         self.check_all_configs("test_init_run_main", config, api=API_PYTHON)
 
@@ -1147,6 +1149,7 @@ def test_init_main(self):
             'run_command': code + '\n',
             'parse_argv': 2,
             '_init_main': 0,
+            'sys_path_0': '',
         }
         self.check_all_configs("test_init_main", config,
                                api=API_PYTHON,
diff --git a/Lib/test/test_interpreters.py b/Lib/test/test_interpreters.py
@@ -1,5 +1,7 @@
 import contextlib
+import json
 import os
+import os.path
 import sys
 import threading
 from textwrap import dedent
@@ -9,6 +11,7 @@
 from test import support
 from test.support import import_helper
 from test.support import threading_helper
+from test.support import os_helper
 _interpreters = import_helper.import_module('_xxsubinterpreters')
 _channels = import_helper.import_module('_xxinterpchannels')
 from test.support import interpreters
@@ -488,6 +491,154 @@ def task():
             pass
 
 
+class StartupTests(TestBase):
+
+    # We want to ensure the initial state of subinterpreters
+    # matches expectations.
+
+    _subtest_count = 0
+
+    @contextlib.contextmanager
+    def subTest(self, *args):
+        with super().subTest(*args) as ctx:
+            self._subtest_count += 1
+            try:
+                yield ctx
+            finally:
+                if self._debugged_in_subtest:
+                    if self._subtest_count == 1:
+                        # The first subtest adds a leading newline, so we
+                        # compensate here by not printing a trailing newline.
+                        print('### end subtest debug ###', end='')
+                    else:
+                        print('### end subtest debug ###')
+                self._debugged_in_subtest = False
+
+    def debug(self, msg, *, header=None):
+        if header:
+            self._debug(f'--- {header} ---')
+            if msg:
+                if msg.endswith(os.linesep):
+                    self._debug(msg[:-len(os.linesep)])
+                else:
+                    self._debug(msg)
+                    self._debug('<no newline>')
+            self._debug('------')
+        else:
+            self._debug(msg)
+
+    _debugged = False
+    _debugged_in_subtest = False
+    def _debug(self, msg):
+        if not self._debugged:
+            print()
+            self._debugged = True
+        if self._subtest is not None:
+            if True:
+                if not self._debugged_in_subtest:
+                    self._debugged_in_subtest = True
+                    print('### start subtest debug ###')
+                print(msg)
+        else:
+            print(msg)
+
+    def create_temp_dir(self):
+        import tempfile
+        tmp = tempfile.mkdtemp(prefix='test_interpreters_')
+        tmp = os.path.realpath(tmp)
+        self.addCleanup(os_helper.rmtree, tmp)
+        return tmp
+
+    def write_script(self, *path, text):
+        filename = os.path.join(*path)
+        dirname = os.path.dirname(filename)
+        if dirname:
+            os.makedirs(dirname, exist_ok=True)
+        with open(filename, 'w', encoding='utf-8') as outfile:
+            outfile.write(dedent(text))
+        return filename
+
+    @support.requires_subprocess()
+    def run_python(self, argv, *, cwd=None):
+        # This method is inspired by
+        # EmbeddingTestsMixin.run_embedded_interpreter() in test_embed.py.
+        import shlex
+        import subprocess
+        if isinstance(argv, str):
+            argv = shlex.split(argv)
+        argv = [sys.executable, *argv]
+        try:
+            proc = subprocess.run(
+                argv,
+                cwd=cwd,
+                capture_output=True,
+                text=True,
+            )
+        except Exception as exc:
+            self.debug(f'# cmd: {shlex.join(argv)}')
+            if isinstance(exc, FileNotFoundError) and not exc.filename:
+                if os.path.exists(argv[0]):
+                    exists = 'exists'
+                else:
+                    exists = 'does not exist'
+                self.debug(f'{argv[0]} {exists}')
+            raise  # re-raise
+        assert proc.stderr == '' or proc.returncode != 0, proc.stderr
+        if proc.returncode != 0 and support.verbose:
+            self.debug(f'# python3 {shlex.join(argv[1:])} failed:')
+            self.debug(proc.stdout, header='stdout')
+            self.debug(proc.stderr, header='stderr')
+        self.assertEqual(proc.returncode, 0)
+        self.assertEqual(proc.stderr, '')
+        return proc.stdout
+
+    def test_sys_path_0(self):
+        # The main interpreter's sys.path[0] should be used by subinterpreters.
+        script = '''
+            import sys
+            from test.support import interpreters
+
+            orig = sys.path[0]
+
+            interp = interpreters.create()
+            interp.run(f"""if True:
+                import json
+                import sys
+                print(json.dumps({{
+                    'main': {orig!r},
+                    'sub': sys.path[0],
+                }}, indent=4), flush=True)
+                """)
+            '''
+        # <tmp>/
+        #   pkg/
+        #     __init__.py
+        #     __main__.py
+        #     script.py
+        #   script.py
+        cwd = self.create_temp_dir()
+        self.write_script(cwd, 'pkg', '__init__.py', text='')
+        self.write_script(cwd, 'pkg', '__main__.py', text=script)
+        self.write_script(cwd, 'pkg', 'script.py', text=script)
+        self.write_script(cwd, 'script.py', text=script)
+
+        cases = [
+            ('script.py', cwd),
+            ('-m script', cwd),
+            ('-m pkg', cwd),
+            ('-m pkg.script', cwd),
+            ('-c "import script"', ''),
+        ]
+        for argv, expected in cases:
+            with self.subTest(f'python3 {argv}'):
+                out = self.run_python(argv, cwd=cwd)
+                data = json.loads(out)
+                sp0_main, sp0_sub = data['main'], data['sub']
+                self.assertEqual(sp0_sub, sp0_main)
+                self.assertEqual(sp0_sub, expected)
+        # XXX Also check them all with the -P cmdline flag?
+
+
 class FinalizationTests(TestBase):
 
     def test_gh_109793(self):
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-27-18-01-06.gh-issue-109853.coQQiL.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-27-18-01-06.gh-issue-109853.coQQiL.rst
@@ -0,0 +1 @@
+``sys.path[0]`` is now set correctly for subinterpreters.
diff --git a/Modules/main.c b/Modules/main.c
@@ -556,6 +556,11 @@ pymain_run_python(int *exitcode)
         goto error;
     }
 
+    // XXX Calculate config->sys_path_0 in getpath.py.
+    // The tricky part is that we can't check the path importers yet
+    // at that point.
+    assert(config->sys_path_0 == NULL);
+
     if (config->run_filename != NULL) {
         /* If filename is a package (ex: directory or ZIP file) which contains
            __main__.py, main_importer_path is set to filename and will be
@@ -571,24 +576,37 @@ pymain_run_python(int *exitcode)
     // import readline and rlcompleter before script dir is added to sys.path
     pymain_import_readline(config);
 
+    PyObject *path0 = NULL;
     if (main_importer_path != NULL) {
-        if (pymain_sys_path_add_path0(interp, main_importer_path) < 0) {
-            goto error;
-        }
+        path0 = Py_NewRef(main_importer_path);
     }
     else if (!config->safe_path) {
-        PyObject *path0 = NULL;
         int res = _PyPathConfig_ComputeSysPath0(&config->argv, &path0);
         if (res < 0) {
             goto error;
         }
-
-        if (res > 0) {
-            if (pymain_sys_path_add_path0(interp, path0) < 0) {
-                Py_DECREF(path0);
-                goto error;
-            }
+        else if (res == 0) {
+            Py_CLEAR(path0);
+        }
+    }
+    // XXX Apply config->sys_path_0 in init_interp_main().  We have
+    // to be sure to get readline/rlcompleter imported at the correct time.
+    if (path0 != NULL) {
+        wchar_t *wstr = PyUnicode_AsWideCharString(path0, NULL);
+        if (wstr == NULL) {
             Py_DECREF(path0);
+            goto error;
+        }
+        config->sys_path_0 = _PyMem_RawWcsdup(wstr);
+        PyMem_Free(wstr);
+        if (config->sys_path_0 == NULL) {
+            Py_DECREF(path0);
+            goto error;
+        }
+        int res = pymain_sys_path_add_path0(interp, path0);
+        Py_DECREF(path0);
+        if (res < 0) {
+            goto error;
         }
     }
 
diff --git a/Python/initconfig.c b/Python/initconfig.c
@@ -97,6 +97,7 @@ static const PyConfigSpec PYCONFIG_SPEC[] = {
     SPEC(pythonpath_env, WSTR_OPT),
     SPEC(home, WSTR_OPT),
     SPEC(platlibdir, WSTR),
+    SPEC(sys_path_0, WSTR_OPT),
     SPEC(module_search_paths_set, UINT),
     SPEC(module_search_paths, WSTR_LIST),
     SPEC(stdlib_dir, WSTR_OPT),
@@ -770,6 +771,7 @@ PyConfig_Clear(PyConfig *config)
     CLEAR(config->exec_prefix);
     CLEAR(config->base_exec_prefix);
     CLEAR(config->platlibdir);
+    CLEAR(config->sys_path_0);
 
     CLEAR(config->filesystem_encoding);
     CLEAR(config->filesystem_errors);
@@ -3051,6 +3053,7 @@ _Py_DumpPathConfig(PyThreadState *tstate)
     PySys_WriteStderr("  import site = %i\n", config->site_import);
     PySys_WriteStderr("  is in build tree = %i\n", config->_is_python_build);
     DUMP_CONFIG("stdlib dir", stdlib_dir);
+    DUMP_CONFIG("sys.path[0]", sys_path_0);
 #undef DUMP_CONFIG
 
 #define DUMP_SYS(NAME) \
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
@@ -1209,6 +1209,31 @@ init_interp_main(PyThreadState *tstate)
         }
     }
 
+    if (!is_main_interp) {
+        // The main interpreter is handled in Py_Main(), for now.
+        if (config->sys_path_0 != NULL) {
+            PyObject *path0 = PyUnicode_FromWideChar(config->sys_path_0, -1);
+            if (path0 == NULL) {
+                return _PyStatus_ERR("can't initialize sys.path[0]");
+            }
+            PyObject *sysdict = interp->sysdict;
+            if (sysdict == NULL) {
+                Py_DECREF(path0);
+                return _PyStatus_ERR("can't initialize sys.path[0]");
+            }
+            PyObject *sys_path = PyDict_GetItemWithError(sysdict, &_Py_ID(path));
+            if (sys_path == NULL) {
+                Py_DECREF(path0);
+                return _PyStatus_ERR("can't initialize sys.path[0]");
+            }
+            int res = PyList_Insert(sys_path, 0, path0);
+            Py_DECREF(path0);
+            if (res) {
+                return _PyStatus_ERR("can't initialize sys.path[0]");
+            }
+        }
+    }
+
     assert(!_PyErr_Occurred(tstate));
 
     return _PyStatus_OK();

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+``sys.path[0]`` is now set correctly for subinterpreters.