search

Sarcasm / irony-mode

A C/C++ minor mode for Emacs powered by libclang
GNU General Public License v3.0
901 stars 98 forks source link

Preserve RPATH on macOS. #474

Closed hotpxl closed 6 years ago

hotpxl commented 6 years ago

This solves #462 I'm using MacPorts, and the installer script of irony server would find the correct shared library, but removing rpath would cause it to become unusable.

Sarcasm commented 6 years ago

Sorry, I don't think the build file is an appropriate place for this, it is more like a packaging/system issue.

However, I would be interested in finding the proper place for this. Right now, the best place is for the user to edit the cmake command line when doing M-x irony-install-server RET, and to add -DCMAKE_INSTALL_RPATH_USE_LINK_PATH=ON to the command line, if it knows it make sense on his system.

hotpxl commented 6 years ago

That makes sense as well. So are you suggesting having Emacs detect OS and put together compile flags?

Sarcasm commented 6 years ago

That would also probably incorrect, it's not an OS-specific thing. There is a plethora of methods to install Clang on macOS, and some of them needs this.

Sorry, for not being a very practical person.

Right now, I guess it could be a irony-mode configuration variable, e.g.: irony-extra-cmake-args.

If I wanted irony-mode to work "out of the box" for the major platforms, I would download a prebuilt LLVM release, put it in a directory only irony-mode uses, and set the appropriate flags if it needs it. But this comes with its own set of issues, so the user variable may be a better starting point. And it could be useful to specify other options anyway.

hotpxl commented 6 years ago

I agree. I think ycm packages a libclang, cquery probably does it too. It's always painful. I would agree to the extra config variable route. This way I can put it in my init.el and forget about it.

hotpxl commented 6 years ago

I created PR #475 How do you like that? I explicitly did not quote because if the user wants multiple arguments he can just do that

Sarcasm commented 6 years ago

@hotpxl

How do you like that?

LGTM at a quick glance, will take a look on the PR itself!

@MaskRay I don't mind people using cquery/ccls, I personally tried, even made changes, but I don't feel like it is very appropriate to promote it in this specific issue. You don't seem to address a specific concern with the issue at hand. If the issue was a feature request for an indexer, you could say "Hey, sorry for the self-promotion, but this is implemented by cquery/ccls". It's nice @hotpxl proposes a PR, listen to feedbacks and help improve irony-mode. I would feel bad if he said "Hey, ccls solve my issue, bye".

Closing this PR as #475 takes over.

MaskRay commented 6 years ago

Deleted. hotpxl was beside me when I made that comment (as a joke). Sorry, deleted in case it makes others puzzled.

Sarcasm commented 6 years ago

No problem, actually I was surprised because I think I saw you and hotpxl might know each other (https://github.com/blxlrsmb).

Now that the post is deleted, people will be puzzled by my comment. :)