-
Notifications
You must be signed in to change notification settings - Fork 764
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 a issue that libclang.dylib won't be loaded when it is linked with libLLVM dynamically #66
Conversation
Sadly, this PR breaks YCM compilation for me. Here's what I get:
I have a standard setup (nothing custom) so if this breaks for me, it will break for others. Feel free to update this PR with changes and ping me then. I recommend trying out your PR on a vanilla Mac setup (not the custom one you have) to ensure everything still works. |
Yeah, I knew the possibility of this failure, but never thought it came so quickly. I will resubmit with a new patch which uses |
Updated. The new patch should have the same effect as the previous one, but more error-tolerant. |
Nope, still doesn't work:
|
In a short survey, it worked perfectly with cmake 3.1.0 and 3.0.2 (forgive me if I am always using the latest softwares for testing). I updated a patch in a different approach. Maybe you want to try with it. |
This now works. There's a comment right above the section you have edited:
If I'm not mistaken, what you are doing here is going against that comment. Doesn't your change mean that a system libclang could be loaded before a custom libclang.dylib placed right next to the ycm_core.dylib? If so, we can't merge this. We don't want the system libclang accidentally overwriting the custom libclang. |
No, it won't load the system's version. Since the path of To confirm it, we can do a small test.
import ycm_core
print 'Clang version: ' + ycm_core.ClangVersion()
DYLD_PRINT_LIBRARIES=1 python test.py In my case (with llvm 3.5.1-rc2 built from source, shared libraries enabled), I have:
while without this patch, it is:
What we know here:
|
Thanks for the explanation! Could you add a note about |
…h libLLVM dynamically
I have added some comments to the |
Thanks for the PR! |
Fix a issue that libclang.dylib won't be loaded when it is linked with libLLVM dynamically
Thank you too. |
resent PR #65, comments are welcome.