This is an archive of the discontinued LLVM Phabricator instance.

[libc++abi] Refactor CMake build to avoid object libraries
AbandonedPublic

Authored by ldionne on Apr 2 2019, 4:54 PM.

Details

Summary

Object libraries don't respect the default choice of PIC for shared
libraries. The change also makes the CMake code more localized and simpler.

Event Timeline

ldionne created this revision.Apr 2 2019, 4:54 PM
Herald added a project: Restricted Project. · View Herald Transcript
ldionne marked an inline comment as done.Apr 2 2019, 5:01 PM
ldionne added inline comments.
libcxxabi/src/CMakeLists.txt
205

I want to mention that assuming that LIBUNWIND_XXX is defined here is pretty bad, as it means none of this works unless libc++abi and libunwind are actually subprojects of the same CMake project. This is a pre-existing condition, but it's not great.

smeenai accepted this revision.Apr 2 2019, 5:18 PM

LGTM

libcxxabi/src/CMakeLists.txt
206

Will you want to do something similar with libunwind (where it's not build as an object library anymore), at which point this logic will change too?

This revision is now accepted and ready to land.Apr 2 2019, 5:18 PM
ldionne marked an inline comment as done.Apr 2 2019, 5:23 PM
ldionne added inline comments.
libcxxabi/src/CMakeLists.txt
206

If we don't build object libraries for libunwind, then this would change to $<TARGET_OBJECTS:unwind_static> and $<TARGET_OBJECTS:unwind> respectively, IIUC.

smeenai requested changes to this revision.Apr 2 2019, 5:39 PM
smeenai added inline comments.
libcxxabi/src/CMakeLists.txt
206

$<TARGET_OBJECTS:...> only works with object libraries (https://cmake.org/cmake/help/v3.4/manual/cmake-generator-expressions.7.html), so it'd need a bit more tweaking, but yeah, that's kinda independent of this patch :)

I did realize a problem with this though. In our minimum supported CMake version (3.4.3), you can only use TARGET_SOURCES inside the sources list of add_library and add_executable. Later versions lift this restriction, but we need to adhere to it.

This revision now requires changes to proceed.Apr 2 2019, 5:39 PM
smeenai added inline comments.Apr 2 2019, 5:41 PM
libcxxabi/src/CMakeLists.txt
206

I meant TARGET_OBJECTS, not TARGET_SOURCES.

smeenai added inline comments.Apr 2 2019, 5:44 PM
libcxxabi/src/CMakeLists.txt
206

Although I guess it's unclear if passing the generator expression to target_sources will end up having the same effect as listing it in the sources list directly in the add_library call (and would therefore be okay on older CMake versions). Can you test this with a CMake version below 3.9 (which lifted the restriction)?