-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
(dev): 3.11 testing #3923
(dev): 3.11 testing #3923
Changes from all commits
f777165
056965c
e29f7a9
574bd95
60a2ab8
5045e2e
78ff640
0d7c4b7
9887cdc
1cd2513
dfa1a22
9b0dad5
e0e5673
04d60f1
6f43cc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
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; | ||
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):
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? 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. 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 internals
here to-dev
elsewhere is confusing/distracting.