This repository has been archived by the owner. It is now read-only.

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

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

samueljohn commented Dec 17, 2012

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.

Contributor

derekbrokeit commented Dec 18, 2012

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.

Contributor

derekbrokeit commented Dec 18, 2012

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.

Contributor

derekbrokeit commented Dec 18, 2012

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

Contributor

samueljohn commented Dec 18, 2012

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.

Contributor

derekbrokeit commented Dec 18, 2012

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.

Contributor

samueljohn commented Dec 18, 2012

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 Dec 18, 2012

@derekbrokeit derekbrokeit referenced this pull request in derekbrokeit/homebrew-pymol Dec 18, 2012

Closed

External GUI doesn't show #1

Contributor

derekbrokeit commented Dec 18, 2012

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?

Contributor

samueljohn commented Dec 18, 2012

Now, that is a good reason.

Contributor

samueljohn commented Dec 18, 2012

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.

Contributor

samueljohn commented Dec 18, 2012

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

Contributor

samueljohn commented Dec 18, 2012

Python seems to ignore the env var TCLLIBPATH, too.

Contributor

samueljohn commented Dec 18, 2012

I give up.

Contributor

derekbrokeit commented Dec 18, 2012

On my computer, if you set TCL_LIBRARY in the environment to
/usr/local/lib/tcl8.5
it works, but this in inelegant to require an environment variable. That is
the point of my pull request on homebrew/dupes.

I have asked Comp.lang.python if they have any idea how to do this more
straightforwardly. tcl's "wish" program works, so it Must be a problem with
how python attempts to find the TK_LIBRARY as a relative directory to
TCL_LIBRARY. We could bypass this by explicitly setting the variable in TCL
and TK make files, but that seems wrong to me (though that is what my pull
request does). I'll let you know if I or anyone there comes up with a
solution. Thanks for looking into it!

2012�$BG/�(B12�$B7n�(B18�$BF|2PMKF|�(B Samuel John notifications@github.com:

I give up.

�$B!=�(B
Reply to this email directly or view it on GitHubhttps://github.com/mxcl/homebrew/pull/16626#issuecomment-11481997.

Contributor

derekbrokeit commented Dec 18, 2012

For reference, I posted a question here. I hope that's the right place for
that kind of question.

https://groups.google.com/forum/m/?fromgroups#!topic/comp.lang.python/Yd-eHzGCoDU

2012�$BG/�(B12�$B7n�(B18�$BF|2PMKF|�(B Derek Ashley Thomas derekathomas@gmail.com:

On my computer, if you set TCL_LIBRARY in the environment to /usr/local/lib/tcl8.5
it works, but this in inelegant to require an environment variable. That is
the point of my pull request on homebrew/dupes.

I have asked Comp.lang.python if they have any idea how to do this more
straightforwardly. tcl's "wish" program works, so it Must be a problem with
how python attempts to find the TK_LIBRARY as a relative directory to
TCL_LIBRARY. We could bypass this by explicitly setting the variable in TCL
and TK make files, but that seems wrong to me (though that is what my pull
request does). I'll let you know if I or anyone there comes up with a
solution. Thanks for looking into it!

2012�$BG/�(B12�$B7n�(B18�$BF|2PMKF|�(B Samuel John notifications@github.com <javascript:_e({},
'cvml', 'notifications@github.com');>:

I give up.

�$B!=�(B
Reply to this email directly or view it on GitHubhttps://github.com/mxcl/homebrew/pull/16626#issuecomment-11481997.

Contributor

samueljohn commented Dec 18, 2012

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

Contributor

samueljohn commented Dec 18, 2012

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

Contributor

derekbrokeit commented Dec 18, 2012

Did you try my homebrew/dupes pull request? It is linked above. That worked
for me. How are you testing this? I was performing simple TK window tests
and then I tried a complex program like pymol which worked on my computer.
Complete Brew install instructions (including your pull request and
mine) are here:

http://github.com/scicalculator/homebrew-pymol

2012�$BG/�(B12�$B7n�(B18�$BF|2PMKF|�(B Samuel John notifications@github.com:

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

�$B!=�(B
Reply to this email directly or view it on GitHubhttps://github.com/mxcl/homebrew/pull/16626#issuecomment-11483252.

Contributor

samueljohn commented Dec 18, 2012

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.

Owner

MikeMcQuaid commented Dec 18, 2012

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

Contributor

samueljohn commented Dec 18, 2012

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'

?

Owner

MikeMcQuaid commented Dec 18, 2012

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

Contributor

samueljohn commented Dec 18, 2012

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

Contributor

samueljohn commented Dec 18, 2012

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

Contributor

derekbrokeit commented Dec 20, 2012

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' :)

Contributor

samueljohn commented Dec 20, 2012

@scicalculator I am on it.

Contributor

samueljohn commented Dec 20, 2012

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

@samueljohn samueljohn reopened this Dec 20, 2012

Contributor

samueljohn commented Dec 20, 2012

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.

Owner

MikeMcQuaid commented Dec 22, 2012

Rebase?

Contributor

samueljohn commented Dec 22, 2012

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

Contributor

samueljohn commented Dec 30, 2012

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

Owner

MikeMcQuaid commented Dec 30, 2012

Looks good to me. Patch submitted upstream?

Contributor

samueljohn commented Dec 30, 2012

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

Contributor

samueljohn commented Jan 3, 2013

Reported:

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

samueljohn commented Jan 11, 2013

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

Owner

MikeMcQuaid commented Jan 11, 2013

Done. Congratulations.

Contributor

samueljohn commented Jan 11, 2013

Congratulations.

... you charmer ;-)

Owner

MikeMcQuaid commented Jan 11, 2013

This broke Python 3.

MikeMcQuaid added a commit that referenced this pull request Jan 11, 2013

Revert "python: Allow --with-brewed-tk."
This reverts commit 1d8f01d.

Reopens #16626, reopens #16574.
Owner

MikeMcQuaid commented Jan 11, 2013

Reverted.

Contributor

samueljohn commented Jan 11, 2013

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

Contributor

samueljohn commented Jan 11, 2013

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 added a commit to dholm/homebrew that referenced this pull request Jan 14, 2013

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>

dholm added a commit to dholm/homebrew that referenced this pull request Jan 14, 2013

Revert "python: Allow --with-brewed-tk."
This reverts commit 1d8f01d.

Reopens #16626, reopens #16574.

norioxkimura added a commit to norioxkimura/homebrew that referenced this pull request Jan 16, 2013

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>

norioxkimura added a commit to norioxkimura/homebrew that referenced this pull request Jan 16, 2013

Revert "python: Allow --with-brewed-tk."
This reverts commit 1d8f01d.

Reopens #16626, reopens #16574.

@xu-cheng xu-cheng locked and limited conversation to collaborators Feb 16, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.