This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Fix the setting of LIBCXX_HEADER_DIR in standalone build
ClosedPublic

Authored by phosek on Jul 23 2018, 6:25 PM.

Details

Summary

This is an alternative approach to r337727 which broke the build
because libc++ headers were copied into the location outside of
directories used by Clang. This change sets LIBCXX_HEADER_DIR to
different values depending on whether libc++ is being built as
part of LLVM w/ per-target multiarch runtime, LLVM or standalone.

Diff Detail

Repository
rL LLVM

Event Timeline

phosek created this revision.Jul 23 2018, 6:25 PM
aheejin added a comment.EditedJul 23 2018, 6:37 PM

This fixes our local waterfall build failure. Thank you! I'm not very familiar with CMake, so I'll wait for someone to take a look and approve.

I've tested this on macOS and everything seems to be working,

LGTM. FWIW, I'm really not a huge fan of all these different ways of building libc++. There should be one canonical way of doing it, and it should be simple. However, it seems like this patch is the right thing to do given the changes that have already been made to account for specific use cases.

IOW, I don't like this whole trend, but we're already down that road.

mclow.lists added a subscriber: mclow.lists.EditedJul 24 2018, 7:56 AM

Testing this on my system (Mac OS 10.13.6, using system libc++abi), I get a lot of files in my build's include/c++/v1 folder. Previously, there were two (__cxxabi_config.h and ccxxabi.h).
Now there are 131 files - all the libc++ header files.

Why?

The cmake command I use is: CXX=$LLVM_BIN/clang++ cmake -DLLVM_PATH=$LLVM/llvm -DLIBCXX_CXX_ABI=libcxxabi -DLIBCXX_CXX_ABI_INCLUDE_PATHS=/usr/include $LIBCXX

phosek updated this revision to Diff 157043.Jul 24 2018, 8:14 AM

phosek updated this revision to Diff 157043.

This fixes the problem with copying all the include files.
This *also* fixes my broken build (that was broken by r337630 wanting to copy cxxabi.h to /include/c++/v1/).

LG for my configuration.

phosek updated this revision to Diff 157050.Jul 24 2018, 8:49 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jul 24 2018, 8:49 AM
This revision was automatically updated to reflect the committed changes.