Re: [PATCH] fix build failure with Python 3.7

Re: [PATCH] fix build failure with Python 3.7

[Sergio Durigan Junior]

On Thursday, May 31 2018, Paul Koning wrote:

> This cures PR gdb/33470, build failure due to calling an internal 
> API in Python that changed in V3.7.  The code now uses the
> "inittab" mechanism in Python, which is the approved way to
> make compiled-in modules known to Python.
>
> Gdb starts properly and I can see the _gdb module.  On my test
> system (a Mac) it's hard to get gdb to run at all, so the
> test suite is a bit problematic.  What other tests are needed?

Thanks for the patch, Paul.  I'll run this on BuildBot and see what
comes out of it.  Meanwhile...

> 	paul
>
> gdb/ChangeLog:
>
> 2018-05-31  Paul Koning  <paul_koning@dell.com>
>
> 	PR gdb/33470

This bug number is actually from Python's bugzilla, not GDB's.  So it's
not correct to mention it here in the ChangeLog/commit message.  AFAIK,
there's no correspondent GDB bug filed for this issue.

> 	* python/python.c (do_start_initialization):
> 	Avoid call to internal Python API.
> 	(PyInit__gdb): New function.
>
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index c29e7d7a6b..89443aee25 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -1667,6 +1667,14 @@ finalize_python (void *ignore)
>    restore_active_ext_lang (previous_active);
>  }
>  
> +#ifdef IS_PY3K
> +PyMODINIT_FUNC
> +PyInit__gdb (void)
> +{
> +  return PyModule_Create (&python_GdbModuleDef);
> +}
> +#endif

I think it's a good idea to add a comment to this function.

> +
>  static bool
>  do_start_initialization ()
>  {
> @@ -1707,6 +1715,9 @@ do_start_initialization ()
>       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", PyInit__gdb);
>  #else
>    Py_SetProgramName (progname.release ());
>  #endif
> @@ -1716,9 +1727,7 @@ do_start_initialization ()
>    PyEval_InitThreads ();
>  
>  #ifdef IS_PY3K
> -  gdb_module = PyModule_Create (&python_GdbModuleDef);
> -  /* Add _gdb module to the list of known built-in modules.  */
> -  _PyImport_FixupBuiltin (gdb_module, "_gdb");
> +  gdb_module = PyImport_ImportModule ("_gdb");
>  #else
>    gdb_module = Py_InitModule ("_gdb", python_GdbMethods);
>  #endif

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH] fix build failure with Python 3.7

[Paul.Koning]

> On May 31, 2018, at 4:10 PM, Sergio Durigan Junior <sergiodj@redhat.com> wrote:
> 
>> 
>> gdb/ChangeLog:
>> 
>> 2018-05-31  Paul Koning  <paul_koning@dell.com>
>> 
>> 	PR gdb/33470
> 
> This bug number is actually from Python's bugzilla, not GDB's.  So it's
> not correct to mention it here in the ChangeLog/commit message.  AFAIK,
> there's no correspondent GDB bug filed for this issue.

Ok, I removed that.

> 
>> 	* python/python.c (do_start_initialization):
>> 	Avoid call to internal Python API.
>> 	(PyInit__gdb): New function.
>> 
>> diff --git a/gdb/python/python.c b/gdb/python/python.c
>> index c29e7d7a6b..89443aee25 100644
>> --- a/gdb/python/python.c
>> +++ b/gdb/python/python.c
>> @@ -1667,6 +1667,14 @@ finalize_python (void *ignore)
>>   restore_active_ext_lang (previous_active);
>> }
>> 
>> +#ifdef IS_PY3K
>> +PyMODINIT_FUNC
>> +PyInit__gdb (void)
>> +{
>> +  return PyModule_Create (&python_GdbModuleDef);
>> +}
>> +#endif
> 
> I think it's a good idea to add a comment to this function.

