classification
Title: Incorporate Tcl/Tk/Tix into the Windows build process
Type: enhancement Stage: resolved
Components: Build, Windows Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: zach.ware Nosy List: BreamoreBoy, haypo, jeremy.kloth, jkloth, loewis, python-dev, serhiy.storchaka, terry.reedy, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2012-09-18 19:36 by jkloth, last changed 2014-07-16 20:33 by zach.ware. This issue is now closed.

Files
File name Uploaded Description Edit
029d1cdf6422.diff jkloth, 2012-10-22 13:18 review
issue15968_update.diff zach.ware, 2013-06-14 21:06 Updated patch review
issue15968.v2.diff zach.ware, 2013-12-05 22:35 review
issue15968_tix.svndiff zach.ware, 2013-12-05 22:36 Patch to tix-8.4.3.x
issue15968.v3.diff zach.ware, 2014-03-12 18:44 review
Repositories containing patches
https://jkloth@bitbucket.org/jkloth/cpython-tcltk
Messages (20)
msg170668 - (view) Author: Jeremy Kloth (jkloth) * Date: 2012-09-18 19:36
This patch incorporates Tcl/Tk/Tix into the MSVC build in the same fashion as OpenSSL has been done.

Highlights:
- A new project, tcltk, is added that simply calls the Python script build_tkinter.py to build the externals.
- New helper module PCbuild/buildlib.py to parse and evaluate the MSBuild property files.
- build_tkinter.py is updated to use MSBuild properties for locating the sources.
- The tcltk output directory is now inside the PCbuild directory to enable different Tcl/Tk/Tix versions for different Python versions
msg191153 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-06-14 21:06
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.
msg192907 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-07-11 21:12
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.
msg205326 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-12-05 22:35
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.
msg213275 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-03-12 18:44
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 $(tcltkDir)/$(tcltk64Dir) and $(tcltk[64]Lib)/$(tcltk[64]LibDebug): there is now only $(tcltkDir) and $(tcltkLib).  x64.props redefines $(tcltkDir) to be $(externalsDir)\tcltk64, and $(tcltkLib) is defined in terms of $(tcltkDir) and $(TclDebugExt).

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 $(OutDir) is not set properly for the Tcl and Tk projects, and thus their DLLs are copied into $(SolutionDir) instead of the expected output dir.  There are other (unrelated to tkinter) issues with building PGI and PGO, though, so I don't consider that problem a blocker for this issue.

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)?
msg213281 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-03-12 19:50
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.
msg213283 - (view) Author: Tim Golden (tim.golden) * (Python committer) Date: 2014-03-12 19:51
I haven't looked at the patch, but +1 to anything which brings 
Tcl/Tk/Tix support into a state of default usability. Thanks for picking 
this up, Zachary.
msg213284 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-03-12 20:05
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?
msg213325 - (view) Author: Jeremy Kloth (jeremy.kloth) Date: 2014-03-12 23:17
I'm curious as to the issues that arose in getting 'build_tkinter.py'
to work within the Visual Studio IDE, as that is what I used to
develop the patches to start with.  I would like to look over the new
changes, but will not have any time to do so until 3-15-2014 at the
earliest.  Hopefully that is still enough time.
msg213403 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-03-13 14:52
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.
msg214140 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-03-19 21:29
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.
msg214436 - (view) Author: Roundup Robot (python-dev) Date: 2014-03-22 03:58
New changeset c2e2dc6c8769 by Zachary Ware in branch 'default':
Issue #15968: Incorporated Tcl, Tk, and Tix builds into the Windows build
http://hg.python.org/cpython/rev/c2e2dc6c8769
msg214437 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-03-22 04:08
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!
msg214445 - (view) Author: Roundup Robot (python-dev) Date: 2014-03-22 05:36
New changeset c12cc78d59c1 by Zachary Ware in branch 'default':
Issue #15968: Temporarily revert change to PCbuild/rt.bat
http://hg.python.org/cpython/rev/c12cc78d59c1
msg214785 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-03-25 08:21
It looks like test_idlelib fails on Windows buildbots since the changeset c2e2dc6c8769b6f37638149a9e9d0aad5845b3f1: issue #21059.
msg215081 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-03-28 22:07
I just pulled and recompiled and got makefile error messages that I do not remember seeing:
13>EXEC : error : ..\..\tix-8.4.3.4 doesn't exist.
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: The command "
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: IF EXIST ..\..\tcltk\lib\tix8.4.3\tix84g.dll (
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073:     echo Tix is already built and available.
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073:     exit /b 0
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: )
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: 
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: IF NOT EXIST ..\..\tix-8.4.3.4 (
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073:     echo error: ..\..\tix-8.4.3.4 doesn't exist.
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073:     exit 1
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: )
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: 
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: IF "Win32" EQU "Win32" set TclMachine=IX86
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: IF "Win32" EQU "x64" set TclMachine=AMD64
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: 
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: IF "Debug" EQU "Debug" (
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073:     set TixDebug=1
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: ) ELSE (
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073:     set TixDebug=0
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: )
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: 
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: cd ..\..\tix-8.4.3.4\win
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073: nmake -f python.mak MACHINE=%TclMachine% DEBUG=%TixDebug% TCL_DIR=F:\Python\dev\5\py35\PCbuild\..\..\tcl-8.6.1.0 TK_DIR=F:\Python\dev\5\py35\PCbuild\..\..\tk-8.6.1.0 all && nmake -f python.mak MACHINE=%TclMachine% DEBUG=%TixDebug% TCL_DIR=F:\Python\dev\5\py35\PCbuild\..\..\tcl-8.6.1.0 TK_DIR=F:\Python\dev\5\py35\PCbuild\..\..\tk-8.6.1.0 INSTALL_DIR=F:\Python\dev\5\py35\PCbuild\..\..\tcltk install
13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073:     " exited with code 1.

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.
msg215086 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-03-28 22:30
Terry J. Reedy added the comment:
> I just pulled and recompiled and got makefile error messages that I do not remember seeing:
> 13>EXEC : error : ..\..\tix-8.4.3.4 doesn't exist.

