[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Roundup Robot added the comment: New changeset 951318b651be by Zachary Ware in branch 'default': Issue #20035: Reimplement tkinter._fix module as a C function. https://hg.python.org/cpython/rev/951318b651be -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Zachary Ware added the comment: Committed, thanks for the reviews, Serhiy and Steve! -- assignee: - zach.ware resolution: - fixed stage: patch review - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Steve Dower added the comment: Use !='true' rather than =='', but otherwise it's good. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Zachary Ware added the comment: I committed a presence test for Tix in #21337 that should catch problems with loading Tix after this patch. There shouldn't be any problem though, we actually install Tix into Tcl/Tk whereas TIX_LIBRARY is (to my understanding) for helping Tcl/Tk find a Tix that it can't find itself. Steve, I think I have BuildForRelease patched in properly; it works with BuildForRelease unset at least :) -- Added file: http://bugs.python.org/file39416/issue20035.v7.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Steve Dower added the comment: There's an MSBuild property that I set for release builds (either BuildForRelease or ReleaseBuild, not Configuration) that should exclude the preprocessor directive so we don't include an unnecessary full path. I can make that change later if you don't want to hold this patch up any longer though, as the rest looks fine to me. I don't know enough about Tix to comment on Serhiy's point. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Zachary Ware added the comment: Here's a new patch that actually applies, and has a couple of improvements. I'd greatly appreciate some review on this; I'm planning on committing before beta1 anyway. -- nosy: +paul.moore, steve.dower, tim.golden priority: normal - high Added file: http://bugs.python.org/file39401/issue20035.v6.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Serhiy Storchaka added the comment: I afraid about Tix. _fix.py searches not only Tcl/Tk, but Tix too. New patch doesn't set TIX_LIBRARY. Unfortunately there are no Tix tests at all, and we can break the support of Tix without the notice. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Mark Lawrence added the comment: ping. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Changes by Stefan Behnel sco...@users.sourceforge.net: -- nosy: -scoder ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Mark Lawrence added the comment: Can we have (hopefully) a final review and get this committed as that would also allow us to close #10652. -- nosy: +BreamoreBoy ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Zachary Ware added the comment: Ok, here's another attempt which should address both points you raised, Serhiy. I don't have an East-Asian Windows install to test on (and wouldn't be able to read anything to do the test, anyway!), but I did test with a prefix containing East-Asian characters on US-English Windows. Command Prompt had issues and MSVC choked completely, so I could not run with the debugger for that test, but Python and Tkinter seemed to work fine. -- Added file: http://bugs.python.org/file35563/issue20035.v5.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Zachary Ware added the comment: Thank you, Serhiy; those are exactly the kinds of things I don't know enough about and had concerns about. I'll take another stab and see if I can come up with anything better. Suggestions welcome :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Zachary Ware added the comment: Ping. I still want to get this in, but not without a proper review. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Serhiy Storchaka added the comment: I want to get this too, but perhaps there are some issues in a code. 1. Py_GetPrefix() returns wchar_t* string with maximal length MAXPATHLEN (defined in Include/osdefs.h as 256 on Windows). Then wcstombs() converts it to char* string. Are you sure that MAX_PATH (defined as 260 on Windows) is enough for converted string? I afraid that for multi-byte encoding it can be 2*MAXPATHLEN or even 3*MAXPATHLEN bytes. 2. After converting _tcl_library contains a path in locale encoding. And _putenv() works with it. I'm not sure, but I afraid that Tcl_SetVar() can expect UTF-8 encoded string. Please test with prefix containing non-ASCII characters (or even better with prefix containing East-Asian characters on East-Asian Windows). -- nosy: +haypo, scoder ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Zachary Ware added the comment: Ok, debugging again with better breakpoints, I see what I missed before, so here's a new patch that does things a little differently. This patch sets the TCL_LIBRARY envvar just before calling Tcl_FindExecutable, and unsets it after the call. The $tcl_library Tcl var is set after interpreter creation, as in the previous patch. Both places do nothing if TCL_LIBRARY envvar is already set or the Tcl library isn't in one of the two expected locations. I attempted to get things to work by setting Tcl's encoding search path before the call to Tcl_FindExecutable, but doing so seems to require some part of the initialization done by Tcl_FindExecutable. I would much prefer a solution that doesn't mess around with the TCL_LIBRARY envvar at all, but I've had no luck with it. -- Added file: http://bugs.python.org/file34830/issue20035.v4.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Serhiy Storchaka added the comment: TCL_VERSION should be set before call of Tcl_FindExecutable() (for correct Tcl encodings initialization). Tcl_FindExecutable() is called in PyInit__tkinter(). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Zachary Ware added the comment: Serhiy Storchaka wrote: TCL_VERSION should be set before call of Tcl_FindExecutable() (for correct Tcl encodings initialization). Tcl_FindExecutable() is called in PyInit__tkinter(). I assume you mean TCL_LIBRARY (since TCL_VERSION is #defined in Tcl.h)? You are correct though, I missed that part of the comment at the top of tkinter._fix. However, I don't know what issues arise from not having TCL_LIBRARY set before Tcl_FindExecutable() (I haven't seen any problems, but I live in an ASCII world). Also, I have tried stepping through the Tcl_FindExecutable() call in PyInit__tkinter() with the VS debugger, and didn't see anything that looked like it was trying to read TCL_LIBRARY or hit the filesystem at all, which makes me suspect that it may not actually need TCL_LIBRARY prior to Tcl_FindExecutable(). Can you give me steps to reproduce problems with not having TCL_LIBRARY set that early? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Zachary Ware added the comment: Here's an even less ugly new version of the patch; it does everything with multi-byte strings instead of wide-char strings (so that there's just one conversion of prefix from wcs to mbs at the beginning of the block, and TCL_VERSION is used directly). This patch also cleans up the Tkinter tests to remove the previous workarounds and un-reverts the change to PCbuild/rt.bat that I reverted after #15968 in an attempt to avoid test failures (that apparently didn't work). -- components: +Windows -Tests title: Suppress 'os.environ was modified' warning on Tcl/Tk tests - Clean up Tcl library discovery in Tkinter on Windows versions: +Python 3.5 -Python 3.4 Added file: http://bugs.python.org/file34662/issue20035.v3.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20035] Clean up Tcl library discovery in Tkinter on Windows
Zachary Ware added the comment: Also, I have confirmed that the blind symlink issue in non-English Windows Vista (issue3881) is not a problem in Tcl/Tk 8.6. That issue was easy to reproduce in a standard installation of Python 3.3 (with Tcl/Tk 8.5) on German Windows Vista by setting TCL_LIBRARY manually; the same test with a standard installation of Python 3.4 (with Tcl/Tk 8.6) showed no problem. Thus, I don't believe the convert_path() acrobatics from the top of Lib/tkinter/_fix.py are necessary in the C replacement. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20035 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
