Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

python: Allow --with-brewed-tk and fix PYTHONFRAMEWORKDIR #16626

Closed
wants to merge 1 commit into from

3 participants

@samueljohn

Fixes #16574

I reordered the formula such that the options come first and then the depends_on. I think it is more logical now, since some depends_on are conditional on the given options.

@scicalculator

This builds great! Thanks for setting it up. The only issue is that finally building python with this library brought out a nasty bug in our tk-tcl installation. Essentially, this will break tkinter support without this fix to tcl.

@scicalculator

I also think we should add to the caveats that with-brewed-tk is an option only available if you tap homebrew-dupes. It seems strange that this option relies on homebrew-dupes, while python (a technical "dupe") is in the main repository, but this addition (installing with homebrew-tk) seems like obvious functionality in my opinion.

@scicalculator

Actually looking into it, this seems more of a problem that might be sorted out on the python end since it seems to check the wrong library directories. I'll ask upstream to python

@samueljohn

Oh fuck, I didn't see that Tk comes from homebrew/dupes. Not sure what to do about it. Perhaps retract this PR or allow make it explicitly depends_on 'homebrew/dupes/tk'?

Python is just too important to be a dupe :-)
Seriously, there are good reasons.

@scicalculator

I certainly understand why python is in the main repository, but if depends_on 'homebrew/dupes/tk' is not an option, there is a possible alternative. We could instead have an option to disable tkinter when you install python. Then, add a py-tkinter brew to dupes. That is what macports does by default. The disadvantage to this is that it is more complicated and requires installation of yet another brew pytkinter.

An explicit dependency would be better in my opinion, but it depends on what others think. Perhaps there needs to be some discussion about how to deal with homebrew/... taps which are formally maintained, but not the "main" repo.

@samueljohn

add a py-tkinter brew to dupes. That is what macports does by default.

No, we don't do that. It gets complicated and we don't allow multiple formulae for the same software.

There are only three options:

  • Either we go with depends_on 'homebrew/dupes/tk' if build.include? 'with-brewed-tk' or
  • we depends_on 'tk' if build.include? 'with-brewed-tk' or
  • we continue to live with what Apple provides (even if that Tk is somewhat behind)

Are there any showstoppers for you @scicalculator with The Tk from Apple?

@scicalculator the two of us can discuss here further and try to find a solution. But until Homebrew/homebrew-dupes#132 is fixed, I will close this so it doesn't get pulled by accident. We can re-open later.

@samueljohn samueljohn closed this
@scicalculator scicalculator referenced this pull request in scicalculator/homebrew-pymol
Closed

External GUI doesn't show #1

@scicalculator

The problem with always relying on Apple's tk is that they have built it without threading capabilities. Homebrew's tk is capable of being built with --enable-threads which is required for some software that expects the GUI to run in a thread parallel to the main thread. Because Apple's tk is not thread safe, applications either do not display or crash depending on how they are made. This is the reason for #16574.

I understand the need for simplicity in the homebrew installation, so I am not sure what the best way to go forward is. Have there been any formulas in the past that referenced formulas in alternative taps? I somehow doubt that is the first to consider it?

@samueljohn

Now, that is a good reason.

@samueljohn

Let's first try to fix the Tk bug you found and then ask the Homebrewers if that is acceptable to depend on a formula from the Homebrew organization (not an external one) if it is guarded by an option. I am pretty sure this is not the first time that questions comes up.

@samueljohn

I am not able to fix that Tk bug. Open for your suggestions.

@samueljohn

Python seems to ignore the env var TCLLIBPATH, too.

@samueljohn

I give up.

@scicalculator
@scicalculator
@samueljohn

And when you use tcltksh and can you "require tk" or how you would use tk when not using wish?

@samueljohn

I think patching tk or tcl would be a way to go. But I failed to do so. How did you do that?

@scicalculator
@samueljohn

Yes your PR works great.
Once that is pulled, I reopen this to ask the core team about the dependency on a formula in homebrew/dupes.

