Re: [PATCH] Handle Python 3.11 deprecation of PySys_SetPath and Py_SetProgramName

[Simon Marchi <simark@simark.ca>]

On 6/30/22 19:10, Kevin Buettner via Gdb-patches wrote:

> Python 3.11 deprecates PySys_SetPath and Py_SetProgramName.  The

> PyConfig API replaces these and other functions.  This commit uses the

> PyConfig API to provide (hopefully) equivalent functionality while

> also preserving support for older versions of Python, i.e. those

> before Python 3.8.

>

> A beta version of Python 3.11 is available in Fedora Rawhide.  Both

> Fedora 35 and Fedora 36 use Python 3.10, while Fedora 34 still used

> Python 3.9.  I've tested these changes on Fedora 34, Fedora 36, and

> rawhide, though complete testing was not possible on rawhide due to

> a kernel bug.  That being the case, I decided to enable the newer

> PyConfig API by testing PY_VERSION_HEX against 0x030a0000.  This

> corresponds to Python 3.10.

>

> We could try to use the new API for Python versions as early as 3.8,

> but I'm reluctant to do this as there may have been PyConfig related

> bugs in earlier versions which have since been fixed.  Recent linux

> distributions should include support for Python 3.10.  This should be

> more than adequate for testing the new Python initialization code in

> GDB.

>

> Information about the PyConfig API as well as the motivation behind

> deprecating the old interface can be found at these links:

>

> https://github.com/python/cpython/issues/88279

> https://peps.python.org/pep-0587/

> https://docs.python.org/3.11/c-api/init_config.html

> ---

>  gdb/python/python-internal.h |  5 +++++

>  gdb/python/python.c          | 43 ++++++++++++++++++++++++++++++++++--

>  2 files changed, 46 insertions(+), 2 deletions(-)

>

> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h

> index 5ff9989af83..3dae4e13a4c 100644

> --- a/gdb/python/python-internal.h

> +++ b/gdb/python/python-internal.h

> @@ -177,6 +177,10 @@ gdb_PySys_GetObject (const char *name)

>

>  #define PySys_GetObject gdb_PySys_GetObject

>

> +/* PySys_SetPath was deprecated in Python 3.11.  Disable the deprecated

> +   code for Python 3.10 and newer.  */

> +#if PY_VERSION_HEX < 0x030a0000

> +

>  /* PySys_SetPath's 'path' parameter was missing the 'const' qualifier

>     before Python 3.6.  Hence, we wrap it in a function to avoid errors

>     when compiled with -Werror.  */

> @@ -190,6 +194,7 @@ gdb_PySys_SetPath (const GDB_PYSYS_SETPATH_CHAR *path)

>  }

>

>  #define PySys_SetPath gdb_PySys_SetPath

> +#endif

>

>  /* Wrap PyGetSetDef to allow convenient construction with string

>     literals.  Unfortunately, PyGetSetDef's 'name' and 'doc' members

> diff --git a/gdb/python/python.c b/gdb/python/python.c

> index 8f526bba84e..ce42c59f1ae 100644

> --- a/gdb/python/python.c

> +++ b/gdb/python/python.c

> @@ -2001,16 +2001,44 @@ do_start_initialization ()

>      }

>    setlocale (LC_ALL, oldloc.c_str ());

>

> +  /* Py_SetProgramName was deprecated in Python 3.11.  Use PyConfig

> +     mechanisms for Python 3.10 and newer.  */

> +#if PY_VERSION_HEX < 0x030a0000

>    /* Note that Py_SetProgramName expects the string it is passed to

>       remain alive for the duration of the program's execution, so

>       it is not freed after this call.  */

>    Py_SetProgramName (progname_copy);

> -

>    /* Define _gdb as a built-in module.  */

>    PyImport_AppendInittab ("_gdb", init__gdb_module);

> -#endif

> +  Py_Initialize ();

> +#else

> +  PyStatus status;



Declare status when initializing it.



> +  PyConfig config;

> +

> +  PyConfig_InitPythonConfig (&config);

> +  status = PyConfig_SetString (&config, &config.program_name, progname_copy);

>

> +  config.write_bytecode = !Py_DontWriteBytecodeFlag;



I see that Py_DontWriteBytecodeFlag gets deprecated in Python 3.12:



  https://docs.python.org/3.12/c-api/init.html#c.Py_DontWriteBytecodeFlag



So we should perhaps avoid new code using it.  I think that the "set

python dont-write-bytecode" command could set some bool flag in GDB,

which we then use during initialization.  The "old" path would copy that

bool to Py_DontWriteBytecodeFlag, and the "new" path would copy it to

config.write_bytecode.



> +  config.use_environment = !python_ignore_environment;

> +

> +  if (!PyStatus_Exception (status))



status here contains the result of PyConfig_SetString.  If

PyConfig_SetString returns an error (same for all calls returning a

PyStatus), should we bail out?  They seem like unrecoverable errors to

me.



> +    status = PyConfig_Read (&config);

> +

> +  /* Define _gdb as a built-in module.  */

> +  if (PyImport_AppendInittab ("_gdb", init__gdb_module) == -1)

> +    return false;

> +

> +  if (!PyStatus_Exception (status))

> +    status = Py_InitializeFromConfig (&config);

> +

> +  PyConfig_Clear (&config);



Should we call PyConfig_Clear in any case, including on error?  If so,

it means we should call it before returning above.  Perhaps using an

RAII object or SCOPE_EXT would make that easier.



Simon