Page MenuHomePhabricator

[libclang] Install both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC=ON
ClosedPublic

Authored by zhuhan0 on Apr 20 2020, 9:31 PM.

Details

Summary

When LIBCLANG_BUILD_STATIC=ON and LLVM_ENABLE_PIC=ON, PIC version of libclang.a and libclang.so are built as expected. However libclang.a is not installed. Looking at the macro llvm_add_library(), when both SHARED and STATIC are set, it renames the static library to ${name}_static and then adds it to targets. But when add_clang_library() calls install, it only checks if ${name} is in targets.

To work around this issue, loop through both ${name} and ${name}_static and install both of them if they're in targets. This is still correct if only shared or static library is built. In those cases, only ${name} is added to targets and cmake install will generate the right install script depending on the library's type.

Test plan:
cmake with LIBCLANG_BUILD_STATIC=ON and then ninja install, from master and this diff. Compare the result directory trees. Confirm that only difference is the added libclang.a.

Diff Detail

Event Timeline

zhuhan0 created this revision.Apr 20 2020, 9:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 20 2020, 9:31 PM
zhuhan0 edited the summary of this revision. (Show Details)Apr 20 2020, 9:41 PM
zhuhan0 edited the summary of this revision. (Show Details)Apr 20 2020, 11:22 PM
smeenai added subscribers: phosek, beanz.

This makes sense to me, but I'd like @beanz and/or @phosek to take a look as well.

Hello @beanz @phosek @smeenai any comment on this patch? :)

phosek accepted this revision.Mon, Apr 27, 11:07 AM

LGTM

This revision is now accepted and ready to land.Mon, Apr 27, 11:07 AM

@zhuhan0, do you need someone to commit this for you?

Committed. Please forward any buildbot failure emails to me.

This revision was automatically updated to reflect the committed changes.
hans added a subscriber: hans.Tue, Apr 28, 1:17 AM

This broke builds configured with -DLLVM_ENABLE_PIC=OFF, e.g.

$ cmake -GNinja -DCMAKE_BUILD_TYPE=Release '-DLLVM_ENABLE_PROJECTS=clang' '-DLLVM_TARGETS_TO_BUILD=X86' -DLLVM_ENABLE_PIC=OFF ../llvm

                                                                           
CMake Error at                                                             
/b/s/w/ir/cache/builder/src/third_party/llvm/clang/tools/libclang/CMakeLists.txt:123
(target_compile_definitions):                                              
    target_compile_definitions called with non-compilable target type

I've reverted in f03b505ee7f23018493b93e3828fc3672c6f903c

Thanks @hans My bad. Didn't test this when LLVM_ENABLE_PIC=OFF.

Submitted D79059 to better solve this problem.