Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-27 Thread Pavel Labath via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL258921: Fix linking with LLVM_LINK_LLVM_DYLIB=ON (authored 
by labath).

Changed prior to commit:
  http://reviews.llvm.org/D16293?vs=45667&id=46116#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D16293

Files:
  lldb/trunk/cmake/modules/AddLLDB.cmake

Index: lldb/trunk/cmake/modules/AddLLDB.cmake
===
--- lldb/trunk/cmake/modules/AddLLDB.cmake
+++ lldb/trunk/cmake/modules/AddLLDB.cmake
@@ -56,7 +56,7 @@
   if (PARAM_OBJECT)
 add_library(${name} ${libkind} ${srcs})
   else()
-llvm_add_library(${name} ${libkind} ${srcs})
+llvm_add_library(${name} ${libkind} DISABLE_LLVM_LINK_LLVM_DYLIB ${srcs})
 
 lldb_link_common_libs(${name} "${libkind}")
 
@@ -93,7 +93,7 @@
 endmacro(add_lldb_library)
 
 macro(add_lldb_executable name)
-  add_llvm_executable(${name} ${ARGN})
+  add_llvm_executable(${name} DISABLE_LLVM_LINK_LLVM_DYLIB ${ARGN})
   set_target_properties(${name} PROPERTIES FOLDER "lldb executables")
 endmacro(add_lldb_executable)
 


Index: lldb/trunk/cmake/modules/AddLLDB.cmake
===
--- lldb/trunk/cmake/modules/AddLLDB.cmake
+++ lldb/trunk/cmake/modules/AddLLDB.cmake
@@ -56,7 +56,7 @@
   if (PARAM_OBJECT)
 add_library(${name} ${libkind} ${srcs})
   else()
-llvm_add_library(${name} ${libkind} ${srcs})
+llvm_add_library(${name} ${libkind} DISABLE_LLVM_LINK_LLVM_DYLIB ${srcs})
 
 lldb_link_common_libs(${name} "${libkind}")
 
@@ -93,7 +93,7 @@
 endmacro(add_lldb_library)
 
 macro(add_lldb_executable name)
-  add_llvm_executable(${name} ${ARGN})
+  add_llvm_executable(${name} DISABLE_LLVM_LINK_LLVM_DYLIB ${ARGN})
   set_target_properties(${name} PROPERTIES FOLDER "lldb executables")
 endmacro(add_lldb_executable)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-26 Thread Zachary Turner via lldb-commits 
zturner added a comment.

Sorry for the delay, lgtm


http://reviews.llvm.org/D16293



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

Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-26 Thread Zachary Turner via lldb-commits 
Sorry for the delay, lgtm

On Tue, Jan 26, 2016 at 1:59 AM Pavel Labath  wrote:

> labath added a comment.
>
> Ping?
>
>
> http://reviews.llvm.org/D16293
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-26 Thread Pavel Labath via lldb-commits 
labath added a comment.

Ping?


http://reviews.llvm.org/D16293



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

Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-22 Thread Pavel Labath via lldb-commits 
labath updated this revision to Diff 45667.
labath added a comment.

Let's try a different approach. This should disable the logic of
LINK_LLVM_DYLIB for lldb binaries, and thereby enabling that build
to work. It should be the safest thing in the short term, and we
can figure out a better fix later.


http://reviews.llvm.org/D16293

Files:
  cmake/modules/AddLLDB.cmake

Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -56,7 +56,7 @@
   if (PARAM_OBJECT)
 add_library(${name} ${libkind} ${srcs})
   else()
-llvm_add_library(${name} ${libkind} ${srcs})
+llvm_add_library(${name} ${libkind} DISABLE_LLVM_LINK_LLVM_DYLIB ${srcs})
 
 lldb_link_common_libs(${name} "${libkind}")
 
@@ -93,7 +93,7 @@
 endmacro(add_lldb_library)
 
 macro(add_lldb_executable name)
-  add_llvm_executable(${name} ${ARGN})
+  add_llvm_executable(${name} DISABLE_LLVM_LINK_LLVM_DYLIB ${ARGN})
   set_target_properties(${name} PROPERTIES FOLDER "lldb executables")
 endmacro(add_lldb_executable)
 


Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -56,7 +56,7 @@
   if (PARAM_OBJECT)
 add_library(${name} ${libkind} ${srcs})
   else()
-llvm_add_library(${name} ${libkind} ${srcs})
+llvm_add_library(${name} ${libkind} DISABLE_LLVM_LINK_LLVM_DYLIB ${srcs})
 
 lldb_link_common_libs(${name} "${libkind}")
 
@@ -93,7 +93,7 @@
 endmacro(add_lldb_library)
 
 macro(add_lldb_executable name)
