From 198c9c21372047cf43856d4d01f0cc56d2f9a11f Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 5 Aug 2022 08:58:53 -0700 Subject: [PATCH 1/9] Add debug fprintf to test_interpreter.cpp --- tests/test_embed/test_interpreter.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 1c45457a05..a41a224ce0 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -173,6 +173,18 @@ bool has_pybind11_internals_static() { TEST_CASE("Restart the interpreter") { // Verify pre-restart state. + fprintf(stdout, + "\nLOOOK PYTHONPATH %s\n", + py::module_::import("os") + .attr("environ") + .attr("get")("PYTHONPATH") + .attr("__str__")() + .cast() + .c_str()); + fflush(stdout); + auto sys_path = py::module_::import("sys").attr("path").attr("__str__")().cast(); + fprintf(stdout, "\nLOOOK sys.path %s\n", sys_path.c_str()); + fflush(stdout); REQUIRE(py::module_::import("widget_module").attr("add")(1, 2).cast() == 3); REQUIRE(has_pybind11_internals_builtin()); REQUIRE(has_pybind11_internals_static()); From 25ababab2756e0db374bdbaacc879c9b47631e03 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 5 Aug 2022 15:06:48 -0700 Subject: [PATCH 2/9] Update `sys.path` from `PYTHONPATH` in Python >= 3.11 branch of `initialize_interpreter()` --- include/pybind11/embed.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index d6999cd779..718a8028ff 100644 --- a/include/pybind11/embed.h +++ b/include/pybind11/embed.h @@ -166,6 +166,9 @@ inline void initialize_interpreter(bool init_signal_handlers = true, throw std::runtime_error(PyStatus_IsError(status) ? status.err_msg : "Failed to init CPython"); } + // Emulates established behavior (prior to Python 3.11 implemented by PySys_SetArgvEx()). + PyRun_SimpleString("import sys, os; " + "sys.path[:0] = os.environ.get('PYTHONPATH', []).split(os.pathsep)"); if (add_program_dir_to_path) { PyRun_SimpleString("import sys, os.path; " "sys.path.insert(0, " From 65aa526a9b3b78bdc3ac6cf920aafbdf22d0f2d2 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 18 Aug 2022 10:57:49 -0700 Subject: [PATCH 3/9] Use `config.isolated = 0; config.use_environment = 1;` As suggsted by @vstinner here: https://github.com/pybind/pybind11/pull/4119#issuecomment-1219442853 --- include/pybind11/embed.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index 718a8028ff..0ac609e0f1 100644 --- a/include/pybind11/embed.h +++ b/include/pybind11/embed.h @@ -150,6 +150,8 @@ inline void initialize_interpreter(bool init_signal_handlers = true, #else PyConfig config; PyConfig_InitIsolatedConfig(&config); + config.isolated = 0; + config.use_environment = 1; config.install_signal_handlers = init_signal_handlers ? 1 : 0; PyStatus status = PyConfig_SetBytesArgv(&config, argc, const_cast(argv)); @@ -166,9 +168,6 @@ inline void initialize_interpreter(bool init_signal_handlers = true, throw std::runtime_error(PyStatus_IsError(status) ? status.err_msg : "Failed to init CPython"); } - // Emulates established behavior (prior to Python 3.11 implemented by PySys_SetArgvEx()). - PyRun_SimpleString("import sys, os; " - "sys.path[:0] = os.environ.get('PYTHONPATH', []).split(os.pathsep)"); if (add_program_dir_to_path) { PyRun_SimpleString("import sys, os.path; " "sys.path.insert(0, " From 379da551b93102799b2556e09466ddef0a88e65f Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 19 Aug 2022 14:10:04 -0700 Subject: [PATCH 4/9] Add `TEST_CASE("PYTHONPATH is used to update sys.path")` --- tests/test_embed/catch.cpp | 14 ++++++++++++++ tests/test_embed/test_interpreter.cpp | 20 ++++++++------------ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/tests/test_embed/catch.cpp b/tests/test_embed/catch.cpp index 96d2e3f92a..2d687e0ee3 100644 --- a/tests/test_embed/catch.cpp +++ b/tests/test_embed/catch.cpp @@ -20,7 +20,21 @@ namespace py = pybind11; int main(int argc, char *argv[]) { + // Setup for TEST_CASE in test_interpreter.cpp, tagging on a large random number: + std::string updated_pythonpath("/pybind11_test_embed_PYTHONPATH_2099743835476552"); + const char *preexisting_pythonpath = getenv("PYTHONPATH"); + if (preexisting_pythonpath != nullptr) { +#if defined(_MSC_VER) || defined(__MINGW32__) + updated_pythonpath += ';'; +#else + updated_pythonpath += ':'; +#endif + updated_pythonpath += preexisting_pythonpath; + } + setenv("PYTHONPATH", updated_pythonpath.c_str(), /*overwrite=*/1); + py::scoped_interpreter guard{}; + auto result = Catch::Session().run(argc, argv); return result < 0xff ? result : 0xff; diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index a41a224ce0..5eb2b72d4d 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -75,6 +75,14 @@ PYBIND11_EMBEDDED_MODULE(throw_error_already_set, ) { d["missing"].cast(); } +TEST_CASE("PYTHONPATH is used to update sys.path") { + // The setup for this TEST_CASE is in catch.cpp! + std::string sys_path + = py::module_::import("sys").attr("path").attr("__str__")().cast(); + REQUIRE_THAT(sys_path, + Catch::Matchers::Contains("/pybind11_test_embed_PYTHONPATH_2099743835476552")); +} + TEST_CASE("Pass classes and data between modules defined in C++ and Python") { auto module_ = py::module_::import("test_interpreter"); REQUIRE(py::hasattr(module_, "DerivedWidget")); @@ -173,18 +181,6 @@ bool has_pybind11_internals_static() { TEST_CASE("Restart the interpreter") { // Verify pre-restart state. - fprintf(stdout, - "\nLOOOK PYTHONPATH %s\n", - py::module_::import("os") - .attr("environ") - .attr("get")("PYTHONPATH") - .attr("__str__")() - .cast() - .c_str()); - fflush(stdout); - auto sys_path = py::module_::import("sys").attr("path").attr("__str__")().cast(); - fprintf(stdout, "\nLOOOK sys.path %s\n", sys_path.c_str()); - fflush(stdout); REQUIRE(py::module_::import("widget_module").attr("add")(1, 2).cast() == 3); REQUIRE(has_pybind11_internals_builtin()); REQUIRE(has_pybind11_internals_static()); From eea4e26a340f74ed154c36851fef6be4ef115ac9 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 19 Aug 2022 19:02:38 -0700 Subject: [PATCH 5/9] Fix clang-tidy error. --- tests/test_embed/test_interpreter.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 5eb2b72d4d..e005dcad6a 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -77,8 +77,7 @@ PYBIND11_EMBEDDED_MODULE(throw_error_already_set, ) { TEST_CASE("PYTHONPATH is used to update sys.path") { // The setup for this TEST_CASE is in catch.cpp! - std::string sys_path - = py::module_::import("sys").attr("path").attr("__str__")().cast(); + auto sys_path = py::module_::import("sys").attr("path").attr("__str__")().cast(); REQUIRE_THAT(sys_path, Catch::Matchers::Contains("/pybind11_test_embed_PYTHONPATH_2099743835476552")); } From 5e08ab3a665734874fed37b2afad47343d9ef04c Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 19 Aug 2022 19:16:07 -0700 Subject: [PATCH 6/9] Use `_putenv_s()` under Windows. --- tests/test_embed/catch.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_embed/catch.cpp b/tests/test_embed/catch.cpp index 2d687e0ee3..680a5004ea 100644 --- a/tests/test_embed/catch.cpp +++ b/tests/test_embed/catch.cpp @@ -24,14 +24,18 @@ int main(int argc, char *argv[]) { std::string updated_pythonpath("/pybind11_test_embed_PYTHONPATH_2099743835476552"); const char *preexisting_pythonpath = getenv("PYTHONPATH"); if (preexisting_pythonpath != nullptr) { -#if defined(_MSC_VER) || defined(__MINGW32__) +#if defined(_WIN32) updated_pythonpath += ';'; #else updated_pythonpath += ':'; #endif updated_pythonpath += preexisting_pythonpath; } +#if defined(_WIN32) + _putenv_s("PYTHONPATH", updated_pythonpath.c_str()); +#else setenv("PYTHONPATH", updated_pythonpath.c_str(), /*overwrite=*/1); +#endif py::scoped_interpreter guard{}; From bfbf4b3b5ad95774fa0f4b9e04d8872961e2a992 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 19 Aug 2022 20:51:51 -0700 Subject: [PATCH 7/9] Fix clang-tidy error: argument name ... in comment does not match parameter name --- tests/test_embed/catch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_embed/catch.cpp b/tests/test_embed/catch.cpp index 680a5004ea..d572eff0bd 100644 --- a/tests/test_embed/catch.cpp +++ b/tests/test_embed/catch.cpp @@ -34,7 +34,7 @@ int main(int argc, char *argv[]) { #if defined(_WIN32) _putenv_s("PYTHONPATH", updated_pythonpath.c_str()); #else - setenv("PYTHONPATH", updated_pythonpath.c_str(), /*overwrite=*/1); + setenv("PYTHONPATH", updated_pythonpath.c_str(), /*replace=*/1); #endif py::scoped_interpreter guard{}; From e4340b408049b5bae7d91fd81192f40a5b7ff8cf Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 19 Aug 2022 20:57:55 -0700 Subject: [PATCH 8/9] Remove slash from PYTHONPATH addition, to work around Windows slash-vs-backslash issue. --- tests/test_embed/catch.cpp | 2 +- tests/test_embed/test_interpreter.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_embed/catch.cpp b/tests/test_embed/catch.cpp index d572eff0bd..a03a8b37c4 100644 --- a/tests/test_embed/catch.cpp +++ b/tests/test_embed/catch.cpp @@ -21,7 +21,7 @@ namespace py = pybind11; int main(int argc, char *argv[]) { // Setup for TEST_CASE in test_interpreter.cpp, tagging on a large random number: - std::string updated_pythonpath("/pybind11_test_embed_PYTHONPATH_2099743835476552"); + std::string updated_pythonpath("pybind11_test_embed_PYTHONPATH_2099743835476552"); const char *preexisting_pythonpath = getenv("PYTHONPATH"); if (preexisting_pythonpath != nullptr) { #if defined(_WIN32) diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index e005dcad6a..21b41a4663 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -79,7 +79,7 @@ TEST_CASE("PYTHONPATH is used to update sys.path") { // The setup for this TEST_CASE is in catch.cpp! auto sys_path = py::module_::import("sys").attr("path").attr("__str__")().cast(); REQUIRE_THAT(sys_path, - Catch::Matchers::Contains("/pybind11_test_embed_PYTHONPATH_2099743835476552")); + Catch::Matchers::Contains("pybind11_test_embed_PYTHONPATH_2099743835476552")); } TEST_CASE("Pass classes and data between modules defined in C++ and Python") { From 086609cdb22650d8b33ca67f971cfd8c5a6c460d Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 20 Aug 2022 13:18:57 -0700 Subject: [PATCH 9/9] Use `py::str(...)` instead of `.attr("__str__")` as suggested by @skylion007 Co-authored-by: Aaron Gokaslan --- tests/test_embed/test_interpreter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 21b41a4663..6299293b91 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -77,7 +77,7 @@ PYBIND11_EMBEDDED_MODULE(throw_error_already_set, ) { TEST_CASE("PYTHONPATH is used to update sys.path") { // The setup for this TEST_CASE is in catch.cpp! - auto sys_path = py::module_::import("sys").attr("path").attr("__str__")().cast(); + auto sys_path = py::str(py::module_::import("sys").attr("path")).cast(); REQUIRE_THAT(sys_path, Catch::Matchers::Contains("pybind11_test_embed_PYTHONPATH_2099743835476552")); }