Incorporate Tcl/Tk/Tix into the Windows build process #60172
Comments
|
This patch incorporates Tcl/Tk/Tix into the MSVC build in the same fashion as OpenSSL has been done. Highlights:
|
|
The current patch doesn't apply cleanly anymore and contained an accidental reversion of a commit, so I took the liberty of creating an updated version. I'll also be leaving comments on Rietveld. |
|
I'm not sure that moving the tcltk output dir into PCbuild is the right way to go about making different versions of Tcl/Tk available to different Python versions. For one, because it's not true if you're working out of a single checkout for multiple versions. I think I'd rather see 'tcltk'/'tcltk64' stay where they are, but grow extensions to denote different builds--'tcltk-Win32-py34' and 'tcltk-x64-py35', for example. Also, PCbuild/rt.bat needs to be updated if tcltk[64] is changed. I can't comment much on buildlib.py as I haven't had the time to learn everything I'd need to to understand it all, but it seems to work well doing what it is meant to do. However, it does seem like substantial overkill for what the comment at the top says it is for :P Barring the issues I've raised so far, I really like the general idea and direction of the patch. I really hope this can make it to inclusion before too long. |
|
Here's a new patch based on Jeremy's that addresses all of my review comments and includes the update to Tcl/Tk 8.6.1. Also, I'll be attaching a patch to tix-8.4.3.x which allows it to be built in debug configuration and removes many warnings about unrecognized command line options. |
|
I started looking at this again recently, and discovered that building from within Visual Studio doesn't work with the proposed patches. So, here's a different approach that adds 'Makefile' projects for each of Tcl, Tk, and Tix instead of using build_tkinter.py. The "NMakeBuildCommandLine" for each new project is actually a short, fairly straightforward script that first checks for the necessary components in the install location ($(tcltkDir)) and bails out with a success message if they're already there, or builds if they're not. The Tix project still requires http://bugs.python.org/file32998/issue15968_tix.svndiff to function properly on Debug builds (which means this patch isn't quite right, the proper Tix version number will be 8.4.3.4). There are a couple of changes that are made trivial by the main change here, namely copying the Tcl and Tk DLLs into the output dir where _tkinter can find them, which means PCbuild/rt.bat doesn't need to mess with PATH (nor do individuals need to change PATH when testing Tkinter manually). To simplify the new projects, I also simplified I've not tested every possible configuration, but I can confirm that Debug and Release configurations work as expected on both Win32 and x64 platforms, whether built by command line (msbuild) or Visual Studio. There is an issue with PGInstrument/PGUpdate builds on Win32 (and probably x64, untested) where One thing I'm not certain about, should this go into maintenance branches (namely 3.4, possibly 3.3), or just default (after 3.4 is branched)? |
|
I wouldn't change the maintenance branches. I expect that the buildbots will break when the patch is applied, since they will fail to fetch the tix sources (unless I'm missing something). Nevertheless, I like the approach, hoping that you'll be available to fix urgent issues that arise once it is applied. |
|
I haven't looked at the patch, but +1 to anything which brings |
|
Martin: I'll wait until after 3.4 is branched to commit to default, and make sure I have some time to pick up the pieces when I do :). The buildbots should be fine, though; external-common.bat is patched to pull in the tix sources as well. issue15968_tix.svndiff will need to be applied to http://svn.python.org/projects/external/tix-8.4.3.x and be tagged as tix-8.4.3.4 before changes are made to the cpython repo, though, but I don't have access to svn.python.org (nor enough experience with svn to be comfortable making the change). Could you possibly do that for me? |
|
I'm curious as to the issues that arose in getting 'build_tkinter.py' |
|
The problem with the previous approach and building inside Visual Studio was that makefile.vc in both Tcl and Tk first checks for one of "MSDEVDIR", "MSVCDIR", "VCINSTALLDIR", "MSSDK" or "WINDOWSSDKDIR" being set, which Visual Studio doesn't set. That issue could have been fixed basically the same way I fixed it in the new approach (set "MSDEVDIR" to a dummy value, probably in build_tkinter.py), but the new approach strikes me as being simpler and cleaner. Not to mention that without the dependency python.vcxproj, tcl/tk/tix can be built in parallel with other projects (like pythoncore, the longest-running Python project) using the msbuild /m switch. I welcome having more eyes on it :). I definitely won't be committing before 3.4.0 final (scheduled for the 16th), and it may be a week or better after that before I have good opportunity. |
|
If there are no objections before then, I'll split out the Tix parts of this patch and commit the rest this weekend. I'll open a new issue for adding Tix to the mix. |
|
New changeset c2e2dc6c8769 by Zachary Ware in branch 'default': |
|
Committed. Instead of splitting out the Tix stuff, I just disabled building Tix in Debug configuration. I will open a new issue for enabling the Debug Tix build. Thank you Martin and Tim for the votes of confidence, and Jeremy for starting this off! |
|
New changeset c12cc78d59c1 by Zachary Ware in branch 'default': |
|
It looks like test_idlelib fails on Windows buildbots since the changeset c2e2dc6c8769b6f37638149a9e9d0aad5845b3f1: issue bpo-21059. |
|
I just pulled and recompiled and got makefile error messages that I do not remember seeing: I also got 6 failures instead of the usual 3 or 4. On the other hand, import idlelib.idle worked and the test_idle work both alone and when running everything. |
|
Terry J. Reedy added the comment:
This is the output of this line:
The rest of the messages are due to the following line:
MSbuild just feels the need to output the entire command when it gets The "error: ..\..\tix-8.4.3.4 doesn't exist" implies that you haven't
test_idle will most likely fail if you run the tkinter tests first |
|
I reran external.bat in all three versions and compile runs better. Test_idle with -ugui fails to find _tkinter after another test that runs tk (including itself) *in the same process*. Adding -j2 to run in separate process eliminates the problem. So does not using test.test_support to run the tests. The following runs fine either at a command line or within Idle. (Does unittest run tests in a subprocess?) from test import support; support.use_resources = ['gui']
import unittest
unittest.main('test.test_tcl', exit=False)
unittest.main('test.test_idle', exit=False) |
|
Terry: the difference you saw between using regrtest and using unittest.main from the interactive prompt was because of regrtest.saved_test_environment: tkinter._fix sets environment variables at the beginning of one test which are cleared at the end by saved_test_environment, and not reset at the beginning of the next. bpo-20035 has my preferred fix for that situation. Mark: Glad this helped, and thanks for the ping. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: