FindLibClang: add version 8.0.0_1

[goxberry]

This commit adds the version string "8.0.0_1" to the list of LLVM versions because the current version of LLVM in Homebrew (as of the time of this commit) is version 8.0.0_1.

Another approach for detecting a macOS Homebrew install of the "llvm" package (latest stable version) would be to add /usr/local/opt/llvm to the list of search paths. Homebrew installs of older major versions of LLVM are symlinked to paths of the form /usr/local/opt/llvm@${MAJOR_VERSION} (e.g., /usr/local/opt/llvm@7/, /usr/local/opt/llvm@6/).

[goxberry]

Fixes #540.

[Sarcasm]

Hum, one thing I don't understand with Homebrew. I feel like CMakeLists.txt should not care much about Homebrew, but Homebrew should adapt to packages standard conventions.

Isn't there some common install prefix that Homebrew install all its softwares? Does Homebrew install the ClangConfig.cmake file? If so, where is it located?

Maybe supporting Homebrew is just a matter of specifying a -DCMAKE_PREFIX_PATH=<some/Homebrew/prefix>. When you do which clang, what path do you get?

[eagleflo]

ClangConfig.cmake is located in /usr/local/opt/llvm/lib/cmake/clang/ when LLVM is installed via Homebrew.

[Sarcasm]

Reading the Homebrew documentation, I am under the impression that each package is installed in a versioned directory under a "Cellar", but it should be symlink to /usr/local (or another customized prefix). That's what I get from: https://brew.sh/#question

That looks like it should requires nothing from CMake, apart maybe from -DCMAKE_PREFIX_PATH=/usr/local if this path is not part of the CMAKE_SYSTEM_PREFIX_PATH.

If the LLVM/Clang package is different from that, it would be nice to know why, and what is expected to do for consumers like irony-mode?

[Sarcasm]

Does the build work when you use?

cmake -DCMAKE_PREFIX_PATH=/usr/local/opt/llvm <irony-args...>

[eagleflo]

Yes, adding that prefix to the M-x irony-install-server line fixes the build issue for me (with LLVM installed from Homebrew).

You are right that Homebrew usually symlinks its packages under /usr/local, but LLVM is a special "keg-only" formula where this isn't done by default. The reasoning is that Xcode already installs Apple's fork of LLVM and Clang and having both Apple's fork and mainline LLVM / Clang in the PATH at the same time might cause trouble.

(Apple's own libclang.dylib can be found under /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/. It doesn't ship with a ClangConfig.cmake file.)

[Sarcasm]

Well, I would prefer a PR that adds /usr/local/opt/llvm to the search paths. There is already quite a few hardcoded macOs paths.

But to be honest, I don't think it fits in this place anymore, and it would be better to specify -DCMAKE_PREFIX_PATH=/usr/local/opt/llvm from the command line. That way, the ClangConfig.cmake shipped by LLVM might be found, thanks to a recent change:

https://github.com/Sarcasm/irony-mode/blob/c3ae899b61124a747ebafc705086345e460ac08e/server/src/CMakeLists.txt#L4

This can be done by settings irony-extra-cmake-args appropriately.

I'm not sure I will update the LIBCLANG_KNOWN_LLVM_VERSIONS variable anymore, since modern LLVM versions should provide usable CMake configs.

[goxberry]

@Sarcasm I think the suggestion to set irony-extra-cmake-args is useful and more elegant than the current PR. I also suspect that the most common LLVM configuration for Homebrew would use the latest stable version of LLVM provided by Homebrew; hardcoding the search path suggested by @eagleflo would cover this case and would likely decrease support burden. As implied by your most recent post, I'd add the search paths for Clang's CMake config file to the PATHS argument of find_package(Clang) currently at irony-mode/server/src/CMakeLists.txt:4.

I'm happy to make that change, along with similar modifications for other common configurations (Debian/Ubuntu, MacPorts, FreeBSD, Gentoo), if someone else is willing to provide the appropriate paths.

[goxberry]

It's also worth noting that ClangConfig.cmake is present in LLVM 3.9.0 and above. Correctly setting search paths in the find_package(Clang) call at irony-mode/server/src/CMakeLists.txt:4 would obviate the need for adding maintenance versions of recent LLVM (e.g., 7.1.0) to LIBCLANG_KNOWN_LLVM_VERSIONS.

[goxberry]

Changes made in 5396dd3.

[Sarcasm]

Hum, I'm not fond of the patch, I think this is better set externally, in the user config or eventually in some .el in irony mode (but that can come at a latter time).

Right now we could add the following, with instructions in the Wiki or README:

(setq irony-extra-cmake-args
      (list (format "-DCMAKE_PREFIX_PATH=%s" (or (getenv "HOMEBREW_PREFIX")
                                                 "/usr/local/opt/llvm"))))

I don't want to handle all the package managers out there, the benefit of find_package(Clang) over the FindLibclang.cmake, was to simplify the CMake files.

[goxberry]

@Sarcasm I like your suggestion. Perhaps the code snippet should be slightly modified?

(setq irony-extra-cmake-args
      (list (format "-DCMAKE_PREFIX_PATH=%s" (concat (or (getenv "HOMEBREW_PREFIX")
                                                 "/usr/local")) "/opt/llvm"))

HOMEBREW_PREFIX will only give the Homebrew prefix install directory, which is /usr/local by default on macOS and ${HOME}/.linuxbrew by default on Linux. There could, of course, be additional logic to fall back to the default install directory that depends on the OS, but I'm not sure that additional complexity would be helpful. Assuming the snippet above is syntactically correct -- I don't write much code in Emacs Lisp -- it should cover most use cases.

[Sarcasm]

Ah yes you are right. Since you use Homebrew, you can try it out to see if that is ok.