-  add_llvm_executable(${name} ${ARGN})
+  add_llvm_executable(${name} DISABLE_LLVM_LINK_LLVM_DYLIB ${ARGN})
   set_target_properties(${name} PROPERTIES FOLDER "lldb executables")
 endmacro(add_lldb_executable)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-21 Thread Zachary Turner via lldb-commits 
zturner added a comment.

Unfortunately that's not going to work.  Because lldb libraries are not layered 
very well, linking against any one library is going to cause a transitive link 
dependency on every other library.  I did a lot of work to improve that in 
order to get the Python stuff separated out, but it's still not in a great 
state.  So right now, there's no way to just link against host, because that's 
the same as linking against everything


http://reviews.llvm.org/D16293



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

Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-21 Thread Pavel Labath via lldb-commits 
labath added a comment.

I think a similar command as for argdumper would fix this (probably 
`target_link_libraries(lldb, lldbHost)`). I thought about doing that instead, 
but then I realised that netbsd probably needs this as well, and lldb-mi seems 
to be using getopt also, etc. So I figured I'll make this as surgical as 
possible, and enable the fix on linux only.

When this gets into 3.8, I will add a separate patch, which sets the private 
keyword everywhere, getopt (through lldbHost) to lldb and lldb-mi on relevant 
platforms. How does that sound?


http://reviews.llvm.org/D16293



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

Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-21 Thread Pavel Labath via lldb-commits 
labath updated this revision to Diff 45529.
labath added a comment.

Use private keyword for linux only


http://reviews.llvm.org/D16293

Files:
  cmake/modules/AddLLDB.cmake
  tools/argdumper/CMakeLists.txt

Index: tools/argdumper/CMakeLists.txt
===
--- tools/argdumper/CMakeLists.txt
+++ tools/argdumper/CMakeLists.txt
@@ -1,8 +1,16 @@
+include(${LLDB_PROJECT_ROOT}/cmake/LLDBDependencies.cmake)
+
 add_lldb_executable(lldb-argdumper
   argdumper.cpp
   )
 
-target_link_libraries(lldb-argdumper liblldb)
+if (LLDB_LINKER_SUPPORTS_GROUPS)
+  target_link_libraries(lldb-argdumper -Wl,--start-group ${LLDB_USED_LIBS} 
-Wl,--end-group)
+else()
+  target_link_libraries(lldb-argdumper ${LLDB_USED_LIBS})
+endif()
+llvm_config(lldb-argdumper ${LLVM_LINK_COMPONENTS})
+
 
 install(TARGETS lldb-argdumper
   RUNTIME DESTINATION bin)
Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -4,7 +4,11 @@
   endif()
 
   if(${targetkind} MATCHES "SHARED")
-set(LINK_KEYWORD ${cmake_2_8_12_PUBLIC})
+if(CMAKE_SYSTEM_NAME MATCHES "Linux")
+  set(LINK_KEYWORD ${cmake_2_8_12_PRIVATE})
+else()
+  set(LINK_KEYWORD ${cmake_2_8_12_PUBLIC})
+endif()
   endif()
 
   if(${targetkind} MATCHES "SHARED" OR ${targetkind} MATCHES "EXE")
@@ -62,10 +66,10 @@
 
 if (PARAM_SHARED)
   if (LLDB_LINKER_SUPPORTS_GROUPS)
