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

Make the default of the PYTHON variable consistent #31

Merged
merged 3 commits into from
Apr 12, 2015

Conversation

tyilo
Copy link
Contributor

@tyilo tyilo commented Apr 12, 2015

in the mac and linux makefile.

@oleavr
Copy link
Member

oleavr commented Apr 12, 2015

I don't think the -f option exists in the BSD readlink; did you check if this works on Mac?

@tyilo
Copy link
Contributor Author

tyilo commented Apr 12, 2015

You're right. Is there even a reason that we need to resolve the symlink if PYTHON is a symlink?

@oleavr
Copy link
Member

oleavr commented Apr 12, 2015

Ok. It's just to have the directories not clash inside build, so you can do "make" and later "make PYTHON=/foo" to build another python binding side-by-side.

@tyilo
Copy link
Contributor Author

tyilo commented Apr 12, 2015

That explains why we need to get the path to the python binary, but not why we need to resolve the symlink.

@oleavr
Copy link
Member

oleavr commented Apr 12, 2015

Because which python might be a symlink, and we use its basename to name the directory. Ideally we want it to end up as e.g. lib/python2.7, so you can do make PYTHON=/usr/local/bin/python3.4 later to have lib/python3.4 side-by-side.

@tyilo
Copy link
Contributor Author

tyilo commented Apr 12, 2015

Say I have two versions of python 2.7.x installed, eg. OS X system one and another one from homebrew, would they be built side by side?
If yes, then we need to resolve the symlink.
If no, we just need to detect the python version, which we should do by running $(PYTHON) -c 'import sys; v = sys.version_info; print "{}.{}".format(v[0], v[1])'

@oleavr
Copy link
Member

oleavr commented Apr 12, 2015

No, they'll still clash. This is however not a very important feature. I'd say just to proper detection like you mentioned, it's way better anyway.

@oleavr
Copy link
Member

oleavr commented Apr 12, 2015

Sweet!

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.

2 participants