[Python-apps-team] Bug#943608: tests segfault when run with Python 3.8

[Daniele Nicolodi]

On 25/12/2019 11:41, Pierre-Elliott Bécue wrote:
> Le mardi 10 décembre 2019 à 12:27:35-0700, Daniele Nicolodi a écrit :
>> Hello,
>>
>> On Tue, 26 Nov 2019 13:06:24 +0100 "Dr. Tobias Quathamer"
>> <toddy at debian.org> wrote:> I've forwarded this bug upstream, and further
>> investigation by them
>>> seems to indicate that this is actually a bug in CPython.
>>>
>>> I'm therefore cloning and reassigning this bug, feel free to
>>> revert this if you don't agree.
>>
>> A while ago I opened a merge request with upstream with a workaround
>> that fixes the segfault with CPython 3.8. As it seems that CPython 3.8.1
>> will be released with the bug not fixed, it may be worth to apply the
>> patch to the Debian package.
>>
>> https://bitbucket.org/blais/beancount/pull-requests/139
>>
>> All the patch does is to make the C code Python 3.9 ready, avoiding
>> raising a warning, which ultimately results in the segfault. The patch
>> seems very low risk to me.
> 
> Dear Daniele,
> 
> Thanks for the patch.
> 
> I adapted it a little to reduce it's volume:
> 
> https://salsa.debian.org/python-team/applications/beancount/blob/debian/master/debian/patches/0003-parser-Work-around-python3.8-bug-causing-a-segmentat.patch
> 
> The #define PY_SSIZE_T_CLEAN was in both parser.h and parser.c, and the
> removal of the two other includes seems out of scope for this bugfix
> IMO.
> 
> Is it fine with you?

Because the PY_SSIZE_T_CLEAN define modifies the API exposed by
Python.h, I think it is better to explicitly define it everywhere
Python.h is included. However, if the reduced version solves the problem
you are seeing, I don't see a reason to insist.

Please note that removing the other two includes is technically also a
bug-fix as those are technically two Python internal header files. I
removed the includes because I am not sure how including those files
directly interacts with the changes to the public API enabled by
PY_SSIZE_T_CLEAN. Again, if you don't see any problem, I don't see a
reason to insist.

The patch I proposed has been merged upstream, along with another that
solves another problem in the test suite with Python 3.8 (although I
didn't encounter this second bug in my testing). You may want to base
your package on a more recent (but unreleased) version. Since the 2.2.3
release I see mainly bug-fixes and improvements to the emacs
beancount-mode (which I don't know if you package along with the
application).

Cheers,
Dan