I added this (after the #ifdef):

/* This is called via the PyImport_AppendInittab mechanism called
   during initialization, to make the built-in _gdb module known to
   Python.  */

	paul

Re: [PATCH] fix build failure with Python 3.7

[Tom Tromey]

>>>>>   <Paul.Koning@dell.com> writes:

>> This bug number is actually from Python's bugzilla, not GDB's.  So it's
>> not correct to mention it here in the ChangeLog/commit message.  AFAIK,
>> there's no correspondent GDB bug filed for this issue.

> Ok, I removed that.

There may be other cases where we stick a URL of some non-gdb bug into
the ChangeLog (or at least the commit message), and that seems like it
would save some time for some future archaeologist, so I'd suggest doing
that.

Tom

Re: [PATCH] fix build failure with Python 3.7

[Phil Muldoon]

On 01/06/18 02:27, Tom Tromey wrote:
>>>>>>   <Paul.Koning@dell.com> writes:
> 
>>> This bug number is actually from Python's bugzilla, not GDB's.  So it's
>>> not correct to mention it here in the ChangeLog/commit message.  AFAIK,
>>> there's no correspondent GDB bug filed for this issue.
> 
>> Ok, I removed that.
> 
> There may be other cases where we stick a URL of some non-gdb bug into
> the ChangeLog (or at least the commit message), and that seems like it
> would save some time for some future archaeologist, so I'd suggest doing
> that.
> 
> Tom
> 

Hi all,

I think the best thing to do would be to create a GDB bug that then
has a reference to the Python bug tracking system.  There is a Red Hat
bugzilla entry for this, but from an upstream perspective, that's
neither here nor there.

Cheers,

Phil

Re: [PATCH] fix build failure with Python 3.7

[Phil Muldoon]

On 31/05/18 22:20, Sergio Durigan Junior wrote:
> On Thursday, May 31 2018, Paul Koning wrote:
> 
>>> On May 31, 2018, at 4:10 PM, Sergio Durigan Junior <sergiodj@redhat.com> wrote:
>>>
>>>>
>>>> gdb/ChangeLog:
>>>>
>>>> 2018-05-31  Paul Koning  <paul_koning@dell.com>
>>>>
>>>> 	PR gdb/33470
>>>
>>> This bug number is actually from Python's bugzilla, not GDB's.  So it's
>>> not correct to mention it here in the ChangeLog/commit message.  AFAIK,
>>> there's no correspondent GDB bug filed for this issue.
>>
>> Ok, I removed that.
>>
>>>
>>>> 	* python/python.c (do_start_initialization):
>>>> 	Avoid call to internal Python API.
>>>> 	(PyInit__gdb): New function.
>>>>
>>>> diff --git a/gdb/python/python.c b/gdb/python/python.c
>>>> index c29e7d7a6b..89443aee25 100644
>>>> --- a/gdb/python/python.c
>>>> +++ b/gdb/python/python.c
>>>> @@ -1667,6 +1667,14 @@ finalize_python (void *ignore)
>>>>   restore_active_ext_lang (previous_active);
>>>> }
>>>>
>>>> +#ifdef IS_PY3K
>>>> +PyMODINIT_FUNC
>>>> +PyInit__gdb (void)
>>>> +{
>>>> +  return PyModule_Create (&python_GdbModuleDef);
>>>> +}
>>>> +#endif
>>>
>>> I think it's a good idea to add a comment to this function.
>>
>> I added this (after the #ifdef):
>>
>> /* This is called via the PyImport_AppendInittab mechanism called
>>    during initialization, to make the built-in _gdb module known to
>>    Python.  */
> 
> Thanks.
> 
> FWIW, I ran the full testsuite here and no regressions were found.
> 

Hey Sergio,

Thanks for doing this. Did you test it against a GDB built against 3.7
and a GDB build with a 2.x version of Python?

Cheers

Phil

Re: [PATCH] fix build failure with Python 3.7

[Pedro Alves]

On 05/31/2018 09:45 PM, Paul.Koning@dell.com wrote:

>>> @@ -1667,6 +1667,14 @@ finalize_python (void *ignore)
>>>   restore_active_ext_lang (previous_active);
>>> }
>>>
>>> +#ifdef IS_PY3K
>>> +PyMODINIT_FUNC
>>> +PyInit__gdb (void)
>>> +{
>>> +  return PyModule_Create (&python_GdbModuleDef);
>>> +}
>>> +#endif
>>
>> I think it's a good idea to add a comment to this function.
> 
> I added this (after the #ifdef):
> 
> /* This is called via the PyImport_AppendInittab mechanism called
>    during initialization, to make the built-in _gdb module known to
>    Python.  */

Can the function be made static?

I'm a little surprised to see the function being named "Py...", since
that kind of looks like stepping in Python's namespace.

Thanks,
Pedro Alves

Re: [PATCH] fix build failure with Python 3.7

[Paul.Koning]

> On Jun 1, 2018, at 8:15 AM, Pedro Alves <palves@redhat.com> wrote:
> 
> On 05/31/2018 09:45 PM, Paul.Koning@dell.com wrote:
> 
>>>> @@ -1667,6 +1667,14 @@ finalize_python (void *ignore)
>>>>  restore_active_ext_lang (previous_active);
>>>> }
>>>> 
>>>> +#ifdef IS_PY3K
>>>> +PyMODINIT_FUNC
>>>> +PyInit__gdb (void)
>>>> +{
>>>> +  return PyModule_Create (&python_GdbModuleDef);
>>>> +}
>>>> +#endif
>>> 
>>> I think it's a good idea to add a comment to this function.
>> 
>> I added this (after the #ifdef):
>> 
>> /* This is called via the PyImport_AppendInittab mechanism called
>>   during initialization, to make the built-in _gdb module known to
>>   Python.  */
> 
> Can the function be made static?

No; I did that first but PyMODINIT_FUNC is a #define that conflicts with "static".

> I'm a little surprised to see the function being named "Py...", since
> that kind of looks like stepping in Python's namespace.

True.  How about "init__gdb_module"?

	paul

Re: [PATCH] fix build failure with Python 3.7

[Pedro Alves]

On 06/01/2018 01:54 PM, Paul.Koning@dell.com wrote:

>> Can the function be made static?
> 
> No; I did that first but PyMODINIT_FUNC is a #define that conflicts with "static".

I see.

>> I'm a little surprised to see the function being named "Py...", since
>> that kind of looks like stepping in Python's namespace.
> 
> True.  How about "init__gdb_module"?

Sounds fine to me.

Thanks,
Pedro Alves

Re: [PATCH] fix build failure with Python 3.7

[Paul.Koning]

Ok, here is the patch in its current state.  Ok to commit?

	paul

gdb/ChangeLog:

2018-06-01  Paul Koning  <paul_koning@dell.com>

	* python/python.c (do_start_initialization):
	Avoid call to internal Python API.
	(init__gdb_module): New function.

diff --git a/gdb/python/python.c b/gdb/python/python.c
index c29e7d7a6b..dbd2f1ab2d 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1667,6 +1667,17 @@ finalize_python (void *ignore)
   restore_active_ext_lang (previous_active);
 }
 
