tcl-tk: fixes #12114 for python and #12808 for python3 by bumping tcl-tk version #15950

telamonian · 2017-07-23T20:51:59Z

to a version in which the upstream has fixed the relevant bugs.

Hopefully this is a better attempt at a fix than my last one (#15855).

Have you followed the guidelines for contributing?
Have you checked that there aren't other open pull requests for the same formula update/change?
Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
[mostly] Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

So apparently all of the trouble that people have been having with brew python install --with-tcl-tk and brew python3 install --with-tcl-tk (#12114 and #12808) is due to a whole mess of bugs in the upstream that have apparently all been fixed.

However, there's still no official release aside from the version of 8.6.6 that was put out last year. So what I've done is to update the formula so that it pulls from the core_8_6_7_rc branch in which all of the relevant fixes have been applied. After installing tcl-tk with the updated formula and then building python2/3 with --with-tcl-tk, brew test passes for both pythons, and both of

python -c 'import Tkinter; root = Tkinter.Tk(); root.mainloop()'
python3 -c 'import tkinter; root = tkinter.Tk(); root.mainloop()'

open an application window as expected, so everything seems good.

The one remaining hiccup is that brew audit --strict is still picking up a single error. This is due to the fact that tk now installs a couple of icon files (Tk.icns and Tk.tiff) in /usr/local/opt/tcl-tk/lib alongside the libraries, and audit really doesn't like that. I looked through the configure and makefile files in tk, and the installation of the icons into the library appears to be intended behavior on the part of the tcltk devs. How should I fix this?

ilovezfs · 2017-07-23T20:54:08Z

Don't worry about the audit issue.

However, this version will need to go in a devel do block until upstream puts out an actual new release.

telamonian · 2017-07-23T20:55:34Z

Can do. Can you please point me to a good example of a devel do block in an existing formula?

ilovezfs · 2017-07-23T20:57:32Z

dbus

telamonian · 2017-07-23T21:51:47Z

I put the 8.6.7 stuff in a devel do block, and I brought back the old 8.6.6 stuff in a stable do block so that devel could have it's own separate tk resource. And then I tested everything again.

ilovezfs · 2017-07-24T13:36:33Z

Formula/tcl-tk.rb

  bottle do
    sha256 "de26155e0b2fee960af4791d39e3d6c79421c635c0a914be8a0254ff28f4fad2" => :sierra
    sha256 "9481cea8f38c644eb12f6a42463082469f1e91fe7616b167fbda46ded9bef336" => :el_capitan
    sha256 "f137c8176792d8363989981c2f3838f8edee73738a02bf899caddd8460abdd86" => :yosemite
  end

+  devel do


actually let's make this head do since there's no 8.6.7-rc tag yet

ilovezfs · 2017-07-24T13:37:11Z

Formula/tcl-tk.rb

  bottle do
    sha256 "de26155e0b2fee960af4791d39e3d6c79421c635c0a914be8a0254ff28f4fad2" => :sierra
    sha256 "9481cea8f38c644eb12f6a42463082469f1e91fe7616b167fbda46ded9bef336" => :el_capitan
    sha256 "f137c8176792d8363989981c2f3838f8edee73738a02bf899caddd8460abdd86" => :yosemite
  end

+  devel do
+    url "https://github.com/tcltk/tcl/archive/a29278e79c748bfaf8766c486eb93eca7ad586be.tar.gz"


url "https://github.com/tcltk/tcl.git", :branch => "core_8_6_7_rc"

ilovezfs · 2017-07-24T13:37:25Z

Formula/tcl-tk.rb

  bottle do
    sha256 "de26155e0b2fee960af4791d39e3d6c79421c635c0a914be8a0254ff28f4fad2" => :sierra
    sha256 "9481cea8f38c644eb12f6a42463082469f1e91fe7616b167fbda46ded9bef336" => :el_capitan
    sha256 "f137c8176792d8363989981c2f3838f8edee73738a02bf899caddd8460abdd86" => :yosemite
  end

+  devel do
+    url "https://github.com/tcltk/tcl/archive/a29278e79c748bfaf8766c486eb93eca7ad586be.tar.gz"
+    version "8.6.7"


remove this line please

ilovezfs · 2017-07-24T13:37:32Z

Formula/tcl-tk.rb

+  devel do
+    url "https://github.com/tcltk/tcl/archive/a29278e79c748bfaf8766c486eb93eca7ad586be.tar.gz"
+    version "8.6.7"
+    sha256 "90202b013184fea551a5800404f572b1b4cbadaebc60dfaa6516a7d8ad837fff"


remove this line please

ilovezfs · 2017-07-24T13:37:56Z

Formula/tcl-tk.rb

+    sha256 "90202b013184fea551a5800404f572b1b4cbadaebc60dfaa6516a7d8ad837fff"
+
+    resource "tk" do
+      url "https://github.com/tcltk/tk/archive/8aecb568ca2896cfae4253ead2fe62442d977b55.tar.gz"


url "https://github.com/tcltk/tk.git", :branch => "core_8_6_7_rc"

ilovezfs · 2017-07-24T13:38:05Z

Formula/tcl-tk.rb

+
+    resource "tk" do
+      url "https://github.com/tcltk/tk/archive/8aecb568ca2896cfae4253ead2fe62442d977b55.tar.gz"
+      version "8.6.7"


remove this line please

ilovezfs · 2017-07-24T13:38:10Z

Formula/tcl-tk.rb

+    resource "tk" do
+      url "https://github.com/tcltk/tk/archive/8aecb568ca2896cfae4253ead2fe62442d977b55.tar.gz"
+      version "8.6.7"
+      sha256 "3c4f38783f2fca6e0b4e9152279fbc0576e63cbbd8024e47a006497dd8db4379"


remove this line please

ilovezfs · 2017-07-24T13:38:26Z

Formula/tcl-tk.rb

@@ -58,7 +73,7 @@ def install

        cd "unix" do
          system "./configure", *args
-          system "make", "TK_LIBRARY=#{lib}"


why this change?

Originally I thought that passing an explicit TK_LIBRARY might be what was causing the icon files to install to the lib. But what I found over the course of about a dozen test builds is that it makes no difference to any part of the build whether that argument is there or not.

Since it wasn't doing anything, and seems like a potential source of issues, I removed it.

telamonian · 2017-07-24T17:36:41Z

Changing from devel to head broke the build. Long story short, a flag along the lines of -L/usr/local/Cellar/tcl-tk/head-a29278e/lib gets passed to clang during the tk portion of the build. The flag is necessary in order for some previously built libs to be found, but some part of superenv seems to be removing/sanitizing the libpath before it actually gets passed to the linker.

I did manage to get the head build working, but I had to change a couple of things around in the formula and in one of the low level homebrew .rb files.

ilovezfs · 2017-07-24T17:48:53Z

@telamonian that may be a brew bug.

CC @MikeMcQuaid it sounds like superenv is allowing #{lib} for stable and devel, but filtering #{lib} out for head.

telamonian · 2017-07-24T17:55:37Z

Maybe? I know at least half of what went wrong with superenv

First, I had to remove the revision 1 statement from the tcl-tk formula. For whatever reason, it seems like having a _1 in the L/usr/local/Cellar/tcl-tk/head-a29278e_1/lib flag is too much for some part of superenv, though I still have no idea which.

I also had to change line 214 in $(brew --prefix)/Homebrew/Library/Homebrew/shims/super/cc from

if formula_prefix && path.start_with?("#{formula_prefix}/")

to

if formula_prefix && path.start_with?("#{formula_prefix}")

Basically, formula_prefix was set to /usr/local/Cellar/tcl-tk/head, whereas path was set to /usr/local/Cellar/tcl-tk/head-a29278e/lib, so with the extra forward slash /usr/local/Cellar/tcl-tk/head/ did not match to the start of path. Once path fails that check, it leads to a couple of other events that then cause it to be filtered out.

If there's a security concern to removing the forward slash from the end of the check, maybe formula_prefix can be changed in order to include the commit-specific stuff that gets stuck on the end of a head cellar path?

ilovezfs · 2017-07-24T17:57:53Z

@telamonian this reminds me of
Homebrew/brew#535
Homebrew/brew#536

CC @vladshablinsky

telamonian · 2017-07-24T18:20:07Z

Okay, merged all of the changes and tested them. There's no revision spec in there at the moment, so that's not a problem, but you do still definitely need to hack the cc shim.

The devel version was working pretty well, and without all of this head related craziness. What would you think about maybe rolling back to the devel version of the formula?

ilovezfs · 2017-07-24T18:25:19Z

@telamonian I'm fine with using devel do if you can convince upstream to create 8.6.7-rc1 tags for tcl and tk for us to use!

ilovezfs · 2017-07-24T18:29:02Z

@telamonian you'll need to squash this down to a single commit so that the brew pull doesn't 💥

telamonian · 2017-07-24T18:37:09Z

Trying to get the tcltk devs to release a tag when I want them to sounds about as futile as me trying to convince you to bring back the --with-x11 option for the tcl-tk formula :)

ilovezfs · 2017-07-24T18:38:53Z

@telamonian you might be surprised. It's "only" a beta/rc/alpha/pre/whatever tag you'd be asking for.

telamonian · 2017-07-24T19:16:58Z

I'm not really that familiar with squash. I was looking through the docs, and there seem to be a lot of choices. Also, I hadn't realized that I managed to pull in 95 unrelated commits when I fixed those conflicts.

What's the best practice for squashing in this situation? I assume that I'm not going to want to preserve my entire git log.

ilovezfs · 2017-07-25T04:20:33Z

@telamonian you can follow the steps here #15721 (comment)

…and Homebrew#12808

ilovezfs · 2017-07-29T05:22:39Z

@telamonian it looks like upstream has posted 8.6.7rc0 on SourceForge, so we can use that for devel, if you'd like to refresh this PR.

ilovezfs · 2017-07-31T12:28:15Z

@telamonian ping

telamonian · 2017-08-01T01:05:03Z

I'll take care of this tonight. I assume you didn't merge a week ago because of the HEAD related issues in brew? I have two questions then:

Should I strip the semi-functional HEAD build?
Assuming the devel is fine, will you merge this soon? Looking at Homebrew/homebrew-science#5505 there's apparently already been a couple of people who have tried to use the new tcl-tk formula without realizing that it hasn't been pulled in yet.

ilovezfs · 2017-08-01T01:19:49Z

I assume you didn't merge a week ago because of the HEAD related issues in brew

Right.

Should I strip the semi-functional HEAD build?

Yes.

Assuming the devel is fine, will you merge this soon?

Yes.

…y adding devel build. Also removes semi-function HEAD build

telamonian · 2017-08-01T03:32:31Z

All done/tested. Let me know if anything else is needed before the new tcl-tk can be merged.

ilovezfs · 2017-08-02T06:27:21Z

@telamonian Thanks for your first contribution to Homebrew! Without people like you submitting PRs we couldn't run this project. You rock!

telamonian changed the title ~~tcl-tk: fixes #12114 for python and #12808 for python3 by bumping tcl-tk~~ tcl-tk: fixes #12114 for python and #12808 for python3 by bumping tcl-tk version Jul 23, 2017

ilovezfs suggested changes Jul 24, 2017

View reviewed changes

- tcl-tk: added HEAD build to fix python tkinter issues Homebrew#12114 …

300d60e

…and Homebrew#12808

telamonian force-pushed the tcltk_bump_867rc_for_python_fix branch from 7b45e63 to 300d60e Compare July 25, 2017 20:49

tcl-tk: fix python tkinter issues Homebrew#12114 and Homebrew#12808 b…

55e0106

…y adding devel build. Also removes semi-function HEAD build

ilovezfs added 2 commits July 31, 2017 21:33

Update tcl-tk.rb

9b8afd3

Update tcl-tk.rb

1a442e1

ilovezfs approved these changes Aug 2, 2017

View reviewed changes

ilovezfs merged commit 4a61e09 into Homebrew:master Aug 2, 2017

ilovezfs mentioned this pull request Sep 6, 2017

build: fix HOMEBREW_FORMULA_PREFIX for head Homebrew/brew#3129

Merged

5 tasks

Homebrew locked and limited conversation to collaborators May 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tcl-tk: fixes #12114 for python and #12808 for python3 by bumping tcl-tk version #15950

tcl-tk: fixes #12114 for python and #12808 for python3 by bumping tcl-tk version #15950

telamonian commented Jul 23, 2017 •

edited

Loading

ilovezfs commented Jul 23, 2017

telamonian commented Jul 23, 2017

ilovezfs commented Jul 23, 2017

telamonian commented Jul 23, 2017 •

edited

Loading

ilovezfs Jul 24, 2017

ilovezfs Jul 24, 2017

ilovezfs Jul 24, 2017

ilovezfs Jul 24, 2017

ilovezfs Jul 24, 2017

ilovezfs Jul 24, 2017

ilovezfs Jul 24, 2017

ilovezfs Jul 24, 2017

telamonian Jul 24, 2017

telamonian commented Jul 24, 2017 •

edited

Loading

ilovezfs commented Jul 24, 2017

telamonian commented Jul 24, 2017

ilovezfs commented Jul 24, 2017

telamonian commented Jul 24, 2017

ilovezfs commented Jul 24, 2017

ilovezfs commented Jul 24, 2017 •

edited

Loading

telamonian commented Jul 24, 2017

ilovezfs commented Jul 24, 2017

telamonian commented Jul 24, 2017

ilovezfs commented Jul 25, 2017

ilovezfs commented Jul 29, 2017

ilovezfs commented Jul 31, 2017

telamonian commented Aug 1, 2017

ilovezfs commented Aug 1, 2017

telamonian commented Aug 1, 2017

ilovezfs commented Aug 2, 2017

tcl-tk: fixes #12114 for python and #12808 for python3 by bumping tcl-tk version #15950

tcl-tk: fixes #12114 for python and #12808 for python3 by bumping tcl-tk version #15950

Conversation

telamonian commented Jul 23, 2017 • edited Loading

ilovezfs commented Jul 23, 2017

telamonian commented Jul 23, 2017

ilovezfs commented Jul 23, 2017

telamonian commented Jul 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

telamonian commented Jul 24, 2017 • edited Loading

ilovezfs commented Jul 24, 2017

telamonian commented Jul 24, 2017

ilovezfs commented Jul 24, 2017

telamonian commented Jul 24, 2017

ilovezfs commented Jul 24, 2017

ilovezfs commented Jul 24, 2017 • edited Loading

telamonian commented Jul 24, 2017

ilovezfs commented Jul 24, 2017

telamonian commented Jul 24, 2017

ilovezfs commented Jul 25, 2017

ilovezfs commented Jul 29, 2017

ilovezfs commented Jul 31, 2017

telamonian commented Aug 1, 2017

ilovezfs commented Aug 1, 2017

telamonian commented Aug 1, 2017

ilovezfs commented Aug 2, 2017

telamonian commented Jul 23, 2017 •

edited

Loading

telamonian commented Jul 23, 2017 •

edited

Loading

telamonian commented Jul 24, 2017 •

edited

Loading

ilovezfs commented Jul 24, 2017 •

edited

Loading