@mikemcquaid
Owner

Personally I don't mind depending on a dupe formula as long as it's an option and it all Just Works.

@samueljohn

Cool. @mikemcquaid do you prefer:

  • depends_on 'homebrew/dupes/tk' if build.include? 'with-brewed-tk' or
  • depends_on 'tk' if build.include? 'with-brewed-tk'

?

@mikemcquaid
Owner

The prior, definitely. The latter won't work unless it's already tapped, right?

@samueljohn

Alright! At least the latter cannot tell which tap to tap.

@samueljohn

This PR remains dormant until Homebrew/homebrew-dupes#132.

@scicalculator

Just a reminder, Homebrew/homebrew-dupes#132 was merged, so if other's agree, hopefully this can be unsuspended with the change to depends_on 'homebrew/dupes/tk' if build.include? 'with-brewed-tk' :)

@samueljohn

I am testing this now against the newer version from Homebrew/homebrew-dupes#124.

@samueljohn samueljohn reopened this
@samueljohn

Tested with Homebrew/homebrew-dupes#124. Working.
It also worked with the current Tcl/Tk. Both with X11 or with --enable-aqua.
The Tk.Framework check is no longer needed, too.

@mikemcquaid
Owner

Rebase?

@samueljohn

yes, I have to. @mistydemeo fixed an important bug in the meantime, also adding a patch...

@samueljohn

@mikemcquaid rebased.

Added a fix for to set PYTHONFRAMEWORKDIR correctly so python-config --ldflags is usable and others (for exampe gst-python can correctly compile. That change should fix #16739.

@mikemcquaid
Owner

Looks good to me. Patch submitted upstream?

@samueljohn

No, not yet. Neither the patch for tk, nor the PYTHONFRAMEWORKDIR fix. I will do this.

@samueljohn

Reported:

@samueljohn samueljohn python: Allow --with-brewed-tk.
- Fixes #16574.
- Improve Tkinter test by actually calling Tk()
- Check for /Library/Frameworks/Tk.framework no longer needed.
- Fix PYTHONFRAMEWORKDIR so that `python-config --ldflags` is useful.
7941a97
@samueljohn

@mikemcquaid would you be so kind to pull in. I take the full responsibility if something goes wrong :-)

I want to start working on a Python class and a PythonRequirement in homebrew and move the sitecustomize.py out of the formula. But working on multiple branches that alter the same file sucks.

@mikemcquaid mikemcquaid closed this pull request from a commit
@samueljohn samueljohn python: Allow --with-brewed-tk.
- Fixes #16574.
- Improve Tkinter test by actually calling Tk()
- Check for /Library/Frameworks/Tk.framework no longer needed.
- Fix PYTHONFRAMEWORKDIR so that `python-config --ldflags` is useful.

Closes #16626.

Signed-off-by: Mike McQuaid <mike@mikemcquaid.com>
1d8f01d
@mikemcquaid
Owner

Done. Congratulations.

@samueljohn

Congratulations.

... you charmer ;-)

@mikemcquaid
Owner

This broke Python 3.

@mikemcquaid mikemcquaid referenced this pull request from a commit
@mikemcquaid mikemcquaid Revert "python: Allow --with-brewed-tk."
This reverts commit 1d8f01d.

Reopens #16626, reopens #16574.
cc2b8b3
@mikemcquaid
Owner

Reverted.

@samueljohn

I see. The fix is easy. Python3 reused the no-longer-needed framework....

@samueljohn

I can't push into this branch any longer, even if I name it the same (because I deleted earlier). This seems so since @github allowed to delete and undo-delete branches.

@mikemcquaid please pull #17008 instead. I tested that it works as this one but does not break python3.

@dholm dholm referenced this pull request from a commit in dholm/homebrew
@samueljohn samueljohn python: Allow --with-brewed-tk.
- Fixes #16574.
- Improve Tkinter test by actually calling Tk()
- Check for /Library/Frameworks/Tk.framework no longer needed.
- Fix PYTHONFRAMEWORKDIR so that `python-config --ldflags` is useful.

