[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: cmake/modules/LLDBConfig.cmake:354
+
+  check_library_exists(dl dlopen "" HAVE_LIBDL)
+  if (HAVE_LIBDL)
----------------
krytarowski wrote:
> krytarowski wrote:
> > labath wrote:
> > > krytarowski wrote:
> > > > mgorny wrote:
> > > > > krytarowski wrote:
> > > > > > A more portable form of this:
> > > > > > 
> > > > > > ```
> > > > > > foreach(lib ${CMAKE_DL_LIBS})
> > > > > > list(APPEND system_libs -l${lib})
> > > > > > endforeach()
> > > > > > ```
> > > > > > 
> > > > > > Haiku needs 2 libraries, other Unices can use `-lldl` or similar.
> > > > > Didn't this raise the problem of cmake using full paths on some systems? 
> > > > No.
> > > Do you have some reference for the portability claim? (mainly because I'm trying to learn more about how cmake works, but also because I'd rather let cmake figure out when to prepend -l instead of doing it myself).
> > > 
> > > I don't see how the "two libraries" thingy comes into play here. It could be a problem if we are mismatching lists and strings but I don't see why that would necessitate manually tagging with -l.
> > dlopen(3)-like functions in `-ldl` is Linux; HPUX uses `-ldl`, Darwin/BSDs no extra libraries, Haiku `-lroot -lbe` etc.
> > 
> > https://github.com/Kitware/CMake/search?p=2&q=CMAKE_DL_LIBS&type=&utf8=%E2%9C%93
> > 
> > I know that we don't support multiple OSes as of today, but we can make it compatible with every recognizable one with this change.
> > 
> > It looks like the proper syntax is to drop `-l` from my first proposal:
> > 
> > ```
> > foreach(lib ${CMAKE_DL_LIBS})
> > list(APPEND system_libs ${lib})
> > endforeach()
> > ```
> Or perhaps even unconditional: `list(APPEND system_libs ${CMAKE_DL_LIBS})`
I like that: :D


https://reviews.llvm.org/D44379