Fix the static bundle missing session-deps libraries

CMakeLists.txt

@@ -17,7 +17,7 @@ if(CCACHE_PROGRAM)
 endif()
 
 project(libsession-util
-    VERSION 1.9.3
+    VERSION 1.9.4
     DESCRIPTION "Session client utility library"
     LANGUAGES ${LANGS})
 
@@ -167,7 +167,32 @@ if(STATIC_BUNDLE)
 
     include(combine_archives)
 
-    combine_archives(session-util libsession-static-bundle "${LIBSESSION_STATIC_BUNDLE_LIBS}")
+    # Threads::Threads comes from find_package() calls in subdirectories, which make it visible only
+    # there; the bundle walk has to be able to see it.
+    find_package(Threads REQUIRED)
+    libsession_static_bundle_libs(bundle_archives bundle_system_libs ${libsession_export_targets})
+    message(STATUS "Static bundle libsession-util.a combines: ${bundle_archives}")
+    message(STATUS "Static bundle libsession-util.a also needs: ${bundle_system_libs}")
+    combine_archives(session-util libsession-static-bundle ${bundle_archives})
+
+    # Links the bundle and only what the walk says it needs from the system, so that a library
+    # missing from the bundle fails here rather than in whatever consumes it.  Headers come from our
+    # targets' usage requirements, without linking them.
+    add_executable(static-bundle-test tests/static_bundle.cpp)
+    if(ENABLE_NETWORKING)
+        set(bundle_header_target network)
+    else()
+        set(bundle_header_target config)
+        target_compile_definitions(static-bundle-test PRIVATE DISABLE_NETWORKING)
+    endif()
+    target_include_directories(static-bundle-test PRIVATE
+        $<TARGET_PROPERTY:${bundle_header_target},INTERFACE_INCLUDE_DIRECTORIES>)
+    target_compile_definitions(static-bundle-test PRIVATE
+        $<TARGET_PROPERTY:${bundle_header_target},INTERFACE_COMPILE_DEFINITIONS>)
+    target_link_libraries(static-bundle-test PRIVATE
+        "${CMAKE_CURRENT_BINARY_DIR}/libsession-util.a" ${bundle_system_libs})
+    add_dependencies(static-bundle-test session-util)
+
     set(lib_folder lib)
     if(IOS)
         set(lib_folder "${lib_folder}-${ARCH}")

cmake/AddStaticBundleLib.cmake

@@ -1,36 +1,74 @@
 
-set(LIBSESSION_STATIC_BUNDLE_LIBS "" CACHE INTERNAL "list of libs to go into the static bundle lib")
-
-function(_libsession_static_bundle_append tgt)
-    list(APPEND LIBSESSION_STATIC_BUNDLE_LIBS "${tgt}")
-    set(LIBSESSION_STATIC_BUNDLE_LIBS "${LIBSESSION_STATIC_BUNDLE_LIBS}" CACHE INTERNAL "")
-endfunction()
-
 # Call as:
 #
-#     libsession_static_bundle(target [target2 ...])
+#     libsession_static_bundle_libs(ARCHIVES_VAR SYSTEM_VAR target [target2 ...])
 #
-# to append the given target(s) to the list of libraries that will be combined to make the static
-# bundled libsession-util.a.
-function(libsession_static_bundle)
-    foreach(tgt IN LISTS ARGN)
-        if(TARGET "${tgt}" AND NOT "${tgt}" IN_LIST LIBSESSION_STATIC_BUNDLE_LIBS)
-            get_target_property(tgt_type ${tgt} TYPE)
-
-            if(tgt_type STREQUAL STATIC_LIBRARY)
-                message(STATUS "Adding ${tgt} to libsession-util bundled library list")
-                _libsession_static_bundle_append("${tgt}")
+# to walk the link closure of the given targets.  ARCHIVES_VAR is set to every static library in
+# it: the archives that combine into the STATIC_BUNDLE libsession-util.a.  SYSTEM_VAR is set to
+# everything else the closure links: system libraries such as iconv, Apple frameworks, linker flags
+# and imported non-static targets such as Threads::Threads.  None of those can go into an archive,
+# so whatever links the bundle has to supply them.
+#
+# The closure is read from the targets' link properties, so this has to be called after every
+# target_link_libraries() it depends on.
+function(libsession_static_bundle_libs archives_out system_out)
+    set(pending ${ARGN})
+    set(visited)
+    set(archives)
+    set(system)
+    while(NOT "${pending}" STREQUAL "")
+        list(POP_FRONT pending tgt)
+        if(tgt MATCHES "^\\$<LINK_ONLY:(.+)>$")
+            set(tgt "${CMAKE_MATCH_1}")
+        endif()
+        if(NOT TARGET "${tgt}")
+            # Silently skipping either of these could drop a library from the bundle, which then
+            # only shows up when something links against it.  A `::` name is an imported target
+            # created in some subdirectory without GLOBAL, which we cannot see (let alone look
+            # inside) from here: find_package() it before calling this.
+            if(tgt MATCHES "\\$<")
+                message(FATAL_ERROR "Static bundle: don't know how to follow link item ${tgt}")
             endif()
