-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
bpo-34247: actually use environment variables in Py_Initialize #8521
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
Conversation
…sys.flags This patch moves a number of startup options from the _Py_CommandLineDetails to _PyCoreConfig to make it possible to process them in Py_Initialize, actually uses this and adds a function to update the relevant global variables from _Py_Initialize.
| _PyCoreConfig config = _PyCoreConfig_INIT; | ||
|
|
||
| err = _PyCoreConfig_Read(&config); | ||
| err = _PyCoreConfig_Read(&config, 1); |
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.
Not sure about this one, this is the minimal change to make this code compile with my other changes and without any changes to its behaviour. I haven't check the code yet to see if the changed behaviour you get with "0" as the second argument would be more useful here.
|
|
||
| static _PyInitError | ||
| config_read_env_vars(_PyCoreConfig *config) | ||
| config_read_env_vars(_PyCoreConfig *config, int ignore_cmdline) |
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.
I added the ignore_cmdline to be 100% sure that I don't change the behaviour of Py_Main with this patch, and haven't checked yet if this is really necessary.
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.
Provided we are handling the -I option correctly and are also not looking at argv unless using Py_Main, it shouldn't be necessary.
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.
I've talked with Victor about this issue (without looking at this code in detail), the master branch contains a much better implementation of this included much improved unit tests.
I won't update this pull request until he has had time to think about this issue.
|
I proposed a different approach: PR #8659. |
|
I merged my PR #8659 instead which is more complete and contains unit tests. Thanks for your PR anyway! |
As mentioned in bpo-34247 a program embedding Python with Py_Initialize / Py_Run... (no Py_Main) cannot affect a number of interpreter options through environment variables.
This is a pull request against the 3.7 branch, primarily because Nick Coghlan mentioned that there is related work on the master branch by Victor Stinner (bpo-34170).
The patch works for me, but I haven't dug deeply enough in the code to be sure that this is the right patch, I'm particularly unsure about the additional argument to _PyCoreConfig_Read.
https://bugs.python.org/issue34247