bpo-42603: Use pkg-config to get TCL/TK paths for tkinter. #23721
Conversation
|
Thank you for this patch. Please modify |
|
@E-Paine Done. Autoreconf also makes modifications unrelated to the actual change. I've left those out. |
|
I'm not really experienced enough in this area to be reviewing this, but maybe @serhiy-storchaka or @ned-deily you would like to take a look? |
|
If accepted, it would also make sense to backport this patch to a few versions back. Are separate PRs required for this, or will it be handled by a maintainer? |
|
I personally believe this to be an enhancement which is not typically backported. If the core who merges decides otherwise, they'll add labels and let one of the bots cherry-pick it. |
m000
force-pushed
the
fix-issue-42603
branch
2 times, most recently
from
b8c1b8b to
83372ca
Compare
|
@E-Paine Can you poke the Azure Pipelines test? IIRC, it succeeded the first time, but failed after only rewording the NEWS.d file. |
|
@m000 Unfortunately, I cannot do so as I have the same permissions as you. It is an unrelated asyncio failure, though, so can be safely ignored. |
serhiy-storchaka
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 83372ca3ebc11dc954f30e685863c1d70fbe8573 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
|
This PR is stale because it has been open for 30 days with no activity. |
|
Is it normal for the bot to mark "awaiting merge" requests as "stale"? |
|
"stale" status is a recent addition to our workflow meaning a PR that is awaiting review or other action for some time. Sorry for the delay, I do intend to review it in the near future. And thanks for your contribution. |
There was a problem hiding this comment.
This seems to me to be a generally useful enhancement. However, it does introduce a potentially subtle change in behavior on macOS: if you have Tck and Tk frameworks installed in /Library/Frameworks and you have Tcl and Tk installed elsewhere along with a pkg-config that references it (like, say, in a MacPorts environment), by default Python will now try to link with the Tcl/Tk pointed to by pkg-config rather than the /Library/Frameworks version. The behavior can be overriden on configure (./configure --with-tcltk-includes= --with-tcltk-libs=) or make (make TCLTK_INCLUDES= TCLTK_LIBS=). Since the primary documentation for all this is in the top-level setup.py, I think the comments there should be updated to reflect the changes in this PR.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Thanks for making the requested changes! @serhiy-storchaka, @ned-deily: please review the changes made to this pull request. |
bedevere-bot
requested review from
ned-deily and
serhiy-storchaka
and removed request for
a team,
ethanfurman,
markshannon,
pablogsal,
ilevkivskyi,
terryjreedy and
lysnikolaou
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Please do not squash. Reviewers can look at both individual commits, especially when designed for easier review, and the overall change (the 'Files changed' button at the top). I know I do that, and would here if I were reviewing this. I even make separate commits, for my own review, when making substantial PRs. As the big green button below says, we squash just before merging into cpython. |
|
I have made the requested changes; please review again.
|
|
Thanks for making the requested changes! @tiran, @ned-deily, @serhiy-storchaka: please review the changes made to this pull request. |
When paths are not supplied with
--with-tcltk-includesand--with-tcltk-libs, use pkg-config (if available) to get the values ofTCLTK_INCLUDESandTCLTK_LIBS. Makes easier to compile Python with a non-standard Tcl/Tk.https://bugs.python.org/issue42603