From 88699849264150f513dafb98d9ec1bb31a7fa904 Mon Sep 17 00:00:00 2001 From: Arman Date: Thu, 1 Dec 2022 07:17:59 +0200 Subject: [PATCH 1/4] scoped_interpreter. overloaded constructor: PyConfig param (#4330) * scoped_interpreter overloaded ctor: PyConfig param * style: pre-commit fixes * refact: some logics extracted into funcs (precheck_interpreter, _initialize_interpreter); config_guard * style: pre-commit fixes * refact: scoped_config, some funcs hidden in detail ns * refact: macro PYBIND11_PYCONFIG_SUPPORT_PY_VERSION + undef * feat: PYBIND11_PYCONFIG_SUPPORT_PY_VERSION set to 3.8 * tests: Custom PyConfig * ci: python 3.6 -> 3.8 * ci: reverted py 38 back to 36; refact: initialize_interpreter overloads * style: pre-commit fixes * fix: readability-implicit-bool-conversion * refact: each initialize_interpreter overloads in pybind11 ns * Move `initialize_interpreter_pre_pyconfig()` into the `detail` namespace. Move the `PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX` define down to where it is used for the first time, and check if it is defined already, so that it is possible to customize from the compilation command line, just in case there is some unforeseen issue for Python 3.8, 3.9, 3.10. * tests: Add program dir to path, Custom PyConfig with argv * refact: clang-formatted * tests: Add-program-dir-to-path covers both scoped_interpreter overloads * tests: Add-program-dir-to-path fixed * tests: Add-program-dir-to-path py_version dependant validation Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Ralf W. Grosse-Kunstleve --- include/pybind11/embed.h | 119 +++++++++++++++++--------- tests/test_embed/test_interpreter.cpp | 66 ++++++++++++++ 2 files changed, 143 insertions(+), 42 deletions(-) diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index 5b77594b3..749c75beb 100644 --- a/include/pybind11/embed.h +++ b/include/pybind11/embed.h @@ -86,37 +86,22 @@ inline wchar_t *widen_chars(const char *safe_arg) { return widened_arg; } -PYBIND11_NAMESPACE_END(detail) - -/** \rst - Initialize the Python interpreter. No other pybind11 or CPython API functions can be - called before this is done; with the exception of `PYBIND11_EMBEDDED_MODULE`. The - optional `init_signal_handlers` parameter can be used to skip the registration of - signal handlers (see the `Python documentation`_ for details). Calling this function - again after the interpreter has already been initialized is a fatal error. - - If initializing the Python interpreter fails, then the program is terminated. (This - is controlled by the CPython runtime and is an exception to pybind11's normal behavior - of throwing exceptions on errors.) - - The remaining optional parameters, `argc`, `argv`, and `add_program_dir_to_path` are - used to populate ``sys.argv`` and ``sys.path``. - See the |PySys_SetArgvEx documentation|_ for details. - - .. _Python documentation: https://docs.python.org/3/c-api/init.html#c.Py_InitializeEx - .. |PySys_SetArgvEx documentation| replace:: ``PySys_SetArgvEx`` documentation - .. _PySys_SetArgvEx documentation: https://docs.python.org/3/c-api/init.html#c.PySys_SetArgvEx - \endrst */ -inline void initialize_interpreter(bool init_signal_handlers = true, - int argc = 0, - const char *const *argv = nullptr, - bool add_program_dir_to_path = true) { +inline void precheck_interpreter() { if (Py_IsInitialized() != 0) { pybind11_fail("The interpreter is already running"); } +} -#if PY_VERSION_HEX < 0x030B0000 +#if !defined(PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX) +# define PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX (0x03080000) +#endif +#if PY_VERSION_HEX < PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX +inline void initialize_interpreter_pre_pyconfig(bool init_signal_handlers, + int argc, + const char *const *argv, + bool add_program_dir_to_path) { + detail::precheck_interpreter(); Py_InitializeEx(init_signal_handlers ? 1 : 0); # if defined(WITH_THREAD) && PY_VERSION_HEX < 0x03070000 PyEval_InitThreads(); @@ -150,26 +135,30 @@ inline void initialize_interpreter(bool init_signal_handlers = true, auto *pysys_argv = widened_argv.get(); PySys_SetArgvEx(argc, pysys_argv, static_cast(add_program_dir_to_path)); -#else - PyConfig config; - PyConfig_InitIsolatedConfig(&config); - config.isolated = 0; - config.use_environment = 1; - config.install_signal_handlers = init_signal_handlers ? 1 : 0; +} +#endif - PyStatus status = PyConfig_SetBytesArgv(&config, argc, const_cast(argv)); - if (PyStatus_Exception(status)) { +PYBIND11_NAMESPACE_END(detail) + +#if PY_VERSION_HEX >= PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX +inline void initialize_interpreter(PyConfig *config, + int argc = 0, + const char *const *argv = nullptr, + bool add_program_dir_to_path = true) { + detail::precheck_interpreter(); + PyStatus status = PyConfig_SetBytesArgv(config, argc, const_cast(argv)); + if (PyStatus_Exception(status) != 0) { // A failure here indicates a character-encoding failure or the python // interpreter out of memory. Give up. - PyConfig_Clear(&config); - throw std::runtime_error(PyStatus_IsError(status) ? status.err_msg - : "Failed to prepare CPython"); + PyConfig_Clear(config); + throw std::runtime_error(PyStatus_IsError(status) != 0 ? status.err_msg + : "Failed to prepare CPython"); } - status = Py_InitializeFromConfig(&config); - PyConfig_Clear(&config); - if (PyStatus_Exception(status)) { - throw std::runtime_error(PyStatus_IsError(status) ? status.err_msg - : "Failed to init CPython"); + status = Py_InitializeFromConfig(config); + if (PyStatus_Exception(status) != 0) { + PyConfig_Clear(config); + throw std::runtime_error(PyStatus_IsError(status) != 0 ? status.err_msg + : "Failed to init CPython"); } if (add_program_dir_to_path) { PyRun_SimpleString("import sys, os.path; " @@ -177,6 +166,43 @@ inline void initialize_interpreter(bool init_signal_handlers = true, "os.path.abspath(os.path.dirname(sys.argv[0])) " "if sys.argv and os.path.exists(sys.argv[0]) else '')"); } + PyConfig_Clear(config); +} +#endif + +/** \rst + Initialize the Python interpreter. No other pybind11 or CPython API functions can be + called before this is done; with the exception of `PYBIND11_EMBEDDED_MODULE`. The + optional `init_signal_handlers` parameter can be used to skip the registration of + signal handlers (see the `Python documentation`_ for details). Calling this function + again after the interpreter has already been initialized is a fatal error. + + If initializing the Python interpreter fails, then the program is terminated. (This + is controlled by the CPython runtime and is an exception to pybind11's normal behavior + of throwing exceptions on errors.) + + The remaining optional parameters, `argc`, `argv`, and `add_program_dir_to_path` are + used to populate ``sys.argv`` and ``sys.path``. + See the |PySys_SetArgvEx documentation|_ for details. + + .. _Python documentation: https://docs.python.org/3/c-api/init.html#c.Py_InitializeEx + .. |PySys_SetArgvEx documentation| replace:: ``PySys_SetArgvEx`` documentation + .. _PySys_SetArgvEx documentation: https://docs.python.org/3/c-api/init.html#c.PySys_SetArgvEx + \endrst */ +inline void initialize_interpreter(bool init_signal_handlers = true, + int argc = 0, + const char *const *argv = nullptr, + bool add_program_dir_to_path = true) { +#if PY_VERSION_HEX < PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX + detail::initialize_interpreter_pre_pyconfig( + init_signal_handlers, argc, argv, add_program_dir_to_path); +#else + PyConfig config; + PyConfig_InitIsolatedConfig(&config); + config.isolated = 0; + config.use_environment = 1; + config.install_signal_handlers = init_signal_handlers ? 1 : 0; + initialize_interpreter(&config, argc, argv, add_program_dir_to_path); #endif } @@ -264,6 +290,15 @@ public: initialize_interpreter(init_signal_handlers, argc, argv, add_program_dir_to_path); } +#if PY_VERSION_HEX >= PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX + explicit scoped_interpreter(PyConfig *config, + int argc = 0, + const char *const *argv = nullptr, + bool add_program_dir_to_path = true) { + initialize_interpreter(config, argc, argv, add_program_dir_to_path); + } +#endif + scoped_interpreter(const scoped_interpreter &) = delete; scoped_interpreter(scoped_interpreter &&other) noexcept { other.is_valid = false; } scoped_interpreter &operator=(const scoped_interpreter &) = delete; diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 8cfd13fe5..5c306d363 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -166,6 +166,72 @@ TEST_CASE("There can be only one interpreter") { py::initialize_interpreter(); } +#if PY_VERSION_HEX >= PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX +TEST_CASE("Custom PyConfig") { + py::finalize_interpreter(); + PyConfig config; + PyConfig_InitPythonConfig(&config); + REQUIRE_NOTHROW(py::scoped_interpreter{&config}); + { + py::scoped_interpreter p{&config}; + REQUIRE(py::module_::import("widget_module").attr("add")(1, 41).cast() == 42); + } + py::initialize_interpreter(); +} + +TEST_CASE("Custom PyConfig with argv") { + py::finalize_interpreter(); + { + PyConfig config; + PyConfig_InitIsolatedConfig(&config); + char *argv[] = {strdup("a.out")}; + py::scoped_interpreter argv_scope{&config, 1, argv}; + std::free(argv[0]); + auto module = py::module::import("test_interpreter"); + auto py_widget = module.attr("DerivedWidget")("The question"); + const auto &cpp_widget = py_widget.cast(); + REQUIRE(cpp_widget.argv0() == "a.out"); + } + py::initialize_interpreter(); +} +#endif + +TEST_CASE("Add program dir to path") { + static auto get_sys_path_size = []() -> size_t { + auto sys_path = py::module::import("sys").attr("path"); + return py::len(sys_path); + }; + static auto validate_path_len = [](size_t default_len) { +#if PY_VERSION_HEX < 0x030A0000 + // It seems a value remains in sys.path + // left by the previous call of scoped_interpreter ctor. + REQUIRE(get_sys_path_size() > default_len); +#else + REQUIRE(get_sys_path_size() == default_len + 1); +#endif + }; + py::finalize_interpreter(); + + size_t sys_path_default_size = 0; + { + py::scoped_interpreter scoped_interp{true, 0, nullptr, false}; + sys_path_default_size = get_sys_path_size(); + } + { + py::scoped_interpreter scoped_interp{}; // expected to append some to sys.path + validate_path_len(sys_path_default_size); + } +#if PY_VERSION_HEX >= PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX + { + PyConfig config; + PyConfig_InitPythonConfig(&config); + py::scoped_interpreter scoped_interp{&config}; // expected to append some to sys.path + validate_path_len(sys_path_default_size); + } +#endif + py::initialize_interpreter(); +} + bool has_pybind11_internals_builtin() { auto builtins = py::handle(PyEval_GetBuiltins()); return builtins.contains(PYBIND11_INTERNALS_ID); From b14d58b6151b3cc7a9d9b2bf90d636940935db5c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 30 Nov 2022 23:30:36 -0800 Subject: [PATCH 2/4] chore(deps): bump pypa/gh-action-pypi-publish from 1.5.1 to 1.5.2 (#4370) Bumps [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) from 1.5.1 to 1.5.2. - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](https://github.com/pypa/gh-action-pypi-publish/compare/v1.5.1...v1.5.2) --- updated-dependencies: - dependency-name: pypa/gh-action-pypi-publish dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/pip.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pip.yml b/.github/workflows/pip.yml index f03a39701..b787187ee 100644 --- a/.github/workflows/pip.yml +++ b/.github/workflows/pip.yml @@ -98,13 +98,13 @@ jobs: - uses: actions/download-artifact@v3 - name: Publish standard package - uses: pypa/gh-action-pypi-publish@v1.5.1 + uses: pypa/gh-action-pypi-publish@v1.5.2 with: password: ${{ secrets.pypi_password }} packages_dir: standard/ - name: Publish global package - uses: pypa/gh-action-pypi-publish@v1.5.1 + uses: pypa/gh-action-pypi-publish@v1.5.2 with: password: ${{ secrets.pypi_password_global }} packages_dir: global/ From 358ba459d2f05321a15555c3b57c606fe2597ec7 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 1 Dec 2022 09:25:30 -0800 Subject: [PATCH 3/4] Fix test added with PR #4330 (#4372) --- tests/test_embed/test_interpreter.cpp | 47 ++++++++++++++------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 5c306d363..10b20f371 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -14,6 +14,11 @@ PYBIND11_WARNING_DISABLE_MSVC(4996) namespace py = pybind11; using namespace py::literals; +size_t get_sys_path_size() { + auto sys_path = py::module::import("sys").attr("path"); + return py::len(sys_path); +} + class Widget { public: explicit Widget(std::string message) : message(std::move(message)) {} @@ -196,41 +201,39 @@ TEST_CASE("Custom PyConfig with argv") { } #endif -TEST_CASE("Add program dir to path") { - static auto get_sys_path_size = []() -> size_t { - auto sys_path = py::module::import("sys").attr("path"); - return py::len(sys_path); - }; - static auto validate_path_len = [](size_t default_len) { -#if PY_VERSION_HEX < 0x030A0000 - // It seems a value remains in sys.path - // left by the previous call of scoped_interpreter ctor. - REQUIRE(get_sys_path_size() > default_len); -#else - REQUIRE(get_sys_path_size() == default_len + 1); -#endif - }; +TEST_CASE("Add program dir to path pre-PyConfig") { py::finalize_interpreter(); - - size_t sys_path_default_size = 0; + size_t path_size_add_program_dir_to_path_false = 0; { py::scoped_interpreter scoped_interp{true, 0, nullptr, false}; - sys_path_default_size = get_sys_path_size(); + path_size_add_program_dir_to_path_false = get_sys_path_size(); } { - py::scoped_interpreter scoped_interp{}; // expected to append some to sys.path - validate_path_len(sys_path_default_size); + py::scoped_interpreter scoped_interp{}; + REQUIRE(get_sys_path_size() == path_size_add_program_dir_to_path_false + 1); } + py::initialize_interpreter(); +} + #if PY_VERSION_HEX >= PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX +TEST_CASE("Add program dir to path using PyConfig") { + py::finalize_interpreter(); + size_t path_size_add_program_dir_to_path_false = 0; { PyConfig config; PyConfig_InitPythonConfig(&config); - py::scoped_interpreter scoped_interp{&config}; // expected to append some to sys.path - validate_path_len(sys_path_default_size); + py::scoped_interpreter scoped_interp{&config, 0, nullptr, false}; + path_size_add_program_dir_to_path_false = get_sys_path_size(); + } + { + PyConfig config; + PyConfig_InitPythonConfig(&config); + py::scoped_interpreter scoped_interp{&config}; + REQUIRE(get_sys_path_size() == path_size_add_program_dir_to_path_false + 1); } -#endif py::initialize_interpreter(); } +#endif bool has_pybind11_internals_builtin() { auto builtins = py::handle(PyEval_GetBuiltins()); From e133c33d5c6b19acab55fb1d6c10331d918bd830 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 1 Dec 2022 15:15:47 -0500 Subject: [PATCH 4/4] chore: Convert direct multiprocessing.set_start_method("forkserver") call to a pytest fixture. (#4377) * chore: convert multiprocessing set_spawn to fixture in pytest * Switch to early return --- tests/conftest.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 96dffc81c..402fd4b25 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,7 +17,12 @@ import pytest # Early diagnostic for failed imports import pybind11_tests -if os.name != "nt": + +@pytest.fixture(scope="session", autouse=True) +def always_forkserver_on_unix(): + if os.name == "nt": + return + # Full background: https://github.com/pybind/pybind11/issues/4105#issuecomment-1301004592 # In a nutshell: fork() after starting threads == flakiness in the form of deadlocks. # It is actually a well-known pitfall, unfortunately without guard rails. @@ -27,6 +32,7 @@ if os.name != "nt": # running with defaults. multiprocessing.set_start_method("forkserver") + _long_marker = re.compile(r"([0-9])L") _hexadecimal = re.compile(r"0x[0-9a-fA-F]+")