llvm: general cleanup and corrections

[Bo98]

• Have you followed the guidelines for contributing?
• Have you checked that there aren't other open pull requests for the same formula update/change?
• Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
• Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
• Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

---

Some of this could likely be applied to llvm@9 and earlier. The formula was a collection of a years of additions - some of which no longer applies.

I will want to verify the bottle's contents before merging this. Done.

[SMillerDev]

Should this also be applied to older versions?

[SMillerDev]

3 isn't an option?

[Bo98]

I think it is - Linuxbrew uses it. I limited making any actual functional changes (besides fixing polly linking) but I can go ahead and migrate to 3.8 unless there is a particular reason anyone knows not to.

[iMichka]

If we can use python 3.8, I would prefer that too. Just had no time to migrate it until now.

[Bo98]

Apart from Python, is there anything else here you would prefer done here that would be useful for Linux?

[SMillerDev]

@iMichka I linked the monster patch for linux in a slack PM to Bo

[Bo98]

I think I'll need to better understand the original problem and why it is Linux specific.

Looking at the patch alone, it doesn't seem obvious at first if there's a better way. You can do things like supplying sysroots but I think that is not what is wanted.

[Bo98]

Should this also be applied to older versions?

Perhaps in part - I need to be careful.

[Bo98]

Actually this doesn't need depends_on "python@3.8" at runtime really. I could just install it to the correct place so that those who have python@3.8 installed can use it and make Python a build time dependency. Thoughts?

[SMillerDev]

Sounds good

[pthariensflame]

You're sure this preserves all of the previously-included elisp files?

[Bo98]

Yes, I downloaded the bottle from Jenkins to verify:

$ ls ./llvm/10.0.0/share/emacs/site-lisp/llvm
clang-format.el         clang-rename.el  llvm-mode.el
clang-include-fixer.el  emacs.el         tablegen-mode.el

[josephg]

I'm still running into polly linking issues using the formula in this PR. I'm trying to build zig from source:

$  brew install https://raw.githubusercontent.com/Bo98/homebrew-core/llvm-cleanup/Formula/llvm.rb
(4 hours later)
zig/build$ cmake .. -DCMAKE_PREFIX_PATH=/usr/local/opt/llvm  -DCMAKE_CXX_COMPILER=/usr/local/opt/llvm/bin/clang++

Configuring zig version 0.5.0+f6d384450
...
-- Build files have been written to: /Users/josephg/3rdparty/zig/build
$ make
...
Undefined symbols for architecture x86_64:
  "getPollyPluginInfo()", referenced from:
      (anonymous namespace)::EmitAssemblyHelper::EmitAssemblyWithNewPassManager(clang::BackendAction, std::__1::unique_ptr<llvm::raw_pwrite_stream, std::__1::default_delete<llvm::raw_pwrite_stream> >) in libclangCodeGen.a(BackendUtil.cpp.o)
ld: symbol(s) not found for architecture x86_64
clang-10: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [zig0] Error 1
make[1]: *** [CMakeFiles/zig0.dir/all] Error 2
make: *** [all] Error 2

Mangled symbols for getPollyPluginInfo show up in libclang-cpp.dylib and LLVMPolly.so:

$ nm -gU  /usr/local/opt/llvm/lib/libclang-cpp.dylib | grep getPolly                                    
0000000001d294f6 T __Z18getPollyPluginInfov

[Bo98]

I thought the issue you describe was specific to standalone clang builds. The third commit of this pull request changes the type of build LLVM performs to avoid standalone builds entirely. If it doesn’t work on these two quite different setups, I don’t really know what can be done without upstream fixing it on their side.

[Bo98]

If someone can link to me a specific LLVM bug or patch about the issue that is not specific to Clang standalone then let me know. It seems to me that it's not a misconfiguration on Homebrew's side given apt.llvm.org has the same issue and there is a current open bug at Debian about it.

[jedisct1]

At least, this PR totally solved LLVM 10 installation for me.

[Bo98]

I see a lot of discussion pointing here from various issues so I'll summarise what's been done:

• This is now built using the "monorepo" layout. This is generally the layout that is the most tested upstream, and as such may well fix things on its own from changing to this. This also allows --HEAD builds to "work" again (beside from upstream bugs that may prevent compile).
• Fixed a compile issue if Lua was installed.
• Prevented opportunistic linkage to LZMA.
• Greatly simplified the formula to remove installs already handled by CMake and to reduce duplicated files and unneeded wrappers. swig is no longer a dependency. No functionality has been lost as far as I know.
• The binaries are now linked to the shared libraries rather than static. This decreases the size of the bottle and the install by over 75% (4.2GB disk -> 1GB; 1.25GB bottle -> ~300MB). No functionality has been lost, but let me know if there are any significant performance regressions (I doubt it since most other package managers ship it this way).
• libLLVM-C is now included.
• Python bindings are now supplied for Python 3.8 rather than 2.7.
• The tests have been improved to properly test both Xcode and CLT SDKs.
• Some old, broken Polly-related CMake variables were corrected. Probably no actual benefit here though since I suspect they were enabled by default anyway.

As for zig, I do not believe this is Homebrew specific at all. I've seen the same issue on at least 4 package managers so it seems to clear to me that it is an upstream LLVM change. I recommend you speak with them - but feel free to let me know if you hear anything.

I do suspect however that you can avoid the issue altogether by stopping linking against Clang's static libraries and start linking against the shared library clang-cpp. I know Fedora for one have made an effort to move everything they manage to use clang-cpp. I've not tried it in the context of zig however - this would require modifying the zig CMake scripts.

As some general guidance: LLVM does provide some CMake targets for you in the CMake files they ship at lib/cmake. This is the recommended approach to linking LLVM & Clang in CMake. llvm-config doesn't cover clang-cpp as far as I know and has known bugs on macOS so I strongly discourage using it within CMake as much as possible. Based on some of the logs I've seen over the past couple days, those bugs are clearly still present.

[moreindirection]

Thanks for fixing - llvm installs cleanly for me now.

[timotheecour]

@Bo98 that commit 78baeec only does a small subset of the changes in this PR, what about the other changes?

according to ziglang/zig#4897 (comment)

@robinsoncol Homebrew’s LLVM is misconfigured: #52352

[Bo98]

@timotheecour See discussion on ziglang/zig#4799.

I'm aware of some issues on zig 0.6.0 but they don't appear to be the same issues as the one you linked.

[timotheecour]

@Bo98 i followed that thread but the "fix" suggested -DZIG_PREFER_CLANG_CPP_DYLIB=ON doesn't really work: it makes zig build but the zig is useless, see ziglang/zig#5055 (comment) ; where i show that:

• with gcc + manually built llvm+friends + a patch to zig, zig works with or without DZIG_STATIC=ON
• with clang + homebrew built llvm+friends (or manually built), -DZIG_PREFER_CLANG_CPP_DYLIB=ON is needed to avoid the famous getPollyPluginInfo link error, but then zig is useless (with or without with or without -DZIG_STATIC=ON)