Closes #16626.

Signed-off-by: Mike McQuaid <mike@mikemcquaid.com>
3fc5b59
@dholm dholm referenced this pull request from a commit in dholm/homebrew
@mikemcquaid mikemcquaid Revert "python: Allow --with-brewed-tk."
This reverts commit 1d8f01d.

Reopens #16626, reopens #16574.
288689a
@norioxkimura norioxkimura referenced this pull request from a commit in norioxkimura/homebrew
@samueljohn samueljohn python: Allow --with-brewed-tk.
- Fixes #16574.
- Improve Tkinter test by actually calling Tk()
- Check for /Library/Frameworks/Tk.framework no longer needed.
- Fix PYTHONFRAMEWORKDIR so that `python-config --ldflags` is useful.

Closes #16626.

Signed-off-by: Mike McQuaid <mike@mikemcquaid.com>
f2c4d04
@norioxkimura norioxkimura referenced this pull request from a commit in norioxkimura/homebrew
@mikemcquaid mikemcquaid Revert "python: Allow --with-brewed-tk."
This reverts commit 1d8f01d.

Reopens #16626, reopens #16574.
6947e80
@guyzmo guyzmo referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@guyzmo guyzmo referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@cooljeanius cooljeanius referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@rajeeja rajeeja referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@rajeeja rajeeja referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 11, 2013
  1. @samueljohn

    python: Allow --with-brewed-tk.

    samueljohn authored
    - Fixes #16574.
    - Improve Tkinter test by actually calling Tk()
    - Check for /Library/Frameworks/Tk.framework no longer needed.
    - Fix PYTHONFRAMEWORKDIR so that `python-config --ldflags` is useful.
This page is out of date. Refresh to see the latest.
Showing with 37 additions and 28 deletions.
  1. +37 −28 Library/Formula/python.rb
