Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 2.3k
(dev): 3.11 testing#3923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
(dev): 3.11 testing #3923
Changes from all commits
f777165056965ce29f7a9574bd9560a2ab85045e2e78ff6400d7c4b79887cdc1cd2513dfa1a229b0dad5e0e567304d60f16f43cc4File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -345,9 +345,11 @@ struct type_record { | ||
| bases.append((PyObject *) base_info->type); | ||
| if (base_info->type->tp_dictoffset != 0) { | ||
| dynamic_attr = true; | ||
| } | ||
| #if PY_VERSION_HEX < 0x030B0000 | ||
| dynamic_attr |= base_info->type->tp_dictoffset != 0; | ||
henryiii marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| #else | ||
| dynamic_attr |= (base_info->type->tp_flags & Py_TPFLAGS_MANAGED_DICT) != 0; | ||
| #endif | ||
| if (caster) { | ||
| base_info->implicit_casts.emplace_back(type, caster); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -86,37 +86,6 @@ inline wchar_t *widen_chars(const char *safe_arg) { | ||
| return widened_arg; | ||
| } | ||
| /// Python 2.x/3.x-compatible version of `PySys_SetArgv` | ||
| inline void set_interpreter_argv(int argc, const char *const *argv, bool add_program_dir_to_path) { | ||
| // Before it was special-cased in python 3.8, passing an empty or null argv | ||
| // caused a segfault, so we have to reimplement the special case ourselves. | ||
| bool special_case = (argv == nullptr || argc <= 0); | ||
| const char *const empty_argv[]{"\0"}; | ||
| const char *const *safe_argv = special_case ? empty_argv : argv; | ||
| if (special_case) { | ||
| argc = 1; | ||
| } | ||
| auto argv_size = static_cast<size_t>(argc); | ||
| // SetArgv* on python 3 takes wchar_t, so we have to convert. | ||
| std::unique_ptr<wchar_t *[]> widened_argv(new wchar_t *[argv_size]); | ||
| std::vector<std::unique_ptr<wchar_t[], wide_char_arg_deleter>> widened_argv_entries; | ||
| widened_argv_entries.reserve(argv_size); | ||
| for (size_t ii = 0; ii < argv_size; ++ii) { | ||
| widened_argv_entries.emplace_back(widen_chars(safe_argv[ii])); | ||
| if (!widened_argv_entries.back()) { | ||
| // A null here indicates a character-encoding failure or the python | ||
| // interpreter out of memory. Give up. | ||
| return; | ||
| } | ||
| widened_argv[ii] = widened_argv_entries.back().get(); | ||
| } | ||
| auto *pysys_argv = widened_argv.get(); | ||
| PySys_SetArgvEx(argc, pysys_argv, static_cast<int>(add_program_dir_to_path)); | ||
| } | ||
| PYBIND11_NAMESPACE_END(detail) | ||
| /** \rst | ||
| @@ -146,9 +115,64 @@ inline void initialize_interpreter(bool init_signal_handlers = true, | ||
| pybind11_fail("The interpreter is already running"); | ||
| } | ||
| #if PY_VERSION_HEX < 0x030B0000 | ||
| Py_InitializeEx(init_signal_handlers ? 1 : 0); | ||
| detail::set_interpreter_argv(argc, argv, add_program_dir_to_path); | ||
| // Before it was special-cased in python 3.8, passing an empty or null argv | ||
Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm really concerned about inlining the two complex functions here, especially because each version introduces multiple variables which could clash in non-obvious way for one Python version or another. We don't know how this code will need to be evolved in the future, accidents seem likely. I think it would be much better to keep the functions separate, even if we need | ||
| // caused a segfault, so we have to reimplement the special case ourselves. | ||
| bool special_case = (argv == nullptr || argc <= 0); | ||
| const char *const empty_argv[]{"\0"}; | ||
| const char *const *safe_argv = special_case ? empty_argv : argv; | ||
| if (special_case) { | ||
| argc = 1; | ||
| } | ||
| auto argv_size = static_cast<size_t>(argc); | ||
| // SetArgv* on python 3 takes wchar_t, so we have to convert. | ||
| std::unique_ptr<wchar_t *[]> widened_argv(new wchar_t *[argv_size]); | ||
| std::vector<std::unique_ptr<wchar_t[], detail::wide_char_arg_deleter>> widened_argv_entries; | ||
| widened_argv_entries.reserve(argv_size); | ||
| for (size_t ii = 0; ii < argv_size; ++ii) { | ||
| widened_argv_entries.emplace_back(detail::widen_chars(safe_argv[ii])); | ||
| if (!widened_argv_entries.back()) { | ||
| // A null here indicates a character-encoding failure or the python | ||
| // interpreter out of memory. Give up. | ||
| return; | ||
| } | ||
| widened_argv[ii] = widened_argv_entries.back().get(); | ||
| } | ||
| auto *pysys_argv = widened_argv.get(); | ||
| PySys_SetArgvEx(argc, pysys_argv, static_cast<int>(add_program_dir_to_path)); | ||
| #else | ||
| PyConfig config; | ||
Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A comment here would be useful (similar to Henry's comment from a few days ago):
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What has to be duplicated? Collaborator There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had this comment in mind: @henryiii wrote 14 days ago:
When I saw the email that Henry had already merged this PR I stopped looking further. Doing that now: This seems to be what @henryiii was referring to (and the "Link" I had in mind): Comparing that with the new Looking more, it seems this is @vstinner's response to the @henryiii's comment above: Which makes me think adding this link would be useful: Or maybe alternatively the link to Victor's comment. Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PyConfig API initializes the default value of sys.path. But Py_RunMain() then inserts a path to sys.path. Python 3.11 safe_path=1 disables this behavior:
It might be interesting to move more code changing sys.path into the Python initialization (PyConfig), but it's complicated. | ||
| PyConfig_InitIsolatedConfig(&config); | ||
| config.install_signal_handlers = init_signal_handlers ? 1 : 0; | ||
| PyStatus status = PyConfig_SetBytesArgv(&config, argc, const_cast<char *const *>(argv)); | ||
| if (PyStatus_Exception(status)) { | ||
| // 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"); | ||
| } | ||
| 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"); | ||
| } | ||
| if (add_program_dir_to_path) { | ||
| PyRun_SimpleString("import sys, os.path; " | ||
| "sys.path.insert(0, " | ||
| "os.path.abspath(os.path.dirname(sys.argv[0])) " | ||
| "if sys.argv and os.path.exists(sys.argv[0]) else '')"); | ||
| } | ||
| #endif | ||
| } | ||
| /** \rst | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consistency would be nice, e.g.
3.11-latest above (line 33) and here.
I often look at CI logs in bulk, having to manually map
latest internalshere to-develsewhere is confusing/distracting.