gh-98707: Don't let --with-system-libmpdec / --with-system-expat use the vendored headers #98711
Conversation
Misc/NEWS.d/next/Build/2022-10-26-12-37-52.gh-issue-98707.eVXGEx.rst
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
…t use the vendored headers
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 python#98707
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
|
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 🤷🏽) |
|
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. |
|
I tested different configurations: Python build works as expected. main branchVendored libmpdec: System libmpdec: And LIBMPDEC_HEADERS is defined in Makefile with: This PRVendored libmpdec: System libmpdec without Modules/_decimal/libmpdec/ (removed): => 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): Good, as expected, MODULE_PYEXPAT_DEPS is empty. |
|
This change can be backported to Python 3.11. Python 3.10 doesn't seem to be affected: The build works without Modules/_decimal/libmpdec/ nor Modules/expat/ directories (removed). |
|
@erlend-aasland: Do you want to review again the PR? I plan to merge it soon. I'm now waiting for last buildbot jobs.
I always done that, so I'm fine with it :-) I prefer |
|
Thanks @hroncok for the PR, and @erlend-aasland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11. |
|
Sorry, @hroncok and @erlend-aasland, I could not cleanly backport this to |
|
GH-99391 is a backport of this pull request to the 3.11 branch. |
…stem-expat no longer include vendored headers (pythonGH-98711). (cherry picked from commit 6abec1c) Co-authored-by: Miro Hrončok <miro@hroncok.cz>
…pat no longer include vendored headers (python#98711)
This was a regression in Python 3.12.0a2 that prevented Fedora doing this:
Before building Python with --with-system-libmpdec --with-system-expat.
The errors were:
Now the make-dependency on the headers only exists when --with-system-libmpdec / --with-system-expat is not used.
Fixes #98707