gh-98707: Don't let --with-system-libmpdec / --with-system-expat use the vendored headers

[hroncok]

This was a regression in Python 3.12.0a2 that prevented Fedora doing this:

$ rm -r Modules/_decimal/libmpdec
$ rm -r Modules/expat

Before building Python with --with-system-libmpdec --with-system-expat.

The errors were:

make: *** No rule to make target 'Modules/_decimal/libmpdec/basearith.h', needed by 'Modules/_decimal/_decimal.o'. Stop.
make: *** No rule to make target 'Modules/expat/ascii.h', needed by 'Modules/pyexpat.o'. Stop.

Now the make-dependency on the headers only exists when --with-system-libmpdec / --with-system-expat is not used.

Fixes#98707

• Issue: --with-system-libmpdec / --with-system-expat still uses the vendored headers #98707

[merwok]

Python PRs avoid rebases and force pushes because it creates ghost notifications and generally makes it harder for reviewers to see what has changed. All PRs are squash merged so the number and contents of commits does not matter.

(Because of another github UI problem, sometimes the final commit message is a super-long, not useful concatenation of all commit messages, but 🤷🏽)

[hroncok]

Since there is no way of fixing the commit message in a new commit and I do not have merge rights, I've decided to correct the typo instead of leaving it up to the merge-committer who might not notice it. If there is a better way of ensuring the typo won't make it to the final commit message, I will gladly do it that way the next time. Sorry for the trouble.

[erlend-aasland]

Since there is no way of fixing the commit message in a new commit and I do not have merge rights, I've decided to correct the typo instead of leaving it up to the merge-committer who might not notice it. If there is a better way of ensuring the typo won't make it to the final commit message, I will gladly do it that way the next time. Sorry for the trouble.

Use the PR title. IIRC, we've updated the default commit message to be the PR title.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @encukou for commit cb7d840 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

[vstinner]

I tested different configurations: Python build works as expected.

main branch

Vendored libmpdec:

$ make distclean
$ ./configure CFLAGS="-O0" && make 2>&1|tee make.log
$ grep -E '^(MODULE_PYEXPAT_DEPS|MODULE__DECIMAL_DEPS|MODULE__ELEMENTTREE_DEPS)' Makefile
MODULE_PYEXPAT_DEPS=$(LIBEXPAT_HEADERS) $(LIBEXPAT_A)
MODULE__DECIMAL_DEPS=$(srcdir)/Modules/_decimal/docstrings.h $(LIBMPDEC_HEADERS) $(LIBMPDEC_A)
MODULE__ELEMENTTREE_DEPS=$(srcdir)/Modules/pyexpat.c $(LIBEXPAT_HEADERS) $(LIBEXPAT_A)
# no result: _decimal is not linked to libmpdec
$ ldd $(./python -c 'import _decimal; print(_decimal.__file__)')|grep libmpdec

System libmpdec:

$ make distclean
$ ./configure CFLAGS="-O0" --with-system-libmpdec && make 2>&1|tee make.log
$ grep -E '^(MODULE_PYEXPAT_DEPS|MODULE__DECIMAL_DEPS|MODULE__ELEMENTTREE_DEPS)' Makefile
MODULE_PYEXPAT_DEPS=$(LIBEXPAT_HEADERS) $(LIBEXPAT_A)
MODULE__DECIMAL_DEPS=$(srcdir)/Modules/_decimal/docstrings.h $(LIBMPDEC_HEADERS) MODULE__ELEMENTTREE_DEPS=$(srcdir)/Modules/pyexpat.c $(LIBEXPAT_HEADERS) $(LIBEXPAT_A)
$ ldd $(./python -c 'import _decimal; print(_decimal.__file__)')|grep libmpdec
libmpdec.so.3 => /lib64/libmpdec.so.3 (0x00007f79499af000)

And LIBMPDEC_HEADERS is defined in Makefile with:

LIBMPDEC_HEADERS= \
$(srcdir)/Modules/_decimal/libmpdec/basearith.h \
$(srcdir)/Modules/_decimal/libmpdec/bits.h \
$(srcdir)/Modules/_decimal/libmpdec/constants.h \
$(srcdir)/Modules/_decimal/libmpdec/convolute.h \
$(srcdir)/Modules/_decimal/libmpdec/crt.h \
$(srcdir)/Modules/_decimal/libmpdec/difradix2.h \
$(srcdir)/Modules/_decimal/libmpdec/fnt.h \
$(srcdir)/Modules/_decimal/libmpdec/fourstep.h \
$(srcdir)/Modules/_decimal/libmpdec/io.h \
$(srcdir)/Modules/_decimal/libmpdec/mpalloc.h \
$(srcdir)/Modules/_decimal/libmpdec/mpdecimal.h \
$(srcdir)/Modules/_decimal/libmpdec/numbertheory.h \
$(srcdir)/Modules/_decimal/libmpdec/sixstep.h \
$(srcdir)/Modules/_decimal/libmpdec/transpose.h \
$(srcdir)/Modules/_decimal/libmpdec/typearith.h \
$(srcdir)/Modules/_decimal/libmpdec/umodarith.h