-target_link_libraries(${name} ${cmake_2_8_12_PUBLIC}
+target_link_libraries(${name} ${cmake_2_8_12_PRIVATE}
 -Wl,--start-group ${CLANG_USED_LIBS} -Wl,--end-group)
   else()
-target_link_libraries(${name} ${cmake_2_8_12_PUBLIC} 
${CLANG_USED_LIBS})
+target_link_libraries(${name} ${cmake_2_8_12_PRIVATE} 
${CLANG_USED_LIBS})
   endif()
 endif()
 llvm_config(${name} ${LLVM_LINK_COMPONENTS})


Index: tools/argdumper/CMakeLists.txt
===
--- tools/argdumper/CMakeLists.txt
+++ tools/argdumper/CMakeLists.txt
@@ -1,8 +1,16 @@
+include(${LLDB_PROJECT_ROOT}/cmake/LLDBDependencies.cmake)
+
 add_lldb_executable(lldb-argdumper
   argdumper.cpp
   )
 
-target_link_libraries(lldb-argdumper liblldb)
+if (LLDB_LINKER_SUPPORTS_GROUPS)
+  target_link_libraries(lldb-argdumper -Wl,--start-group ${LLDB_USED_LIBS} -Wl,--end-group)
+else()
+  target_link_libraries(lldb-argdumper ${LLDB_USED_LIBS})
+endif()
+llvm_config(lldb-argdumper ${LLVM_LINK_COMPONENTS})
+
 
 install(TARGETS lldb-argdumper
   RUNTIME DESTINATION bin)
Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -4,7 +4,11 @@
   endif()
 
   if(${targetkind} MATCHES "SHARED")
-set(LINK_KEYWORD ${cmake_2_8_12_PUBLIC})
+if(CMAKE_SYSTEM_NAME MATCHES "Linux")
+  set(LINK_KEYWORD ${cmake_2_8_12_PRIVATE})
+else()
+  set(LINK_KEYWORD ${cmake_2_8_12_PUBLIC})
+endif()
   endif()
 
   if(${targetkind} MATCHES "SHARED" OR ${targetkind} MATCHES "EXE")
@@ -62,10 +66,10 @@
 
 if (PARAM_SHARED)
   if (LLDB_LINKER_SUPPORTS_GROUPS)
-target_link_libraries(${name} ${cmake_2_8_12_PUBLIC}
+target_link_libraries(${name} ${cmake_2_8_12_PRIVATE}
 -Wl,--start-group ${CLANG_USED_LIBS} -Wl,--end-group)
   else()
-target_link_libraries(${name} ${cmake_2_8_12_PUBLIC} ${CLANG_USED_LIBS})
+target_link_libraries(${name} ${cmake_2_8_12_PRIVATE} ${CLANG_USED_LIBS})
   endif()
 endif()
 llvm_config(${name} ${LLVM_LINK_COMPONENTS})
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-20 Thread Zachary Turner via lldb-commits 
zturner added a comment.

I get linker errors on Windows with this.  The reason is that Windows doesn't 
have getopt, but getopt is called from from Driver.cpp and linked into the main 
lldb executable.  Since liblldb is built as a shared library, when the keyword 
is public it's linking the liblldb inputs into the executable, which works.  
But when liblldb uses the private keyword, then the inputs don't get fed into 
lldb.exe and it fails to link.

A better solution would involve layering the windows getopt implementation more 
appropriately so that it could just be linked directly into lldb.exe.  But for 
now, I think you will need to put this keyword change behind an OS flag so that 
it continues to use public on Windows (perhaps with a comment explaining why 
this is necessary)


http://reviews.llvm.org/D16293



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

Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-20 Thread Pavel Labath via lldb-commits 
labath added a comment.

Zachary, could you take a look at this please. I'd like to squeeze this into 
3.8 if it is working..

> At some point lldb-argdumper is planned to be reworked just slightly so it 
> had no dependencies on the lldb core. (That would have avoided this I 
> suspect.)


It would, although I don't see a problem with the argdumper re-using the json 
parser from lldb (even though the parser is not the public interface of 
liblldb). The whole LINKER_SUPPORTS_GROUPS thing could be beautified a bit (I 
might do that afterwards), but otherwise, the whole thing seems reasonable to 
me.


http://reviews.llvm.org/D16293



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

Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-19 Thread Todd Fiala via lldb-commits 
tfiala added a subscriber: tfiala.
tfiala added a comment.

> Setting the dependencies as private required a fixup of the argdumper link 
> command, as it was not


actually linking to liblldb (it was referring to symbols from the lldb_private 
namespace, which
are not exposed in liblldb),

At some point lldb-argdumper is planned to be reworked just slightly so it had 
no dependencies on the lldb core.  (That would have avoided this I suspect.)


http://reviews.llvm.org/D16293



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

Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-18 Thread Zachary Turner via lldb-commits 
It's a holiday over here today, so i can't look at this until tomorrow.

On Mon, Jan 18, 2016 at 6:04 AM Pavel Labath  wrote:

> labath added a comment.
>
> I don't quite know how windows linking works, so please give this a run to
> make sure it does not break anything for you.
>
>
> http://reviews.llvm.org/D16293
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-18 Thread Pavel Labath via lldb-commits 
labath added a comment.

I don't quite know how windows linking works, so please give this a run to make 
sure it does not break anything for you.


http://reviews.llvm.org/D16293



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

[Lldb-commits] [PATCH] D16293: [cmake] Make dependencies of lldb libraries private

2016-01-18 Thread Pavel Labath via lldb-commits 
labath created this revision.
labath added a reviewer: zturner.
labath added a subscriber: lldb-commits.

The dependencies of our libraries (only liblldb, really) we marked as public, 
which caused all
their dependencies to be repeated when linking any executables to them. This is 
a problem because
then all the .a files would be linked twice, once to liblldb and once again to 
lldb. I am not
sure why, but for some reason this only surfaced when doing a 
LLVM_LINK_LLVM_DYLIB=ON build,
where we ended having two copies of some symbols and different parts of code 
referring to
different copies.

Setting the dependencies as private required a fixup of the argdumper link 
command, as it was not
actually linking to liblldb (it was referring to symbols from the lldb_private 
namespace, which
are not exposed in liblldb), but rather to the individual component libraries 
(where these
symbols are still available). Since these symbols are now not added to the 
command line as
dependencies of liblldb, I needed to add them explicitly.

http://reviews.llvm.org/D16293

Files:
  cmake/modules/AddLLDB.cmake
  tools/argdumper/CMakeLists.txt

Index: tools/argdumper/CMakeLists.txt
===
--- tools/argdumper/CMakeLists.txt
+++ tools/argdumper/CMakeLists.txt
@@ -1,8 +1,16 @@
+include(${LLDB_PROJECT_ROOT}/cmake/LLDBDependencies.cmake)
+
 add_lldb_executable(lldb-argdumper
   argdumper.cpp
   )
 
-target_link_libraries(lldb-argdumper liblldb)
+if (LLDB_LINKER_SUPPORTS_GROUPS)
+  target_link_libraries(lldb-argdumper -Wl,--start-group ${LLDB_USED_LIBS} 
-Wl,--end-group)
+else()
+  target_link_libraries(lldb-argdumper ${LLDB_USED_LIBS})
+endif()
+llvm_config(lldb-argdumper ${LLVM_LINK_COMPONENTS})
+
 
 install(TARGETS lldb-argdumper
   RUNTIME DESTINATION bin)
Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -4,7 +4,7 @@
   endif()
 
   if(${targetkind} MATCHES "SHARED")
-set(LINK_KEYWORD ${cmake_2_8_12_PUBLIC})
+set(LINK_KEYWORD ${cmake_2_8_12_PRIVATE})
   endif()
 
   if(${targetkind} MATCHES "SHARED" OR ${targetkind} MATCHES "EXE")
@@ -62,10 +62,10 @@
 
 if (PARAM_SHARED)
   if (LLDB_LINKER_SUPPORTS_GROUPS)
-target_link_libraries(${name} ${cmake_2_8_12_PUBLIC}
+target_link_libraries(${name} ${cmake_2_8_12_PRIVATE}
 -Wl,--start-group ${CLANG_USED_LIBS} -Wl,--end-group)
   else()
-target_link_libraries(${name} ${cmake_2_8_12_PUBLIC} 
${CLANG_USED_LIBS})
+target_link_libraries(${name} ${cmake_2_8_12_PRIVATE} 
${CLANG_USED_LIBS})
   endif()
 endif()
 llvm_config(${name} ${LLVM_LINK_COMPONENTS})


Index: tools/argdumper/CMakeLists.txt
===
--- tools/argdumper/CMakeLists.txt
+++ tools/argdumper/CMakeLists.txt
@@ -1,8 +1,16 @@
+include(${LLDB_PROJECT_ROOT}/cmake/LLDBDependencies.cmake)
+
 add_lldb_executable(lldb-argdumper
   argdumper.cpp
   )
 
-target_link_libraries(lldb-argdumper liblldb)
+if (LLDB_LINKER_SUPPORTS_GROUPS)
+  target_link_libraries(lldb-argdumper -Wl,--start-group ${LLDB_USED_LIBS} -Wl,--end-group)
+else()
+  target_link_libraries(lldb-argdumper ${LLDB_USED_LIBS})
+endif()
+llvm_config(lldb-argdumper ${LLVM_LINK_COMPONENTS})
+
 
 install(TARGETS lldb-argdumper
   RUNTIME DESTINATION bin)
Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -4,7 +4,7 @@
   endif()
 
   if(${targetkind} MATCHES "SHARED")
-set(LINK_KEYWORD ${cmake_2_8_12_PUBLIC})
+set(LINK_KEYWORD ${cmake_2_8_12_PRIVATE})
   endif()
 
   if(${targetkind} MATCHES "SHARED" OR ${targetkind} MATCHES "EXE")
@@ -62,10 +62,10 @@
 
 if (PARAM_SHARED)
   if (LLDB_LINKER_SUPPORTS_GROUPS)
-target_link_libraries(${name} ${cmake_2_8_12_PUBLIC}
+target_link_libraries(${name} ${cmake_2_8_12_PRIVATE}
 -Wl,--start-group ${CLANG_USED_LIBS} -Wl,--end-group)
   else()
-target_link_libraries(${name} ${cmake_2_8_12_PUBLIC} ${CLANG_USED_LIBS})
+target_link_libraries(${name} ${cmake_2_8_12_PRIVATE} ${CLANG_USED_LIBS})
   endif()
 endif()
 llvm_config(${name} ${LLVM_LINK_COMPONENTS})
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits