[CMake] Use PUBLIC link mode for static libraries

[phosek]

Summary

Using INTERFACE prevents the use of imported libraries as we've done
in 00b3d49 because these aren't linked against the target, they're
only made part of the interface. This doesn't affect the output since
static libraries aren't being linked into, but it enables the use of
imported libraries.

[smeenai]

I don't quite understand what's going on here. Are you saying that if you have a static library which depends on an imported library as INTERFACE, targets that link against the static library won't link against the imported library, but if the static library depends on the imported library as PUBLIC, targets that link against the static library will link against the imported library?

[phosek]

In D74106#1862442, @smeenai wrote:

I don't quite understand what's going on here. Are you saying that if you have a static library which depends on an imported library as INTERFACE, targets that link against the static library won't link against the imported library, but if the static library depends on the imported library as PUBLIC, targets that link against the static library will link against the imported library?

If a target A depends on a library B as INTERFACE, the library B becomes a part of A's public interface, that is any library that depends on A will "link" against B (link here doesn't just mean regular linking, but also getting appropriate header dependencies), but A itself wouldn't. In case of PUBLIC, not only B becomes part of A's public interface, A will also link against B.

I hit this problem with D74102 where Support would INTERFACE depend on ZLIB::ZLIB which means that anything depending on Support would get ZLIB::ZLIB dependency (both headers and linking where appropriate), but Support itself wouldn't which defeats the purpose of using the imported library.

I'm not sure when would the INTERFACE semantics on static libraries make sense, IIUC the intent was to avoid linking other libraries into static libraries, but that's not something that CMake would ever do since you cannot really link anything to a static library (the only other semantics that would make sense is merging static libraries together but that's not something CMake does).

[smeenai]

Ah, right, I forgot that "linking" includes header dependencies and so on. This makes sense.

Not this diff, but won't a BUILD_SHARED_LIBS build still have issues here, since then Support will only have a PRIVATE dependency on ZLIB::ZLIB and libraries which depend on Support won't get that dependency? (And if that's not a problem, then I'm wondering why the static case needs to be PUBLIC instead of PRIVATE.)

[phosek]

In D74106#1862689, @smeenai wrote:

Ah, right, I forgot that "linking" includes header dependencies and so on. This makes sense.

Not this diff, but won't a BUILD_SHARED_LIBS build still have issues here, since then Support will only have a PRIVATE dependency on ZLIB::ZLIB and libraries which depend on Support won't get that dependency? (And if that's not a problem, then I'm wondering why the static case needs to be PUBLIC instead of PRIVATE.)

That should be fine since only Support needs that dependency, not its dependents as zlib API isn't exposed across the Support API/ABI boundary.

[chapuni]

@phosek Please let me know how to reproduce your issue. I haven't reproduced yet.

llvm/cmake/modules/AddLLVM.cmake

else()
     #
     # It would be nice to verify that we have the dependencies for this library
     # name, but using get_property(... SET) doesn't suffice to determine if a
     # property has been set to an empty value.
     get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name})
   endif()
 
   if(ARG_STATIC)
-    set(libtype INTERFACE)
+    set(libtype PUBLIC)
   else()
     # We can use PRIVATE since SO knows its dependent libs.
     set(libtype PRIVATE)
   endif()
 
   target_link_libraries(${name} ${libtype}
       ${ARG_LINK_LIBS}
       ${lib_deps}