Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

Fix python lib finding on OS X 10.10.4 #42717

Closed
wants to merge 1 commit into from

Conversation

nguillermin
Copy link

While trying to install gnuradio I had some issues with python crashing ("PyThreadState_Get: no current thread Abort trap: 6"-type fatal errors) which I narrowed down to swig using the system library when everything else was linked against my homebrew python. This patch fixes that.

@dunn dunn added the python label Aug 10, 2015
@DomT4
Copy link
Member

DomT4 commented Aug 10, 2015

Everything should be linked against the system Python, per Homebrew's policy. The only things that should be linked against Homebrew's Python are things which are depends_on :python or depends_on :python => :optional.

@nguillermin
Copy link
Author

The gnuradio formula I found (metacollin/homebrew-gnuradio) depends_on :python. Does this mean this commit won't be accepted? Is it a better idea to introduce a swig patch via the gnuradio formula?

@DomT4
Copy link
Member

DomT4 commented Aug 10, 2015

@tdsmith is the Python genius, I'll let him comment, but it seems unlikely that we'll be happy to accept this. The gnuradio formula may have less troubles if it switches to depends_on :python if MacOS.version <= :snow_leopard but that would need investigating.

@tdsmith
Copy link
Contributor

tdsmith commented Aug 10, 2015

I'm surprised this has an effect and I'd like to understand why. Specifically what does this influence? What happens without this patch, and what does this patch change? Which files are affected, and why?

@nguillermin
Copy link
Author

Hi @tdsmith, and sorry, I went back and checked and that's not the only thing that I had to change. I also had to export PYTHON=/usr/local/bin/python in order to get it to compile against homebrew python.

In any case, prior to patch, and after exporting python environment variable, the ./configure script will give you

checking for Python os.name... posix
checking for Python version... python2.7
checking for Python lib dir... lib
checking for Python header files... -I/usr/local/Cellar/python/2.7.10_2/Frameworks/Python.framework/Versions/2.7/include/python2.7 -I/usr/local/Cellar/python/2.7.10_2/Frameworks/Python.framework/Versions/2.7/lib/python2.7/config
checking for Python library directory...
/usr/local/Cellar/python/2.7.10_2/Frameworks/Python.framework/Versions/2.7/lib/python2.7/config
checking for Python library... -lpython2.7

With patch you get

checking for Python os.name... posix
checking for Python version... python2.7
checking for Python lib dir... /usr/local/Cellar/python/2.7.10_2/Frameworks/Python.framework/Versions/2.7/lib
checking for Python header files... -I/usr/local/Cellar/python/2.7.10_2/Frameworks/Python.framework/Versions/2.7/include/python2.7 -I/usr/local/Cellar/python/2.7.10_2/Frameworks/Python.framework/Versions/2.7//usr/local/Cellar/python/2.7.10_2/Frameworks/Python.framework/Versions/2.7/lib/python2.7/config
checking for Python library directory... Not found
checking for Python library... -lpython2.7

It took me until right now to notice that this might actually just trade off autoconf's entries for 'lib dir' and 'library directory'...

Apologies for the confusion, this is the first PR I've ever made.

@@ -12,6 +12,8 @@ class Swig < Formula

option :universal

patch :DATA
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please submit this patch to the upstream developers of this project and add a link to the upstream patch submission and explanation of why the patch is needed in a comment in the formula file. Thanks!

@DomT4
Copy link
Member

DomT4 commented Jan 23, 2016

Closing as dead. Please do feel free to reopen if you wish to with the comments addressed. Thank you for your submission to Homebrew ❤️

@DomT4 DomT4 closed this Jan 23, 2016
@Homebrew Homebrew locked and limited conversation to collaborators Jul 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants