Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TIP 538 prevents building Tkinter against Tcl 8.7 built with bundled libtommath #103839

Closed
Tracked by #104568
chrstphrchvz opened this issue Apr 25, 2023 · 7 comments
Closed
Tracked by #104568
Labels
build The build process and cross-build topic-tkinter type-bug An unexpected behavior, bug, or error

Comments

@chrstphrchvz
Copy link
Contributor

chrstphrchvz commented Apr 25, 2023

Bug report

Separated out from #103194 (comment) since I believe this likely should be addressed first.

TIP 538 means that Tcl 8.7 can be built to use separately-built libtommath, and no longer recommends building with the bundled libtommath. tclTomMath.h requires tommath.h, which Tcl will no longer install a copy of even when the bundled libtommath is used (as another user recently found out: https://core.tcl-lang.org/tcl/info/ff255adc4cb5). Either tommath.h must be obtained separately, or TCL_NO_TOMMATH_H must be defined before including tclTomMath.h.

Package managers presumably would want to build Tcl against separately-built libtommath once they update to 8.7, and in turn build Tkinter against separately-built Tcl and libtommath. But given that nothing else in CPython uses libtommath, how much would the official Python binary distributions prefer that Tcl continue building its bundled libtommath?

Linked PRs

@ned-deily
Copy link
Member

Presumably this is only an issue for the Windows builds and for macOS installer builds? My initial reaction is that we provide a Tcl binary solely to run Tk in turn solely to run tkinter-based applications; we have no need to provide a full Tcl installation with all its third-party dependencies as many of them duplicate functionality provided directly by Python and can only somewhat indirectly used via tkinter. So, if libtommath falls into that category, adding a something (like TCL_NO_TOMMATH_H to our builds of Tcl seems fine. Other opinions?

@terryjreedy
Copy link
Member

@serhiy-storchaka @zooba tcl/tk build question. I suspect your answer will match Ned's.

@arhadthedev arhadthedev added type-feature A feature request or enhancement topic-tkinter build The build process and cross-build type-bug An unexpected behavior, bug, or error and removed type-bug An unexpected behavior, bug, or error type-feature A feature request or enhancement labels Apr 26, 2023
@zooba
Copy link
Member

zooba commented Apr 28, 2023

Yeah, I'd rather omit the dependency. (I couldn't find a nice way to omit the zlib DLL dependency, which is why we now have to carry a second copy of zlib, but let's not use that as precedent to add more unnecessary weight.)

@terryjreedy
Copy link
Member

@ned-deily @zooba Do you comments constitute approval of #103842? (I suspect so but don't understand build stuff.) If so, please say so there and maybe one of you merge it. Should it be backported?

@zooba
Copy link
Member

zooba commented May 17, 2023

That PR seems to be a fairly trivial preprocessor check? Assuming it works, it doesn't bother me at all. Python 3.13 is the first that might include Tcl 8.7 on Windows, so if it helps people who are building on other platforms, it can go wherever it likes.

Or did you mean a different PR?

@ned-deily
Copy link
Member

ned-deily commented May 19, 2023

I've merged the PR (thanks!) to main for inclusion in 3.12. Rather than set potentially false expectations, I think we should not backport it until and unless we decide to backport future PRs to fully support 8.7.

@terryjreedy
Copy link
Member

As I mentioned on the tracking issue #104568, I decided to backport at least the test fixes to 3.11 and note things not backported.

carljm added a commit to gsallam/cpython_with_perfmap_apii that referenced this issue May 20, 2023
* main: (30 commits)
  pythongh-103987: fix several crashes in mmap module (python#103990)
  docs: fix wrong indentation causing rendering error in dis page (python#104661)
  pythongh-94906: Support multiple steps in math.nextafter (python#103881)
  pythongh-104472: Skip `test_subprocess.ProcessTestCase.test_empty_env` if ASAN is enabled (python#104667)
  pythongh-103839: Allow building Tkinter against Tcl 8.7 without external libtommath (pythonGH-103842)
  pythongh-85984: New additions and improvements to the tty library. (python#101832)
  pythongh-104659: Consolidate python examples in enum documentation (python#104665)
  pythongh-92248: Deprecate `type`, `choices`, `metavar` parameters of `argparse.BooleanOptionalAction` (python#103678)
  pythongh-104645: fix error handling in marshal tests (python#104646)
  pythongh-104600: Make type.__type_params__ writable (python#104634)
  pythongh-104602: Add additional test for listcomp with lambda (python#104639)
  pythongh-104640: Disallow walrus in comprehension within type scopes (python#104641)
  pythongh-103921: Rename "type" header in argparse docs (python#104654)
  Improve readability of `typing._ProtocolMeta.__instancecheck__` (python#104649)
  pythongh-96522: Fix deadlock in pty.spawn (python#96639)
  pythonGH-102818: Do not call `PyTraceBack_Here` in sys.settrace trampoline.  (pythonGH-104579)
  pythonGH-103545: Add macOS specific constants for ``os.setpriority`` to ``os`` (python#104606)
  pythongh-104623: Update macOS installer to SQLite 3.42.0 (pythonGH-104624)
  pythongh-104619: never leak comprehension locals to outer locals() (python#104637)
  pythongh-104602: ensure all cellvars are known up front (python#104603)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build The build process and cross-build topic-tkinter type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

5 participants