-
-            if(tgt_type STREQUAL INTERFACE_LIBRARY)
-                get_target_property(tgt_link_deps ${tgt} INTERFACE_LINK_LIBRARIES)
-            else()
-                get_target_property(tgt_link_deps ${tgt} LINK_LIBRARIES)
+            if(tgt MATCHES "::")
+                message(FATAL_ERROR "Static bundle: imported target ${tgt} is not visible here")
             endif()
+            list(APPEND system "${tgt}")
+            continue()
+        endif()
+        get_target_property(aliased ${tgt} ALIASED_TARGET)
+        if(aliased)
+            set(tgt "${aliased}")
+        endif()
+        if(tgt IN_LIST visited)
+            continue()
+        endif()
+        list(APPEND visited "${tgt}")
 
-            if(tgt_link_deps)
-                libsession_static_bundle(${tgt_link_deps})
-            endif()
+        get_target_property(type ${tgt} TYPE)
+        get_target_property(imported ${tgt} IMPORTED)
+        if(type STREQUAL STATIC_LIBRARY)
+            list(APPEND archives "${tgt}")
+        elseif(imported)
+            # Something the system provides (Threads::Threads).  Linking it by name brings
+            # everything it carries, including link options, which this walk does not read.
+            list(APPEND system "${tgt}")
+            continue()
+        endif()
+
+        # An imported target (which is what session-deps produces for each static dependency)
+        # carries its dependencies only in INTERFACE_LINK_LIBRARIES; one built here also has its
+        # private ones in LINK_LIBRARIES.
+        get_target_property(deps ${tgt} INTERFACE_LINK_LIBRARIES)
+        if(NOT type STREQUAL INTERFACE_LIBRARY AND NOT imported)
+            get_target_property(link_deps ${tgt} LINK_LIBRARIES)
+            list(APPEND deps ${link_deps})
         endif()
-    endforeach()
+        # ::@(dir) ... ::@ are cmake's own markers around items that target_link_libraries() added
+        # from another directory, not link items themselves.
+        list(FILTER deps EXCLUDE REGEX "-NOTFOUND$|^::@")
+        list(APPEND pending ${deps})
+    endwhile()
+    list(REMOVE_DUPLICATES system)
+    set(${archives_out} "${archives}" PARENT_SCOPE)
+    set(${system_out} "${system}" PARENT_SCOPE)
 endfunction()

external/CMakeLists.txt

@@ -24,7 +24,6 @@ endfunction()
 # Stock libsodium, not the libsodium-internal fork: the one thing the fork exposed that we used was
 # the field arithmetic behind xed25519::pubkey(), which src/xed25519-tweetnacl.cpp now supplies.
 session_dep(libsodium 1.0.21)
-libsession_static_bundle(sessiondep::libsodium)
 
 # Needed even without networking: oxenc is part of the public interface of `common`.
 if(NOT TARGET oxenc::oxenc)
@@ -40,7 +39,6 @@ if(ENABLE_NETWORKING AND ENABLE_NETWORKING_SROUTER)
     set(SROUTER_JEMALLOC OFF CACHE BOOL "")
 
     add_static_subdirectory(session-router EXCLUDE_FROM_ALL)
-    libsession_static_bundle(session-router::core)
 endif()
 
 # session-router provides oxen::quic when it is built (above); otherwise pull in oxen-libquic alone.
@@ -80,7 +78,6 @@ sessiondep_or_submodule(protobuf-lite 3.21 protobuf protobuf::libprotobuf-lite)
 # configs get deduplicated at the swarm.
 set(BUILD_STATIC_libzstd ON CACHE BOOL "" FORCE)
 session_dep(libzstd 1.5)
-libsession_static_bundle(sessiondep::libzstd)
 
 if(NOT TARGET nlohmann_json::nlohmann_json)
     set(JSON_BuildTests OFF CACHE INTERNAL "")
@@ -89,7 +86,6 @@ if(NOT TARGET nlohmann_json::nlohmann_json)
 endif()
 
 session_dep(simdutf 7)
-libsession_static_bundle(sessiondep::simdutf)
 
 # We need Howard Hinnant's header-only date library for now because the STL implementation of
 # std::chrono::parse() is spotty or broken on:

