[PATCH] [libcxx] Add support for building and testing with an ABI library not along linker paths

Eric Fiselier eric at efcs.ca
Mon Sep 1 18:37:31 PDT 2014


>>! In D5038#9, @danalbert wrote:
>>>! @EricWF wrote:
>> Unfortunately not only does the user have to pass -DCMAKE_LIBRARY_PATH=/path/to/lib to CMake, but they also need to set RPATH=/path/to/lib when building libc++.
>> CMake does not have a mechanism for configuring relative paths via CMake variable.
> 
> find_file() does this. Look at how I select libc++ header paths in the libc++abi CMakeLists.txt.

Sorry, I could have been more clear. I'm using `find_library` but if the library is not along a standard path you need to specify `CMAKE_LIBRARY_PATH`. I incorrectly said relative paths but I meant runtime paths. Let me know if I got anything wrong or am missing anything.

I've respondeded to the inline comments and the changes will follow shortly after.

================
Comment at: lib/CMakeLists.txt:46
@@ -45,1 +45,3 @@
 
+# Linking against libcxxrt requires libdl on linux.
+if ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libcxxrt" AND
----------------
danalbert wrote:
> libcxxrt is the one that should be linking libdl.
> 
> For a static libcxxrt we will need this though. Do we have a way to tell if we're linking a static abi lib?
Huh, Thanks for the correction. I  didn't think of that. I don't know if we can tell the difference. I don't think i'll support that for now, but I'll strip this part out.

================
Comment at: lib/CMakeLists.txt:48
@@ +47,3 @@
+if ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libcxxrt" AND
+    "${CMAKE_SYSTEM}" MATCHES "Linux")
+  append_if(libraries LIBCXX_HAS_DL_LIB dl)
----------------
emaste wrote:
> danalbert wrote:
> > What about FreeBSD? If the BSDs need it too, I think you actually just want to remove the second half of this conditional and let append_if work out this detail.
> libdl isn't needed on FreeBSD, but that case is already handled by append_if, is it not?
It's not handled by the append if, but the outer conditional which is only true on linux.

================
Comment at: test/lit.cfg:342
@@ +341,3 @@
+        lpaths = self.get_lit_conf('library_paths', '').split(';')
+        lpaths = list((l for l in lpaths if l.strip()))
+        self.link_flags += ['-L' + self.obj_root + '/lib']
----------------
danalbert wrote:
>     [l for l in lpaths if l.strip()]
*bows*. That is such a better way to write it.

================
Comment at: www/index.html:440
@@ +439,3 @@
+    <p>
+        <strong>Note: This is not recommended in almost all cases.</strong>
+    </p>
----------------
danalbert wrote:
> Then why are we doing this?
> 
> Assuming we do want to do this, should explain why this isn't recommended.
Great note. I'll make the change. The reason we are doing this is 

1. Testing.
2. The abililty to build and test on machines where libraries cannot be installed. 

================
Comment at: www/index.html:452
@@ +451,3 @@
+        <ul>
+        <li><code>CC=clang CXX=clang++ cmake -G "Unix Makefiles"
+                 -DLIBCXX_CXX_ABI=libc++abi
----------------
danalbert wrote:
> Drop the -G part. It's the default.
Will do.

================
Comment at: www/index.html:456
@@ +455,3 @@
+                 -DCMAKE_LIBRARY_PATH="/path/to/libcxxabi-build/lib"
+                 <libc++-source-dir></code></li>
+        <li><code>export RPATH=/path/to/libcxxabi-build/lib</code></li>
----------------
danalbert wrote:
> Is this how we phrase this elsewhere in the page? I thought we did path/to/libcxx.
I'll double check and make the change.

http://reviews.llvm.org/D5038






More information about the cfe-commits mailing list