[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I proposed a revert here for those who are interested: 
https://reviews.llvm.org/D91099


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D42459/new/

https://reviews.llvm.org/D42459

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D42459#2383794 , @hintonda wrote:

> In D42459#2383753 , @ldionne wrote:
>
>> @hintonda  Is there a reason why this was done except for "standardizing the 
>> usage across projects"?
>
> It was just for standardization.  Please feel free to rollback or modify as 
> necessary.

Ok, thanks a lot for the information. If things worked before, I would like to 
revert this change as we are working around it in our internal build, and it 
just bit me today. I'll send a review.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D42459/new/

https://reviews.llvm.org/D42459

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D42459#2383753 , @ldionne wrote:

> @hintonda  Is there a reason why this was done except for "standardizing the 
> usage across projects"?

It was just for standardization.  Please feel free to rollback or modify as 
necessary.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D42459/new/

https://reviews.llvm.org/D42459

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

@hintonda  Is there a reason why this was done except for "standardizing the 
usage across projects"?

I'm asking because this change prevents us from being able to avoid having a 
rpath completely when building libc++. That is the behavior we want when 
building libc++ as a system library, in which case we specify the install name 
dir to be `/usr/lib`, and we don't want any rpath to appear.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D42459/new/

https://reviews.llvm.org/D42459

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL323492: Reland: [cmake] [libcxx] Call llvm_setup_rpath() 
when adding shared libraries. (authored by dhinton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D42459

Files:
  libcxx/trunk/lib/CMakeLists.txt


Index: libcxx/trunk/lib/CMakeLists.txt
===
--- libcxx/trunk/lib/CMakeLists.txt
+++ libcxx/trunk/lib/CMakeLists.txt
@@ -222,6 +222,9 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES


Index: libcxx/trunk/lib/CMakeLists.txt
===
--- libcxx/trunk/lib/CMakeLists.txt
+++ libcxx/trunk/lib/CMakeLists.txt
@@ -222,6 +222,9 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX323492: Reland: [cmake] [libcxx] Call llvm_setup_rpath() 
when adding shared libraries. (authored by dhinton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D42459?vs=131506=131526#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42459

Files:
  lib/CMakeLists.txt


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -222,6 +222,9 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -222,6 +222,9 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 131506.
hintonda added a comment.

Only call llvm_setup_rpath() if LLVM_FOUND is true.


Repository:
  rCXX libc++

https://reviews.llvm.org/D42459

Files:
  lib/CMakeLists.txt


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -222,6 +222,9 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -222,6 +222,9 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via Phabricator via cfe-commits]

hintonda requested review of this revision.
hintonda added a comment.

I probably should have put this in the summary, but the reason for this patch 
isn't just to clean up the cmake files, it's to fix a cmake configuration 
failure when cross-compiling for Linux on Darwin:

CMake Error at 
/Users/dhinton/projects/llvm_project/libcxx/lib/CMakeLists.txt:224 
(add_library):

  The install of the cxx_shared target requires changing an RPATH from the
  build tree, but this is not supported with the Ninja generator unless on an
  ELF-based platform.  The CMAKE_BUILD_WITH_INSTALL_RPATH variable may be set
  to avoid this relinking step.


Repository:
  rCXX libc++

https://reviews.llvm.org/D42459



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via Phabricator via cfe-commits]

hintonda reopened this revision.
hintonda added a comment.
This revision is now accepted and ready to land.

Reopen after rollback.

@smeenai pointed out LLVM may not be available in standalone builds


Repository:
  rCXX libc++

https://reviews.llvm.org/D42459



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX323453: [cmake] [libcxx] Call llvm_setup_rpath() when 
adding shared libraries. (authored by dhinton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D42459?vs=131195=131478#toc

Repository:
  rCXX libc++

https://reviews.llvm.org/D42459

Files:
  lib/CMakeLists.txt


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -222,6 +222,7 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  llvm_setup_rpath(cxx_shared)
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -222,6 +222,7 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  llvm_setup_rpath(cxx_shared)
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits