Skip to content
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

bpo-46964: Move PyInterpreterState.config to _PyRuntimeState.config #31771

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Mar 8, 2022

As part of this, we also move PyConfig._isolated_interpreter to a new _PyInterpreterConfig struct and split it up into more granular settings. We also change some of the private API names to explicitly indicate they relate to global config.

https://bugs.python.org/issue46964

Python/initconfig.c Outdated Show resolved Hide resolved
Python/initconfig.c Outdated Show resolved Hide resolved
/* Allow the creation of threads. */
unsigned int allow_threading:1;
/* Padding to ensure byte alignment. */
unsigned int :5;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

structs must be at least byte aligned, so this isn't strictly necessary.

@@ -552,7 +552,7 @@ pymain_run_python(int *exitcode)
PyObject *main_importer_path = NULL;
PyInterpreterState *interp = _PyInterpreterState_GET();
/* pymain_run_stdin() modify the config */
PyConfig *config = (PyConfig*)_PyInterpreterState_GetConfig(interp);
PyConfig *config = (PyConfig*)_PyInterpreterState_GetGlobalConfig(interp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the cast?

@@ -15026,7 +15026,7 @@ static PyStatus
init_stdio_encoding(PyInterpreterState *interp)
{
/* Update the stdio encoding to the normalized Python codec name. */
PyConfig *config = (PyConfig*)_PyInterpreterState_GetConfig(interp);
PyConfig *config = (PyConfig*)_PyInterpreterState_GetGlobalConfig(interp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the cast?

@@ -15097,7 +15097,7 @@ init_fs_encoding(PyThreadState *tstate)
/* Update the filesystem encoding to the normalized Python codec name.
For example, replace "ANSI_X3.4-1968" (locale encoding) with "ascii"
(Python codec name). */
PyConfig *config = (PyConfig*)_PyInterpreterState_GetConfig(interp);
PyConfig *config = (PyConfig*)_PyInterpreterState_GetGlobalConfig(interp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@@ -15140,7 +15140,7 @@ int
_PyUnicode_EnableLegacyWindowsFSEncoding(void)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
PyConfig *config = (PyConfig *)_PyInterpreterState_GetConfig(interp);
PyConfig *config = (PyConfig *)_PyInterpreterState_GetGlobalConfig(interp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

void
_PyInterpreterConfig_Clear(_PyInterpreterConfig *config)
{
*config = (_PyInterpreterConfig){};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs struct?

@@ -508,13 +510,20 @@ _PyInterpreterState_SetConfig(const PyConfig *src_config)
goto done;
}

status = _PyConfig_Copy(&tstate->interp->config, &config);
status = _PyConfig_Copy(&tstate->interp->runtime->config, &config);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a very convoluted way to get a static object

@kumaraditya303 kumaraditya303 removed their request for review November 30, 2022 16:22
@encukou
Copy link
Member

encukou commented Mar 19, 2024

_PyInterpreterConfig has been added in #98609.
@ericsnowcurrently, do you have any more plans with this PR?

@ericsnowcurrently
Copy link
Member Author

Yeah, there are a few things that I need to follow up on here.

@ericsnowcurrently ericsnowcurrently marked this pull request as draft November 19, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants