Arch Linux

Please read this before reporting a bug:
https://wiki.archlinux.org/index.php/Reporting_Bug_Guidelines

Do NOT report bugs when a package is just outdated, or it is in the AUR. Use the 'flag out of date' link on the package page, or the Mailing List.

REPEAT: Do NOT report bugs for outdated packages!
Tasklist

FS#60512 - [clang] Use the same PKGBUILD for extra/clang extra/llvm extra/lldb

Attached to Project: Arch Linux
Opened by Ray Song (MaskRay) - Sunday, 21 October 2018, 01:11 GMT
Last edited by Evangelos Foutras (foutrelis) - Sunday, 21 October 2018, 07:15 GMT
Task Type Bug Report
Category Packages: Extra
Status Closed
Assigned To No-one
Architecture All
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Description:
Currently

extra/clang is built with -DBUILD_SHARED_LIBS=ON (generate /usr/lib/libclangAST.so ... instead of libclangAST.a)
-DLLVM_LINK_LLVM_DYLIB=ON (link against libLLVM-7.so)

extra/llvm extra/lldb are built with -DBUILD_SHARED_LIBS=OFF (default) (generate /usr/lib/libLLVMSupport.a , not libLLVMSupport.so)

The discrepancy makes it easy to have collision of static constructors when the user links libLLVMSupport.a and libclangAST.so

# If the user builds https://github.com/MaskRay/ccls with cmake -H. -BRelease -DSYSTEM_CLANG=on -DLLVM_ENABLE_RTTI=on # no -DUSE_SHARED_LLVM=on
% ccls
CommandLine Error: Option 'help-list' registered more than once!

This is because both libLLVM-7.so (needed by libclang*.so) and libLLVMSupport.a define the static constructors of some cl::opt options (e.g. https://github.com/llvm-mirror/llvm/tree/master/lib/Support/CommandLine.cpp#L2039 HLOp "help-list")


I suggest unifying PKGBUILD of

* extra/llvm
* extra/clang (depends on llvm)
* extra/lldb (depends on both llvm and clang)

and using -DLLVM_BUILD_LLVM_DYLIB=ON -DUSE_SHARED_LIBS=ON for the unified build. (there will be libclangAST.so libLLVMSupport.so but no .a)

The separate build is also wasteful.

Note, -DLLVM_BUILD_LLVM_DYLIB=ON creates libLLVM-7.so, which is a unified .so containing all external symbols of libLLVM*.a or libLLVM*.so
This task depends upon

Closed by  Evangelos Foutras (foutrelis)
Sunday, 21 October 2018, 07:15 GMT
Reason for closing:  Not a bug
Additional comments about closing:  Current configuration works OK. The llvm_config cmake macro could be improved but that's an upstream issue.
Comment by Eli Schwartz (eschwartz) - Sunday, 21 October 2018, 01:33 GMT
How is the separate build wasteful? I'd expect that if anything, we should just add those options to our separate packages which we separated for a reason. How we split the PKGBUILD should not matter in the slightest, since quite obviously we *do* link to shared libllvm.

But it looks to me like your problem is that in your downstream build configuration you've chosen to use a shared clang and a static llvm. Not sure how we're supposed to fix that for you.

You incidentally suggest in the middle of all this, that we disable building the static llvm libraries, but we could do that just as easily using split PKGBUILDs. And while not providing static llvm libs would "solve" your issue, it would only do so by papering it over, while simultaneously inconveniencing everyone who has valid reasons for wanting to develop using static libraries (something which is obviously desirable as indicated by the existence of those libs in the first place).
Comment by Ray Song (MaskRay) - Sunday, 21 October 2018, 03:53 GMT
It would be helpful if I could edit the description :)

> How is the separate build wasteful?

* extra/llvm
* extra/clang (depends on llvm)
* extra/lldb (depends on both llvm and clang)

For the build server to build the three packages, clang is built twice (with one build discarded) and llvm is built triply (with two builds discarded)

Some configure options should be kept in sync for ABI compatibility (e.g. if llvm has -DLLVM_ENABLE_RTTI=on but clang has -DLLVM_ENABLE_RTTI=off => ABI mismatch and runtime SIGSEGV)

> Not sure how we're supposed to fix that for you.

This bothered some users as they didn't read the documentation carefully. The current status makes it easy to collide:

-lLLVMSupport -lclangAST => libLLVMSupport.a + libclangAST.so (which needs libLLVM-7.so) are linked => static constructor collision

If libLLVM*.a are replaced by libLLVM*.so (-DBUILD_SHARED_LIBS=on), it will not be possible to collide.

> You incidentally suggest in the middle of all this, that we disable building the static llvm libraries
>
> while simultaneously inconveniencing everyone who has valid reasons for wanting to develop using static libraries (something which is obviously desirable as indicated by the existence of those libs in the first place).

I don't buy this.

libLLVM*.so (-DBUILD_SHARED_LIBS=on) + libLLVM-7.so (-DLLVM_BUILD_LLVM_DYLIB=ON) won't interfere with any -lLLVMSupport -lLLVMSymbolize or -lLLVM usage and it would also decrease package sizes.
Comment by Evangelos Foutras (foutrelis) - Sunday, 21 October 2018, 07:10 GMT
No builds are "discarded"; each package is built only once. In retrospect, it was a good decision to switch to separate PKGBUILDs for each project. [1]

BUILD_SHARED_LIBS=ON was used for clang so its size didn't grow to like 600MB when clang-tools-extra was merged into it. It's not desirable for llvm because it already provides a shared library and tools happily link to it.

Perhaps you could call out to /usr/bin/llvm-config to get the correct library name(s) based on LLVM_LINK_LLVM_DYLIB's value. (Unfortunately, the llvm_config cmake macro doesn't automatically choose libLLVM when LLVM_LINK_LLVM_DYLIB=ON. [2])

[1] https://lists.archlinux.org/pipermail/arch-dev-public/2018-March/029179.html
[2] https://bugs.llvm.org/show_bug.cgi?id=35245#c4

Loading...