This is the output of this line:
<snip 7 lines>

> 13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073:     echo error: ..\..\tix-8.4.3.4 doesn't exist.

The rest of the messages are due to the following line:

> 13>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.MakeFile.Targets(38,5): error MSB3073:     exit 1

MSbuild just feels the need to output the entire command when it gets
a failure at any point in it.

The "error: ..\..\tix-8.4.3.4 doesn't exist" implies that you haven't
run Tools/buildbot/external-common.bat since you updated past
8cf384852680.  Tix has been included in the Windows installer for
quite some time, but had never been pulled in by the external*.bat
scripts or mentioned in the build process at all until this issue.  As
far as I know, Tix is not tested at all currently and nothing in the
source tree depends on it.

> 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.

test_idle will most likely fail if you run the tkinter tests first
(i.e., PCbuild\python_d.exe -m test -uall test_tcl test_tk
test_ttk_guionly test_ttk_textonly test_idle).  Issue #20035 should
fix that (but still needs review).
msg215715 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-04-07 19:07
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.
F:\Python\dev\5\py35>pcbuild\python_d -m test -j2 -ugui test_tcl test_idle

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)
msg222964 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-07-13 18:45
Anything else to do here, noting the references to #21059 and #20035 ?  Great work by the way, things on Windows are far cleaner than they were just a few months ago.
msg223268 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-07-16 20:33
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.  #20035 has my preferred fix for that situation.

Mark: Glad this helped, and thanks for the ping.
History
Date User Action Args
2014-07-16 20:33:10zach.waresetstatus: open -> closed
resolution: fixed
messages: + msg223268
2014-07-13 18:55:43brian.curtinsetnosy: - brian.curtin
2014-07-13 18:45:54BreamoreBoysetnosy: + BreamoreBoy
messages: + msg222964
2014-04-07 19:07:21terry.reedysetnosy: + serhiy.storchaka
messages: + msg215715
2014-03-28 22:30:56zach.waresetmessages: + msg215086
2014-03-28 22:07:43terry.reedysetnosy: + terry.reedy
messages: + msg215081
2014-03-25 08:21:31hayposetstatus: closed -> open

nosy: + haypo
messages: + msg214785

resolution: fixed -> (no value)
2014-03-22 05:36:35python-devsetmessages: + msg214445
2014-03-22 04:08:49zach.waresetstatus: open -> closed
messages: + msg214437

assignee: zach.ware
resolution: fixed
stage: patch review -> resolved
2014-03-22 03:58:25python-devsetnosy: + python-dev
messages: + msg214436
2014-03-19 21:29:57zach.waresetmessages: + msg214140
2014-03-13 14:52:05zach.waresetmessages: + msg213403
2014-03-12 23:17:15jeremy.klothsetnosy: + jeremy.kloth
messages: + msg213325
2014-03-12 20:05:39zach.waresetmessages: + msg213284
versions: - Python 3.4
2014-03-12 19:51:58tim.goldensetmessages: + msg213283
2014-03-12 19:50:09loewissetmessages: + msg213281
2014-03-12 18:44:51zach.waresetfiles: + issue15968.v3.diff

messages: + msg213275
components: + Windows
versions: + Python 3.5
2013-12-05 22:36:46zach.waresetfiles: + issue15968_tix.svndiff
type: enhancement
stage: patch review
2013-12-05 22:35:50zach.waresetfiles: + issue15968.v2.diff

messages: + msg205326
2013-07-11 21:12:03zach.waresetmessages: + msg192907
2013-06-14 21:06:57zach.waresetfiles: + issue15968_update.diff

messages: + msg191153
2013-06-14 20:05:12zach.waresetnosy: + zach.ware
2012-10-22 13:19:39jklothsetnosy: + loewis, tim.golden, brian.curtin
2012-10-22 13:18:24jklothsetfiles: + 029d1cdf6422.diff
keywords: + patch
2012-09-18 19:36:23jklothcreate