This is an archive of the discontinued LLVM Phabricator instance.

[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.

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.Apr 27 2020, 11:07 AM

LGTM

This revision is now accepted and ready to land.Apr 27 2020, 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.Apr 28 2020, 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.