+#ifdef IS_PY3K
+/* This is called via the PyImport_AppendInittab mechanism called
+   during initialization, to make the built-in _gdb module known to
+   Python.  */
+PyMODINIT_FUNC
+init__gdb_module (void)
+{
+  return PyModule_Create (&python_GdbModuleDef);
+}
+#endif
+
 static bool
 do_start_initialization ()
 {
@@ -1707,6 +1718,9 @@ do_start_initialization ()
      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);
 #else
   Py_SetProgramName (progname.release ());
 #endif
@@ -1716,9 +1730,7 @@ do_start_initialization ()
   PyEval_InitThreads ();
 
 #ifdef IS_PY3K
-  gdb_module = PyModule_Create (&python_GdbModuleDef);
-  /* Add _gdb module to the list of known built-in modules.  */
-  _PyImport_FixupBuiltin (gdb_module, "_gdb");
+  gdb_module = PyImport_ImportModule ("_gdb");
 #else
   gdb_module = Py_InitModule ("_gdb", python_GdbMethods);
 #endif

Re: [PATCH] fix build failure with Python 3.7

[Pedro Alves]

On 06/01/2018 02:22 PM, Paul.Koning@dell.com wrote:
> Ok, here is the patch in its current state.  Ok to commit?

Yes, but recall to include the rationale in the git commit log, and
to include the link to the original Python issue in there.

Thanks,
Pedro Alves

Re: [PATCH] fix build failure with Python 3.7

[Pedro Alves]

On 06/01/2018 02:36 PM, Pedro Alves wrote:
> On 06/01/2018 02:22 PM, Paul.Koning@dell.com wrote:
>> Ok, here is the patch in its current state.  Ok to commit?
> 
> Yes, but recall to include the rationale in the git commit log, and
> to include the link to the original Python issue in there.

