gh-99942: correct the pkg-config/python-config flags for cygwin/android#100967
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
…/android On shared build configurations, Cygwin and Android need to link to libpython. See bpo-21536. This was corrected in distutils with an explicit check to only link when libpython is built shared, but implemented in configure.ac unconditionally. The correct approach is to follow distutils, which includes a comment regarding why: - on Android, a shared libpython is RTLD_LOCAL, and thus available only to the main executable, not exported to loaded modules, but a static libpython *is* the main executable and thus exported - on Cygwin, symbols in shared libraries must be resolved at link time It's actually not clear to me what to do for Cygwin. Cygwin doesn't actually build static libpython successfully. If it did, then extensions probably need to be linked to the import library for the executable, rather than the full static library. So omitting this here is probably correct, but incomplete. Either way, it won't matter until other fixes are made.
eli-schwartzforce-pushed the configure-libpython branch from bb0fede to f40ce64Compare
ContributorAuthor
eli-schwartz commented Feb 17, 2023
Thanks for merging. BTW I notice the commit message got truncated during the merge... I spent quite some time writing up the context for it, because I expect it to be crucial knowledge for things like bisecting and git blame as it's not exactly obvious why these conditionals matter. What's the preferred CPython way for me to handle this? Should I add comments to configure.ac instead of writing commit messages? I was a bit afraid of making it overly long, since configure.ac serves many purposes and this is a minor one, and unlike distutils there is no dedicated file for the topic of handling platform flags. Still, if commit messages are discouraged in favor of inline code comments I can do that... |
Member
FFY00 commented Feb 17, 2023
We can easily find the PR from the commit, but having at least a small comment in The only option available to us is squash merge, so I need to copy the message to the commit body manually. I'll try to be more careful in the future. |
ContributorAuthor
eli-schwartz commented Feb 19, 2023
Thanks for clarifying. Also interesting 👀 because that seems like it would be deadly to the ability to cleanly bisect when merging complex PRs that cannot be untangled into multiple discrete PRs, but have multiple distinct commits to them. Although perhaps encouraging separate PRs renders that concern mostly irrelevant. ... I am surprised to learn that it's that difficult to get a good default commit message. Github offers 3 options for this in the settings page available to repository admins:
It sounds like the CPython repository is manually configured to option 1? I wonder why. |
dnicolodi pushed a commit to dnicolodi/meson that referenced this pull request Sep 8, 2023
Disagreement between distutils and sysconfig have been resolved for Python 3.12, see python/cpython#100356 and python/cpython#100967 This is the other half of the fix for mesonbuild#7702.
dnicolodi pushed a commit to dnicolodi/meson that referenced this pull request Sep 8, 2023
Disagreement between distutils and sysconfig have been resolved for Python 3.12, see python/cpython#100356 and python/cpython#100967 This is the other half of the fix for mesonbuild#7702.
dnicolodi pushed a commit to dnicolodi/meson that referenced this pull request Sep 8, 2023
Disagreement between distutils and sysconfig have been resolved for Python 3.12, see python/cpython#100356 and python/cpython#100967 This is the other half of the fix for mesonbuild#7702.
dnicolodi pushed a commit to dnicolodi/meson that referenced this pull request Sep 9, 2023
Disagreement between distutils and sysconfig have been resolved for Python 3.12, see python/cpython#100356 and python/cpython#100967 This is the other half of the fix for mesonbuild#7702.
eli-schwartz added a commit to eli-schwartz/meson that referenced this pull request Sep 22, 2023
…nt python On python >=3.8, this information is expected to be encoded in the sysconfig vars. In distutils, it is always necessary to link to libpython on Windows; for posix platforms, it depends on the value of LIBPYTHON (which is the library to link to, possibly the empty string) as generated by configure.ac and embedded into python.pc and python-config.sh, and then coded a second time in the distutils python sources. There are a couple of caveats which have ramifications for Cygwin and Android: - python.pc and python-config.sh disagree with distutils when python is not built shared. In that case, the former act the same as a shared build, while the latter *never* links to libpython - python.pc disagrees with python-config.sh and distutils when python is built shared. The former never links to libpython, while the latter do The disagreement is resolved in favor of distutils' behavior in all cases, and python.pc is correct for our purposes on python 3.12; see: python/cpython#100356python/cpython#100967 Although it was not backported to older releases, Cygwin at least has always patched in a fix for python.pc, which behavior is now declared canonical. We can reliably assume it is always correct. This is the other half of the fix for mesonbuild#7702
eli-schwartz added a commit to eli-schwartz/meson that referenced this pull request Oct 2, 2023
…nt python On python >=3.8, this information is expected to be encoded in the sysconfig vars. In distutils, it is always necessary to link to libpython on Windows; for posix platforms, it depends on the value of LIBPYTHON (which is the library to link to, possibly the empty string) as generated by configure.ac and embedded into python.pc and python-config.sh, and then coded a second time in the distutils python sources. There are a couple of caveats which have ramifications for Cygwin and Android: - python.pc and python-config.sh disagree with distutils when python is not built shared. In that case, the former act the same as a shared build, while the latter *never* links to libpython - python.pc disagrees with python-config.sh and distutils when python is built shared. The former never links to libpython, while the latter do The disagreement is resolved in favor of distutils' behavior in all cases, and python.pc is correct for our purposes on python 3.12; see: python/cpython#100356python/cpython#100967 Although it was not backported to older releases, Cygwin at least has always patched in a fix for python.pc, which behavior is now declared canonical. We can reliably assume it is always correct. This is the other half of the fix for mesonbuild#7702
eli-schwartz added a commit to eli-schwartz/meson that referenced this pull request Oct 3, 2023
…nt python On python >=3.8, this information is expected to be encoded in the sysconfig vars. In distutils, it is always necessary to link to libpython on Windows; for posix platforms, it depends on the value of LIBPYTHON (which is the library to link to, possibly the empty string) as generated by configure.ac and embedded into python.pc and python-config.sh, and then coded a second time in the distutils python sources. There are a couple of caveats which have ramifications for Cygwin and Android: - python.pc and python-config.sh disagree with distutils when python is not built shared. In that case, the former act the same as a shared build, while the latter *never* links to libpython - python.pc disagrees with python-config.sh and distutils when python is built shared. The former never links to libpython, while the latter do The disagreement is resolved in favor of distutils' behavior in all cases, and python.pc is correct for our purposes on python 3.12; see: python/cpython#100356python/cpython#100967 Although it was not backported to older releases, Cygwin at least has always patched in a fix for python.pc, which behavior is now declared canonical. We can reliably assume it is always correct. This is the other half of the fix for mesonbuild#7702 (cherry picked from commit 2d6c109)
robtaylor pushed a commit to robtaylor/meson that referenced this pull request Oct 14, 2023
…nt python On python >=3.8, this information is expected to be encoded in the sysconfig vars. In distutils, it is always necessary to link to libpython on Windows; for posix platforms, it depends on the value of LIBPYTHON (which is the library to link to, possibly the empty string) as generated by configure.ac and embedded into python.pc and python-config.sh, and then coded a second time in the distutils python sources. There are a couple of caveats which have ramifications for Cygwin and Android: - python.pc and python-config.sh disagree with distutils when python is not built shared. In that case, the former act the same as a shared build, while the latter *never* links to libpython - python.pc disagrees with python-config.sh and distutils when python is built shared. The former never links to libpython, while the latter do The disagreement is resolved in favor of distutils' behavior in all cases, and python.pc is correct for our purposes on python 3.12; see: python/cpython#100356python/cpython#100967 Although it was not backported to older releases, Cygwin at least has always patched in a fix for python.pc, which behavior is now declared canonical. We can reliably assume it is always correct. This is the other half of the fix for mesonbuild#7702
nirbheek pushed a commit to mesonbuild/meson that referenced this pull request Oct 17, 2023
…nt python On python >=3.8, this information is expected to be encoded in the sysconfig vars. In distutils, it is always necessary to link to libpython on Windows; for posix platforms, it depends on the value of LIBPYTHON (which is the library to link to, possibly the empty string) as generated by configure.ac and embedded into python.pc and python-config.sh, and then coded a second time in the distutils python sources. There are a couple of caveats which have ramifications for Cygwin and Android: - python.pc and python-config.sh disagree with distutils when python is not built shared. In that case, the former act the same as a shared build, while the latter *never* links to libpython - python.pc disagrees with python-config.sh and distutils when python is built shared. The former never links to libpython, while the latter do The disagreement is resolved in favor of distutils' behavior in all cases, and python.pc is correct for our purposes on python 3.12; see: python/cpython#100356python/cpython#100967 Although it was not backported to older releases, Cygwin at least has always patched in a fix for python.pc, which behavior is now declared canonical. We can reliably assume it is always correct. This is the other half of the fix for #7702
Volker-Weissmann pushed a commit to Volker-Weissmann/meson that referenced this pull request May 1, 2025
…nt python On python >=3.8, this information is expected to be encoded in the sysconfig vars. In distutils, it is always necessary to link to libpython on Windows; for posix platforms, it depends on the value of LIBPYTHON (which is the library to link to, possibly the empty string) as generated by configure.ac and embedded into python.pc and python-config.sh, and then coded a second time in the distutils python sources. There are a couple of caveats which have ramifications for Cygwin and Android: - python.pc and python-config.sh disagree with distutils when python is not built shared. In that case, the former act the same as a shared build, while the latter *never* links to libpython - python.pc disagrees with python-config.sh and distutils when python is built shared. The former never links to libpython, while the latter do The disagreement is resolved in favor of distutils' behavior in all cases, and python.pc is correct for our purposes on python 3.12; see: python/cpython#100356python/cpython#100967 Although it was not backported to older releases, Cygwin at least has always patched in a fix for python.pc, which behavior is now declared canonical. We can reliably assume it is always correct. This is the other half of the fix for mesonbuild#7702
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On shared build configurations, Cygwin and Android need to link to libpython. See bpo-21536.
This was corrected in distutils with an explicit check to only link when libpython is built shared, but implemented in configure.ac unconditionally. The correct approach is to follow distutils, which includes a comment regarding why:
on Android, a shared libpython is RTLD_LOCAL, and thus available only to the main executable, not exported to loaded modules, but a static libpython is the main executable and thus exported
on Cygwin, symbols in shared libraries must be resolved at link time
It's actually not clear to me what to do for Cygwin. Cygwin doesn't actually build static libpython successfully. If it did, then extensions probably need to be linked to the import library for the executable, rather than the full static library. So omitting this here is probably correct, but incomplete. Either way, it won't matter until other fixes are made.