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

gh-75552: Remove deprecated tkinter.tix module #104902

Merged
merged 2 commits into from
May 27, 2023

Conversation

zware
Copy link
Member

@zware zware commented May 24, 2023

@zware zware requested a review from a team as a code owner May 24, 2023 22:51
@zware zware force-pushed the so_long_and_thanks_for_all_the_Tix branch 3 times, most recently from 72026c5 to e7abc51 Compare May 24, 2023 23:36
@zooba
Copy link
Member

zooba commented May 24, 2023

If you haven't rebuilt the MSI yourself, might be worth touching something in the tools/msi directory so that the PR tests run. I don't think there are any Tix-specific dependencies in it though, so it ought to be fine.

@zware
Copy link
Member Author

zware commented May 24, 2023

Ah, hmm. Because of the way Tcl/Tk/Tix is packaged in cpython-bin-deps, should we create a tcltk-8.6.13.1 without tix?

@zware zware force-pushed the so_long_and_thanks_for_all_the_Tix branch from e7abc51 to 0524a21 Compare May 25, 2023 02:18
@zware
Copy link
Member Author

zware commented May 25, 2023

I suppose at worst the tix libraries just become dead weight in the tcltk installation, and the next time we build a new Tcl/Tk bundle for a new version, the dead weight will be removed.

Interestingly, the TestsMSI workflows "passed", but each one with 4 warnings and 2 errors. This is not new to this PR, though so I won't hold this up based on that. I would like a positive review from someone before I click the button, though :)

@zooba
Copy link
Member

zooba commented May 25, 2023

Yeah, there'll be updates needed to https://github.com/python/release-tools/blob/master/windows-release/tcltk-build.yml, which is where the "real" build happens. I can do that.

We probably need to tag builds separately for "without Tix" unless we've got a major version update coming (i.e. one that won't be used for 3.12)?

@zooba
Copy link
Member

zooba commented May 25, 2023

Interestingly, the TestsMSI workflows "passed", but each one with 4 warnings and 2 errors.

The warnings are all expected... the errors are because we don't build the docs first in that job. I've just filed #104929 to fix that, so if you want to wait and merge it before merging this, that ought to give you clean results?

@hugovk
Copy link
Member

hugovk commented May 25, 2023

Nice branch name! :)

Can some of this stuff be removed?

Modules/tkappinit.c
63:#ifdef WITH_TIX
65:        extern int Tix_Init(Tcl_Interp *interp);
66:        extern int Tix_SafeInit(Tcl_Interp *interp);
67:        Tcl_StaticPackage(NULL, "Tix", Tix_Init, Tix_SafeInit);

Lib/idlelib/help.html
30:    <link rel="prev" title="tkinter.tix — Extension widgets for Tk" href="tkinter.tix.html" />
126:    <p class="topless"><a href="tkinter.tix.html"
127:                          title="previous chapter"><code class="xref py py-mod docutils literal notranslate"><span class="pre">tkinter.tix</span></code> — Extension widgets for Tk</a></p>
163:          <a href="tkinter.tix.html" title="tkinter.tix — Extension widgets for Tk"
1080:    <p class="topless"><a href="tkinter.tix.html"
1081:                          title="previous chapter"><code class="xref py py-mod docutils literal notranslate"><span class="pre">tkinter.tix</span></code> — Extension widgets for Tk</a></p>
1120:          <a href="tkinter.tix.html" title="tkinter.tix — Extension widgets for Tk"

Modules/Setup
225:# specific extension (e.g. Tix or BLT), leave the corresponding line
242:# *** Uncomment and edit for Tix extension only:
243:# -DWITH_TIX -ltix8.1.8.2 \

@zooba
Copy link
Member

zooba commented May 25, 2023

See also python/release-tools#47 where I'm making the Tix build step optional for the cpython-bin-deps build.

The package from my test run is at https://dev.azure.com/Python/cpython/_build/results?buildId=128771&view=artifacts&pathAsName=false&type=publishedArtifacts (for about 30 days I expect) if you want to check that it works. You shouldn't need to log in, but all you can do is click the three dots at the right of the tcltk item to download it.

@zware
Copy link
Member Author

zware commented May 25, 2023

Can some of this stuff be removed?

I opted to keep the WITH_TIX bits in tkappinit.c and Modules/Setup because they're not actually related to including Tix with Python, they're available for someone building something into tkinter specially. There are other WITH_* defs in the surrounding code for Tcl modules that we've never included.

The mentions in idlelib/help.html should go away next time that file is regenerated. I tried it out, but introduced several more changes due to Sphinx version mismatch, so I left it for now. Those bits shouldn't actually be relevant to its use anyway.

@jneb
Copy link
Contributor

jneb commented May 25, 2023

There still is no alternative for the tix module. The Select and Balloon classes are nowhere to be found, no Python replacement, and ttk doesn't even look like tix. What are the poor users to do?

@zware
Copy link
Member Author

zware commented May 25, 2023

Tix has been unmaintained for 10 years now, has open security vulnerabilities, and every update of anything in our toolchain risks breaking it. If you still need it, it's open source and can be vendored into your project or maintained separately, but we will no longer provide it out of the box as of Python 3.13.

@zware zware force-pushed the so_long_and_thanks_for_all_the_Tix branch from c8d477d to 99efec0 Compare May 26, 2023 17:06
@zware zware force-pushed the so_long_and_thanks_for_all_the_Tix branch from 99efec0 to 27c8d16 Compare May 26, 2023 17:08
@zware zware merged commit a989b73 into python:main May 27, 2023
@zware zware deleted the so_long_and_thanks_for_all_the_Tix branch May 27, 2023 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants