This is an archive of the discontinued LLVM Phabricator instance.

[libcxxabi] Build both static and shared versions of libc++abi by default.
ClosedPublic

Authored by EricWF on Mar 2 2015, 12:14 PM.

Details

Summary

This patch builds both static and shared versions of libc++abi by default. It adds/repurposes the following cmake options:

  • LIBCXXABI_ENABLE_SHARED: Enable/disable building the shared library. (Previously using OFF would build the static library instead)
  • LIBCXXABI_ENABLE_STATIC: Enable/disable building the static library.

This patch also re-purposes the CMake target cxxabi to be a meta-target for cxxabi_shared and cxxabi_static. This could potentially break other builds that depend on cxxabi being a library target. We will need to apply a patch to libc++'s CMake before committing this change.

Running the tests is still only supported when the shared version is built. Support for running the tests against the static library will come in another patch.

Diff Detail

Event Timeline

EricWF updated this revision to Diff 21026.Mar 2 2015, 12:14 PM
EricWF retitled this revision from to [libcxxabi] Build both static and shared versions of libc++abi by default..
EricWF updated this object.
EricWF edited the test plan for this revision. (Show Details)
EricWF added a subscriber: Unknown Object (MLST).
danalbert accepted this revision.Mar 2 2015, 12:43 PM
danalbert edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Mar 2 2015, 12:43 PM
mclow.lists edited edge metadata.Mar 2 2015, 12:53 PM

I'm OK with this.

EricWF updated this revision to Diff 21042.Mar 2 2015, 2:29 PM
EricWF edited edge metadata.

Add comments and clean up.

compnerd added inline comments.Mar 2 2015, 2:47 PM
src/CMakeLists.txt
75

You've added the option to build static. Great, let me enable that, on Darwin. You just built it dynamically here. I think that you should either add an FATAL_ERROR message if static is used on Darwin (which I think is harsh) or support both builds on Darwin as well.

83

This is a weird name. It is rather misleading as well. I get why you are doing the object library (no need to rebuild the objects). Perhaps cxxabi_OBJECTS ? Given that you are changing the name of the target from cxxabi to cxxabi_{shared,static}, cxxabi would be even better. Specially when you use the generator expression below ($<TARGET_OBJECTS:cxxabi>).

90–121

Better written as:

set(LIBCXXABI_TARGETS)
EricWF added a comment.Mar 2 2015, 2:51 PM

Address inline comments.

src/CMakeLists.txt
83

I'll change to cxxabi_OBJECTS. I want to save cxxabi as a target name that builds both cxxabi_shared and cxxabi_static.

90–121

Cool, didn't know you could do that. Thanks.

EricWF added a comment.Mar 2 2015, 2:53 PM

Address inline comments.

EricWF updated this revision to Diff 21046.Mar 2 2015, 2:53 PM

Make changes suggested by @compnerd

EricWF updated this revision to Diff 21047.Mar 2 2015, 2:55 PM

Fixed cxxabi_sources to be cxxabi_objects.

compnerd accepted this revision.Mar 2 2015, 3:06 PM
compnerd edited edge metadata.
EricWF closed this revision.Mar 3 2015, 8:01 AM