Actually, should this go to the 8.1 branch too?  If so, then per
standard procedure we will indeed need a GDB bugzilla bug filed
(and then a reference added to the ChangeLog).

Thanks,
Pedro Alves

Re: [PATCH] fix build failure with Python 3.7

[Paul.Koning]

> On Jun 1, 2018, at 9:37 AM, Pedro Alves <palves@redhat.com> wrote:
> 
> On 06/01/2018 02:36 PM, Pedro Alves wrote:
>> On 06/01/2018 02:22 PM, Paul.Koning@dell.com wrote:
>>> Ok, here is the patch in its current state.  Ok to commit?
>> 
>> Yes, but recall to include the rationale in the git commit log, and
>> to include the link to the original Python issue in there.
> 
> Actually, should this go to the 8.1 branch too?  If so, then per
> standard procedure we will indeed need a GDB bugzilla bug filed
> (and then a reference added to the ChangeLog).

I opened the bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23252

	paul

Re: [PATCH] fix build failure with Python 3.7

[Paul.Koning]

> On Jun 8, 2018, at 10:35 AM, Pedro Alves <palves@redhat.com> wrote:
> 
> Hi Paul,
> On 06/01/2018 02:47 PM, Paul.Koning@dell.com wrote:
>> 
>> 
>>> On Jun 1, 2018, at 9:37 AM, Pedro Alves <palves@redhat.com> wrote:
>>> 
>>> On 06/01/2018 02:36 PM, Pedro Alves wrote:
>>>> On 06/01/2018 02:22 PM, Paul.Koning@dell.com wrote:
>>>>> Ok, here is the patch in its current state.  Ok to commit?
>>>> 
>>>> Yes, but recall to include the rationale in the git commit log, and
>>>> to include the link to the original Python issue in there.
>>> 
>>> Actually, should this go to the 8.1 branch too?  If so, then per
>>> standard procedure we will indeed need a GDB bugzilla bug filed
>>> (and then a reference added to the ChangeLog).
>> 
>> I opened the bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23252
> Thanks.  AFAICT, everything's sorted out.  Anything holding this back?

Just waiting for approval to commit.

	paul

Re: [PATCH] fix build failure with Python 3.7

[Pedro Alves]

On 06/08/2018 03:37 PM, Paul.Koning@dell.com wrote:

>> Thanks.  AFAICT, everything's sorted out.  Anything holding this back?
> 
> Just waiting for approval to commit.
> 

Ah, but you already have it:

 https://sourceware.org/ml/gdb-patches/2018-06/msg00015.html

Sorry if that wasn't clear.

Thanks,
Pedro Alves

Re: [PATCH] fix build failure with Python 3.7

[Paul.Koning]

> On Jun 8, 2018, at 10:55 AM, Pedro Alves <palves@redhat.com> wrote:
> 
> On 06/08/2018 03:37 PM, Paul.Koning@dell.com wrote:
> 
>>> Thanks.  AFAICT, everything's sorted out.  Anything holding this back?
>> 
>> Just waiting for approval to commit.
>> 
> 
> Ah, but you already have it:
> 
> https://sourceware.org/ml/gdb-patches/2018-06/msg00015.html
> 
> Sorry if that wasn't clear.
> 
> Thanks,
> Pedro Alves

It was clear, but I managed to overlook the original message.

I committed the patch with a commit message that references the original (Red Hat) bug.

	paul

Re: [PATCH] fix build failure with Python 3.7

[Sergio Durigan Junior]

On Friday, June 08 2018, Paul Koning wrote:

>> On Jun 8, 2018, at 10:55 AM, Pedro Alves <palves@redhat.com> wrote:
>> 
>> On 06/08/2018 03:37 PM, Paul.Koning@dell.com wrote:
>> 
>>>> Thanks.  AFAICT, everything's sorted out.  Anything holding this back?
>>> 
>>> Just waiting for approval to commit.
>>> 
>> 
>> Ah, but you already have it:
>> 
>> https://sourceware.org/ml/gdb-patches/2018-06/msg00015.html
>> 
>> Sorry if that wasn't clear.
>> 
>> Thanks,
>> Pedro Alves
>
> It was clear, but I managed to overlook the original message.
>
> I committed the patch with a commit message that references the original (Red Hat) bug.

Thanks, Paul.  I've made a Fedora GDB release containing the patch now.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/