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

Fix install script so it finds the correct Python. #128

Merged
merged 5 commits into from
Feb 18, 2013
Merged

Fix install script so it finds the correct Python. #128

merged 5 commits into from
Feb 18, 2013

Conversation

xgalaxy
Copy link
Contributor

@xgalaxy xgalaxy commented Feb 17, 2013

This fixes #18 and maybe some other related issues. In particular with homebrew python installations.

Similar fixes are done in various homebrew formula. For example, see: https://github.com/mxcl/homebrew/blob/51d054c/Library/Formula/opencv.rb#L50-73

There are a couple of other problems, but I don't think YouCompleteMe can do anything about them, as they are configuration issues with other projects.

For instance, homebrew vim works with the above fix, but homebrew macvim does not (at least I couldn't get it to without editing the formula), because the macvim seems bent on using system python.

Fix tested with:
OSX 10.7.5
Homebrew
Python (2.7.3) installed with homebrew
Vim 7.3 (patches 1-820) installed with homebrew

@Valloric
Copy link
Member

I'm going to have to test this out before merging but don't have the time right now. Possibly later today.

Thanks for the pull request! The wrong-Python-version nonsense is the most reported issue for YCM.

@xgalaxy
Copy link
Contributor Author

xgalaxy commented Feb 17, 2013

Yea. I highly suggest we don't merge this until we've tested several configurations.
I am only able to test the OSX/Homebrew/Vim path with brewed python.
I am not sure if the non-framework installation will work (not sure if I got the paths / version string correct).

@xgalaxy
Copy link
Contributor Author

xgalaxy commented Feb 17, 2013

Okay.
I think I got the non-framework paths correct.

@Valloric
Copy link
Member

Works great on my machine (Mac OS X 10.8.2, only system Python, latest upstream official MacVim binary). I'll note that I'm slightly worried by the fact that previously cmake would say -- Found PythonLibs: /usr/lib/libpython2.7.dylib and now it says -- Found PythonLibs: /System/Library/Frameworks/Python.framework/Versions/2.7/Python but since everything still works, I'll get over my paranoia and merge this in.

God knows I've had enough of the Python-related install problems (and issues on the tracker) on Mac. Hopefully this will put those to rest.

@Valloric Valloric merged commit 018e670 into ycm-core:master Feb 18, 2013
@kaishin
Copy link

kaishin commented Feb 19, 2013

Not sure this solved the problem as I still had to brew unlink python to get this sorted out...
Using standalone MacVim 7.3 (snapshot 66) on OS X 10.8.2

@xgalaxy
Copy link
Contributor Author

xgalaxy commented Feb 20, 2013

The crux of the issue is there is simply no good way of determining what Python version someone's copy of Vim or MacVim was linked against. Typically a makefile configuration will link against whatever the active Python version is found on the system. There was an inconsistency with how CMake does this, which is what this fix resolved.

Moving forward, I will try and get another pull request in which provides an additional optional parameter to the YCM install script which will allow the user to force a specific version.

Beyond that its up to the user to make sure they are properly setting up their environment.
And by the way, brew unlink python will give you subtle bugs / crashes if you later down the road forget that you did that and then do something like pip install a module that you plan on using in MacVim.

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.

Can't run MacVim after installing YCM
3 participants