This PR

Vendored libmpdec:

$ make distclean
$ ./configure CFLAGS="-O0" && make 2>&1|tee make.log
$ grep -E '^(MODULE_PYEXPAT_DEPS|MODULE__DECIMAL_DEPS|MODULE__ELEMENTTREE_DEPS)' Makefile
MODULE_PYEXPAT_DEPS=$(LIBEXPAT_HEADERS) $(LIBEXPAT_A)
MODULE__DECIMAL_DEPS=$(srcdir)/Modules/_decimal/docstrings.h $(LIBMPDEC_HEADERS) $(LIBMPDEC_A)
MODULE__ELEMENTTREE_DEPS=$(srcdir)/Modules/pyexpat.c $(LIBEXPAT_HEADERS) $(LIBEXPAT_A)
# no result: _decimal is not linked to libmpdec
$ ldd $(./python -c 'import _decimal; print(_decimal.__file__)')|grep libmpdec

System libmpdec without Modules/_decimal/libmpdec/ (removed):

$ rm -rf Modules/_decimal/libmpdec/
$ make distclean
$ ./configure CFLAGS="-O0" --with-system-libmpdec && make 2>&1|tee make.log
$ grep -E '^(MODULE_PYEXPAT_DEPS|MODULE__DECIMAL_DEPS|MODULE__ELEMENTTREE_DEPS)' Makefile
MODULE_PYEXPAT_DEPS=$(LIBEXPAT_HEADERS) $(LIBEXPAT_A)
MODULE__DECIMAL_DEPS=$(srcdir)/Modules/_decimal/docstrings.h MODULE__ELEMENTTREE_DEPS=$(srcdir)/Modules/pyexpat.c $(LIBEXPAT_HEADERS) $(LIBEXPAT_A)
$ ldd $(./python -c 'import _decimal; print(_decimal.__file__)')|grep libmpdec
libmpdec.so.3 => /lib64/libmpdec.so.3 (0x00007f3aa03ff000)

=> Good, as expected, LIBMPDEC_HEADERS no longer in MODULE__DECIMAL_DEPS.

I also tested: system libmpdec without Modules/_decimal/libmpdec/ (removed) and system expat without Modules/expat/ (removed):

$ rm -rf Modules/_decimal/libmpdec/ Modules/expat/
$ make distclean
$ ./configure CFLAGS="-O0" --with-system-libmpdec --with-system-expat && make 2>&1|tee make.log
$ grep -E '^(MODULE_PYEXPAT_DEPS|MODULE__DECIMAL_DEPS|MODULE__ELEMENTTREE_DEPS)' Makefile
MODULE_PYEXPAT_DEPS=
MODULE__DECIMAL_DEPS=$(srcdir)/Modules/_decimal/docstrings.h MODULE__ELEMENTTREE_DEPS=$(srcdir)/Modules/pyexpat.c $ ldd $(./python -c 'import _decimal; print(_decimal.__file__)')|grep libmpdec
libmpdec.so.3 => /lib64/libmpdec.so.3 (0x00007fe29b855000)
$ ldd $(./python -c 'import pyexpat; print(pyexpat.__file__)')|grep expat
libexpat.so.1 => /lib64/libexpat.so.1 (0x00007f763abea000)

Good, as expected, MODULE_PYEXPAT_DEPS is empty.

[vstinner]

LGTM.

[vstinner]

This change can be backported to Python 3.11.

Python 3.10 doesn't seem to be affected:

$ git clean -fdx
$ rm -rf Modules/_decimal/libmpdec/ Modules/expat/
$ ./configure CFLAGS="-O0" --with-system-libmpdec --with-system-expat
$ make 2>&1|tee make.log
$ ldd $(./python -c 'import _decimal; print(_decimal.__file__)')|grep libmpdec
libmpdec.so.3 => /lib64/libmpdec.so.3 (0x00007f1edc402000)
$ ldd $(./python -c 'import pyexpat; print(pyexpat.__file__)')|grep expat
libexpat.so.1 => /lib64/libexpat.so.1 (0x00007f7a66b87000)

The build works without Modules/_decimal/libmpdec/ nor Modules/expat/ directories (removed).

[vstinner]

@erlend-aasland: Do you want to review again the PR?

I plan to merge it soon. I'm now waiting for last buildbot jobs.

Since there is no way of fixing the commit message in a new commit and I do not have merge rights, I've decided to correct the typo instead of leaving it up to the merge-committer who might not notice it.

I always done that, so I'm fine with it :-) I prefer git commit --amend to fix the commit message. Otherwise, there is a risk that the wrong commit message is picked :-(

[erlend-aasland]

LGTM. Thanks Miro for finding and fixing this, and thanks Victor for verifying the fix :)

[miss-islington]

Thanks @hroncok for the PR, and @erlend-aasland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

[miss-islington]

Sorry, @hroncok and @erlend-aasland, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 6abec1caffdba2e282b14fe57c6ce61974de4bbe 3.11

[bedevere-bot]

GH-99391 is a backport of this pull request to the 3.11 branch.