View
65 Library/Formula/python.rb
@@ -1,20 +1,5 @@
require 'formula'
-class TkCheck < Requirement
- def message; <<-EOS.undent
- Tk.framework was detected in /Library/Frameworks
- This can cause Python builds to fail. See:
- https://github.com/mxcl/homebrew/issues/11602
- EOS
- end
-
- def fatal?; false; end
-
- def satisfied?
- not File.exist? '/Library/Frameworks/Tk.framework'
- end
-end
-
class Distribute < Formula
url 'http://pypi.python.org/packages/source/d/distribute/distribute-0.6.34.tar.gz'
sha1 'b6f9cfbaf3e63833b71009812a613be13e68f5de'
@@ -30,28 +15,29 @@ class Python < Formula
url 'http://www.python.org/ftp/python/2.7.3/Python-2.7.3.tar.bz2'
sha1 '842c4e2aff3f016feea3c6e992c7fa96e49c9aa0'
- depends_on TkCheck.new
- depends_on 'pkg-config' => :build
- depends_on 'readline' => :recommended
- depends_on 'sqlite' => :recommended
- depends_on 'gdbm' => :recommended
- depends_on 'openssl' if build.include? 'with-brewed-openssl'
-
option :universal
option 'quicktest', 'Run `make quicktest` after the build'
option 'with-brewed-openssl', "Use Homebrew's openSSL instead of the one from OS X"
+ option 'with-brewed-tk', "Use Homebrew's Tk (has optional Cocoa and threads support)"
option 'with-poll', 'Enable select.poll, which is not fully implemented on OS X (http://bugs.python.org/issue5154)'
-
# --with-dtrace relies on CLT as dtrace hard-codes paths to /usr
option 'with-dtrace', 'Experimental DTrace support (http://bugs.python.org/issue13405)' if MacOS::CLT.installed?
+ depends_on 'pkg-config' => :build
+ depends_on 'readline' => :recommended
+ depends_on 'sqlite' => :recommended
+ depends_on 'gdbm' => :recommended
+ depends_on 'openssl' if build.include? 'with-brewed-openssl'
+ depends_on 'homebrew/dupes/tk' if build.include? 'with-brewed-tk'
+
def patches
p = []
# python fails to build on NFS; patch is merged upstream, will be in next release
# see http://bugs.python.org/issue14662
p << "https://gist.github.com/raw/4349132/25662c6b382315b5db67bf949773d76471bbcee7/python-nfs-shutil.diff"
p << 'https://raw.github.com/gist/3415636/2365dea8dc5415daa0148e98c394345e1191e4aa/pythondtrace-patch.diff' if build.include? 'with-dtrace'
-
+ # Patch to disable the search for Tk.frameworked, since homebrew's Tk is a plain unix build
+ p << DATA if build.include? 'with-brewed-tk'
p
end
@@ -182,6 +168,7 @@ def install
s.gsub!(/^CXX=.*$/, "CXX=xcrun clang++")
s.gsub!(/^AR=.*$/, "AR=xcrun ar")
s.gsub!(/^RANLIB=.*$/, "RANLIB=xcrun ranlib")
+ s.gsub!(/^PYTHONFRAMEWORKDIR=\tPython\.framework/, "PYTHONFRAMEWORKDIR= #{opt_prefix}/Frameworks/Python.framework")
end
end
@@ -196,11 +183,13 @@ def distutils_fix_superenv(args)
# Help Python's build system (distribute/pip) to build things on Xcode-only systems
# The setup.py looks at "-isysroot" to get the sysroot (and not at --sysroot)
cflags += " -isysroot #{MacOS.sdk_path}"
- # For the Xlib.h, Python needs this header dir
- cflags += " -I#{MacOS.sdk_path}/System/Library/Frameworks/Tk.framework/Versions/8.5/Headers"
ldflags += " -isysroot #{MacOS.sdk_path}"
# Same zlib.h-not-found-bug as in env :std (see below)
args << "CPPFLAGS=-I#{MacOS.sdk_path}/usr/include"
+ # For the Xlib.h, Python needs this header dir with the system Tk
+ unless build.include? 'with-brewed-tk'
+ cflags += " -I#{MacOS.sdk_path}/System/Library/Frameworks/Tk.framework/Versions/8.5/Headers"
+ end
end
args << cflags
args << ldflags
@@ -245,7 +234,7 @@ def caveats
Homebrew's Python framework
#{prefix}/Frameworks/Python.framework
- Python demo
+ Python demo #{'🐍' if MacOS.version >= :lion}
#{HOMEBREW_PREFIX}/share/python/Extras
Distribute and Pip have been installed. To update them
@@ -260,6 +249,7 @@ def caveats
They will install into the site-package directory
#{site_packages}
+
Executable python scripts will be put in:
#{scripts_folder}
so you may want to put "#{scripts_folder}" in your PATH, too.
@@ -273,6 +263,25 @@ def test
# and it can occur that building sqlite silently fails if OSX's sqlite is used.
system "#{bin}/python", "-c", "import sqlite3"
# Check if some other modules import. Then the linked libs are working.
- system "#{bin}/python", "-c", "import Tkinter"
+ system "#{bin}/python", "-c", "import Tkinter; root = Tkinter.Tk()"
end
end
+
+__END__
+diff --git a/setup.py b/setup.py
+index 6b47451..b0400f8 100644
+--- a/setup.py
++++ b/setup.py
+@@ -1702,9 +1702,9 @@ class PyBuildExt(build_ext):
+ # AquaTk is a separate method. Only one Tkinter will be built on
+ # Darwin - either AquaTk, if it is found, or X11 based Tk.
+ platform = self.get_platform()
+- if (platform == 'darwin' and
+- self.detect_tkinter_darwin(inc_dirs, lib_dirs)):
+- return
++ # if (platform == 'darwin' and
++ # self.detect_tkinter_darwin(inc_dirs, lib_dirs)):
++ # return
+
+ # Assume we haven't found any of the libraries or include files
+ # The versions with dots are used on Unix, and the versions without
Something went wrong with that request. Please try again.