proto/CMakeLists.txt

@@ -1,7 +1,4 @@
 
-libsession_static_bundle(protobuf::libprotobuf-lite)
-
-
 add_library(protos
     SessionProtos.pb.cc
     WebSocketResources.pb.cc)
@@ -20,8 +17,6 @@ else()
     target_compile_options(protos PUBLIC -Wno-error=unused-parameter)
 endif()
 
-libsession_static_bundle(protos)
-
 add_library(libsession::protos ALIAS protos)
 list(APPEND libsession_export_targets protos)
 set(libsession_export_targets "${libsession_export_targets}" PARENT_SCOPE)

src/CMakeLists.txt

@@ -35,8 +35,6 @@ macro(add_libsession_util_library name)
         OUTPUT_NAME session-${name}
         SOVERSION ${LIBSESSION_LIBVERSION})
 
-    libsession_static_bundle(${name})
-
     list(APPEND export_targets ${name})
 endmacro()
 
@@ -142,8 +140,8 @@ if(ENABLE_NETWORKING)
         PUBLIC
         crypto
         oxen::quic
-        PRIVATE
         nlohmann_json::nlohmann_json
+        PRIVATE
         sessiondep::libsodium
         sessiondep::nettle
         date::date
@@ -205,7 +203,6 @@ else()
     endif()
 endif()
 add_library(version STATIC version.c)
-libsession_static_bundle(version)
 target_include_directories(version PRIVATE ../include)
 target_link_libraries(common INTERFACE version)
 

tests/CMakeLists.txt

@@ -91,10 +91,3 @@ endif()
 
 add_executable(swarm-auth-test EXCLUDE_FROM_ALL swarm-auth-test.cpp)
 target_link_libraries(swarm-auth-test PRIVATE config)
-
-if(STATIC_BUNDLE)
-    add_executable(static-bundle-test static_bundle.cpp)
-    target_include_directories(static-bundle-test PUBLIC ../include)
-    target_link_libraries(static-bundle-test PRIVATE "${PROJECT_BINARY_DIR}/libsession-util.a" oxenc::oxenc quic)
-    add_dependencies(static-bundle-test session-util)
-endif()

tests/static_bundle.cpp

@@ -1,12 +1,21 @@
 // This file isn't designed to do anything useful, but just to test that we can compile and link
-// against the combined static bundle (when using cmake ... -DSTATIC_BUILD=ON)
+// against the combined static bundle (when using cmake ... -DSTATIC_BUNDLE=ON) with nothing else.
+// The calls are there to make the linker pull in the code that needs the bundle's dependencies:
+// config for libsodium/protobuf/zstd, and the network for libquic, gnutls, ngtcp2 and libevent.
 
 #include <random>
 #include <session/config/groups/keys.hpp>
+#ifndef DISABLE_NETWORKING
+#include <session/network/session_network.hpp>
+#endif
 
 int main() {
     if (std::mt19937_64{}() == 123) {
         auto& k = *reinterpret_cast<session::config::groups::Keys*>(12345);
         k.encrypt_message(std::span<const unsigned char>{});
+#ifndef DISABLE_NETWORKING
+        auto& n = *reinterpret_cast<session::network::Network*>(12345);
+        n.close_connections();
+#endif
     }
 }

utils/ios.sh

@@ -222,6 +222,10 @@ if [ "${SHOULD_CREATE_FRAMEWORK}" == "SHOULD_CREATE_FRAMEWORK" ]; then
     # The 'module.modulemap' is needed for XCode to be able to find the headers
     modmap="${OUTPUT_DIR}/module.modulemap"
     echo "module SessionUtil {" >"$modmap"
+    # libunistring (via gnutls -> libidn2) calls iconv, which session-deps takes from the system on
+    # Apple rather than bundling.  An SPM binary target can't declare linker flags, so autolink it
+    # from the module instead.
+    echo "  link \"iconv\"" >>"$modmap"
     echo "  module capi {" >>"$modmap"
     for x in $(cd include && find session -name '*.h'); do
         echo "    header \"$x\"" >>"$modmap"

utils/static-bundle.sh

@@ -71,7 +71,9 @@ cmake -G 'Unix Makefiles' \
     "$@" \
     "$projdir"
 
-make -j${JOBS:-$(nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null || echo 1)} VERBOSE=1 session-util
+# static-bundle-test links against nothing but the bundle and its declared system libraries, so
+# building it is what catches a dependency missing from the bundle.
+make -j${JOBS:-$(nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null || echo 1)} VERBOSE=1 session-util static-bundle-test
 
 if [ -z "$archive" ]; then
     exit 0