Bug 6407 - Cannot build mercurial with Python 3.9: broken hgdemandinfo
Summary: Cannot build mercurial with Python 3.9: broken hgdemandinfo
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: Mercurial (show other bugs)
Version: 5.4
Hardware: PC Linux
: wish feature
Assignee: Bugzilla
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-09-08 09:08 UTC by Petr Stodulka
Modified: 2020-09-24 00:00 UTC (History)
4 users (show)

See Also:
Python Version: ---


Attachments
hgdemandimport fix (479 bytes, patch)
2020-09-08 09:08 UTC, Petr Stodulka
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Petr Stodulka 2020-09-08 09:08 UTC
Created attachment 2086 [details]
hgdemandimport fix

Hi guys,
we have discovered problems during building of mercurial 5.4 with Python 3.9 [0] regarding changes in module loader. The issue is discussed with Python upstream [1]. We already used the patch in attachment to fix the problem for now.

Actual results for mercurial 5.4:

--- snippet ----
make -C doc
make[1]: Entering directory '/builddir/build/BUILD/mercurial-5.4/doc'
/usr/bin/python3 gendoc.py "hg-ssh.8" > hg-ssh.8.txt.tmp
Traceback (most recent call last):
  File "/builddir/build/BUILD/mercurial-5.4/doc/gendoc.py", line 39, in <module>
    from mercurial.i18n import (
  File "/usr/lib64/python3.9/importlib/util.py", line 245, in __getattribute__
    self.__spec__.loader.exec_module(self)
SystemError: <built-in function compile> returned NULL without setting an error
make[1]: *** [Makefile:22: hg-ssh.8.txt] Error 1
make[1]: Leaving directory '/builddir/build/BUILD/mercurial-5.4/doc'
make: *** [Makefile:64: doc] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.j1o0Ut (%build)
---------------

Additional info:
The bug 6268 [2] is related to demandimport as well, but probably it's not related to this particular issue.


[0] https://bugzilla.redhat.com/show_bug.cgi?id=1871992
[1] https://bugs.python.org/issue41631
[2] https://bz.mercurial-scm.org/show_bug.cgi?id=6268
Comment 1 Petr Viktorin 2020-09-08 09:41 UTC
Hello, Python developer here.

This incompatibility was discovered rather late in the Python 3.9 development cycle; it might be too late to make the _ast module work with Mercurial's lazy-loading scheme. I see that there is already a big list of extensions that can't be imported lazily.
How much trouble would it be to add _ast on Mercurial's side?

It might be better to reference Python's issue41631 in the comment.
Comment 2 Manuel Jacob 2020-09-08 09:55 UTC
This was discussed a few days ago on IRC (maybe you brought it up). I could not reproduce it with a freshly compiled Python 3.9.0rc1 on Arch Linux. If I remember correctly, the person bring it up couldn’t reproduce it with a self-compiled version of Python 3.9.0rc1 on Fedora. It seemed like it was a Fedora-only (or Red Hat-only) problem. Was anyone able to reproduce the problem on a non-Red Hat system?

A SystemError means that it is an internal Python interpreter error. The error message indicates that there is a problem at the interpreter side. This doesn’t necessarily mean that the bug is caused only from the Python side (it could also be the case that Mercurial causes the problem but Python raises the wrong exception).

Does Mercurial’s lazy import mechanism violate some interface or protocol? If this is the case, we should try to fix the problem at the root.

Does the initialization of the _ast module in the Python interpreter violate some interface or protocol? If this is the case, it should ideally be fixed in Python. If this isn’t possible before the Python 3.9 final release, we can include the workaround from your patch (it should also add a comment "workaround for ..."), but this is not an ideal solution because it doesn’t fix the problem for users of older versions of Mercurial.

If the problem is caused by a downstream Fedora modification of CPython, the workaround should be applied to the downstream Fedora Mercurial package.
Comment 3 Manuel Jacob 2020-09-08 10:31 UTC
Sorry, I was wrong about not being able to reproduce it on Arch Linux. When I tried again, it now fails the same way as reported. With a debug build, I get "Python/Python-ast.c:231: get_ast_state: Assertion `state != NULL' failed.".
Comment 4 Petr Viktorin 2020-09-09 07:56 UTC
The issue is that the _ast module expects that importing "_ast" will result in the actual built-in module, not the lazy-loading proxy.
In code: https://github.com/python/cpython/blob/master/Python/Python-ast.c#L243-L253
I consider that a bug, but we're running out of time before the 3.9.0 final release.

I'm afraid there's not really a good interface or protocol around loaders *replacing* built-in modules; Mercurial's lazy loading is a great feature, but to be honest it's not tested as well as it could be in CPython. 
importlib.util.LazyLoader "postpones the execution of the loader of a module until the module has an attribute accessed", but there's unfortunately no guarantee that atribute access is the first/only thing a module is needed for :(
Comment 5 HG Bot 2020-09-14 12:05 UTC
Fixed by https://mercurial-scm.org/repo/hg/rev/81b4e7c866ec
Augie Fackler <augie@google.com>
hgdemandimport: bypass demandimport for _ast module (issue6407)

This is broken on Python 3.9rc1, and while it sounds like there may be
a fix in Python, we probably also should have this workaround in place
in hg. See the bug for more details (including on bugs at redhat and
b.p.o).

Differential Revision: https://phab.mercurial-scm.org/D9004

(please test the fix)
Comment 6 Petr Viktorin 2020-09-16 07:58 UTC
In the end, the issue should be fixed in Python 3.9.0rc2 (with major help from Victor Stinner).
So, sorry for the noise!

For Python 3.10, I plan to make Python access an attribute (and thus fully load LazyLoader modules) when getting C-level module state, so this bug doesn't happen in other built-in/extension modules.
Comment 7 Victor Stinner 2020-09-16 08:19 UTC
FYI Python 3.9 bug is fixed and the fix will be part of Python 3.9.0rc2 (scheduled for tomorrowed):
https://bugs.python.org/issue41631
Comment 8 Bugzilla 2020-09-24 00:00 UTC
Bug was set to TESTING for 7 days, resolving