Consolidation step 3 of 8: the DHT package, one partition per former header [androidbuild] [iosbuild]

MODERNIZATION.md

@@ -855,8 +855,60 @@ exactly that. So consolidation walks the dependency chain from the top:
    IMPROVED: clangd-tidy now runs on all 15 module units — full
    analysis coverage, zero suppressions. Verified: Release build, tn
    tests 12+1, proxyclient importer 15/15, standalone builds, lint
-   green.
-3. C-3 streamr-dht (the largest)
+   green. **RETROFITTED in the C-3 PR to the settled architecture:**
+   partitions became named sub-modules (`streamr.trackerlessnetwork.X`),
+   the primary/umbrella unit was deleted, tests and the proxyclient
+   import the individual sub-modules they use, and the MessageRef
+   ordering operator moved from FifoMapWithTTL to the (exported) protos
+   module — as a non-exported file-scope function it had module linkage
+   and was unreachable from other modules instantiating the map
+   templates. One more sub-module gotcha found here: a purview forward
+   declaration of another module's class (NodeList forward-declared
+   ContentDeliveryRpcRemote) attaches the name to the WRONG module and
+   conflicts with the defining one — the import replaces it.
+3. **C-3 streamr-dht** ✅ — the largest step, and where the module
+   ARCHITECTURE was settled through three owner directives measured
+   back to back:
+   **(i) One file per former header** — all 45 public headers
+   (5,013 lines) became per-header module units under `modules/` in the
+   old directory tree; include/ deleted.
+   **(ii) Named sub-modules, not partitions** (`streamr.dht.X`, not
+   `streamr.dht:X`): partitions cannot be imported by external
+   translation units, and the owner requires TESTS to import exactly
+   the sub-modules they test ("tests are internal to the module").
+   All 41 dht test files import their subjects directly.
+   **(iii) NO umbrella module** — a `streamr.dht` unit re-exporting
+   all sub-modules was built and measured, then DELETED: its compiled
+   interface depends on every sub-module, so anyone importing it
+   reinherits the edit-anything-rebuild-everything amplification.
+   Downstream consumers (11 trackerless-network module units, its
+   integration test, the proxyclient implementation) import the
+   individual dht sub-modules they use — module imports now mirror the
+   old header includes one to one. THE MEASUREMENTS (same mid-chain /
+   leaf sub-module touch, whole-tree Release rebuild): partitions with
+   a re-exporting primary ~480 s; named sub-modules behind an umbrella
+   418/546 s; **no umbrella 129/135 s**. The remaining cost is the true
+   dependency cone of central types plus the serial BMI chain; genuinely
+   narrow edits (tests, unimported leaves) rebuild in seconds.
+   Migration mechanics recorded for the remaining packages: sub-modules
+   are stricter about visibility than partitions — each unit must
+   DIRECTLY import what its code uses (the compiler names the missing
+   module explicitly: "declaration of 'X' must be imported from module
+   'M'"), and shorthand names inherited textually from neighboring
+   headers need the file's own using-declaration. The include-hygiene
+   gotcha from C-2 recurred (`<string>` for Branded-key containers ×16
+   units, `<functional>`, `<mutex>`, `<exception>`, folly `Collect.h`).
+   Incidental fixes: the dht standalone `install.sh` had hard-coded
+   `-fsanitize=address` into every configure (committed debugging
+   leftover, fatal once real code moved into module objects); removed.
+   PR #43's race fix (ReplayEventEmitter base of IPendingConnection)
+   ported into the sub-module during the rebase. One clangd-tidy
+   exclusion (ConnectionLockingTest.cpp, owner-approved selective
+   disabling — the analysis tool cannot resolve folly customization
+   points through module interfaces; the compiler accepts the file).
+   Verified: Release build, dht tests 83/83 (incl. the two new PR #43
+   race tests), tn 12+1, proxyclient 15/15, standalone builds of the
+   whole chain, lint green over all module units.
 4. C-4 streamr-proto-rpc
 5. C-5 streamr-utils
 6. C-6 streamr-logger
@@ -868,6 +920,65 @@ One package per PR, `bench.sh` measured at the dht step and at the end
 per-package — the compiler itself enforces that nothing still includes
 them.
 
+### `import std` verdict (investigated 2026-07-04, child session)
+Owner-requested experiment on the consolidated trackerless-network
+partition, with Android as the gate. Findings (full detail and a
+working conversion template on the `experiment/import-std` branch, file
+`import-std-findings.md` — NOT for merging):
+- Host (Homebrew LLVM 22.1.8): works, including the mixed mode the
+  codebase needs (third-party textual includes + `import std;` purview).
+  The existing `STREAMR_IMPORT_STD` scaffold worked unmodified; one
+  toolchain packaging defect requires passing
+  `-DCMAKE_CXX_STDLIB_MODULES_JSON=$LLVM_PREFIX/lib/c++/libc++.modules.json`.
+- **Android gate: FAILS for practical purposes.** NDK r29 is the first
+  NDK to ship the std module sources, but they do not compile against
+  Bionic as delivered (internal-linkage `ctype` inlines); making them
+  build requires overriding the undocumented `__BIONIC_CTYPE_INLINE`
+  macro, disabling `_FORTIFY_SOURCE` hardening, and hand-patching
+  CMake's cross-compilation stdlib detection. Not defensible.
+- **Decision: do not adopt (not even host-only — it would fork the
+  source style).** Re-test each NDK release with the one-line probe in
+  the findings file; adopt when the NDK's sources compile cleanly with
+  fortify enabled AND CMake's detection passes the toolchain target.
+- **NDK r30 beta 1 retest (owner-requested, 2026-07-04): still FAILS —
+  nothing gained by switching.** The blockers are in Bionic itself
+  (internal-linkage ctype functions AND the `_FORTIFY_SOURCE` stdio
+  wrappers, tested separately), textually unchanged in the pre-release.
+  One improvement found during the retest: CMake's broken
+  cross-compilation standard-library probe can be bypassed CLEANLY by
+  adding the Android `--target` to `CMAKE_CXX_FLAGS` (replaces the
+  indefensible hand-patching of generated files). The full minimal
+  adoption recipe for a future fixed NDK is recorded in
+  `import-std-findings.md` on the `experiment/import-std` branch; the
+  probe must include `-D_FORTIFY_SOURCE=2` or the second failure stays
+  hidden.
+
+### After the consolidation: CI speed (owner-approved, 2026-07-04)
+Even with the per-platform dependency caches fixed (Android ~30 → 9.6
+min, iOS ~35 → 4.6 min), each CI job still compiles the monorepo from
+scratch and runs the tests serially. Planned follow-up work, in payoff
+order — scheduled AFTER C-8 because every consolidation step changes
+the build structure the caches would key on:
+1. **Stop compiling the monorepo twice per job.** `install.sh` builds
+   every package standalone AND then the whole root tree. The
+   standalone-package check validates packaging structure, which does
+   not differ per platform — run it on ONE platform (macOS) only.
+2. **Cache the build directories per platform** (with restore-keys
+   fallbacks, like the dependency caches). Ninja then recompiles only
+   what a pull request touches — and the one-partition-per-header
+   structure is exactly what keeps the rebuild cascade small: editing
+   one partition rebuilds that partition, its importers and the links,
+   not whole packages. Watch the shared 10 GB actions-cache budget;
+   if build trees + five dependency caches start evicting each other,
+   move the dependency cache to a GitHub Packages NuGet feed
+   (documented option from the cache investigation, free for public
+   repositories) to reclaim the budget.
+3. **ccache as a complement** if build-dir caching proves fragile:
+   caches ordinary TUs (all test files — the bulk) but not module
+   BMI compiles (tooling support still poor).
+4. **Randomize test ports, then run ctest in parallel** — tests
+   currently use fixed ports and must run serially.
+
 ### Interim posture (adopted now)
 The façade stage is COMPLETE and delivers: uniform `import streamr.<pkg>`
 consumption, −24% clean builds, 250+ tests through import, and module

cmake/StreamrModules.cmake

@@ -120,6 +120,13 @@ function(streamr_add_module_library TARGET)
     # undeclared). Making the flag part of the PUBLIC interface keeps the
     # BMI and every importer consistent. No-op on other platforms.
     target_compile_options(${TARGET} PUBLIC $<$<PLATFORM_ID:Android>:-pthread>)
+    # The module libraries' objects end up inside the shared
+    # libstreamrproxyclient, so they must be position-independent. macOS
+    # emits PIC by default; on Linux the ELF linker rejects the archive
+    # otherwise ("recompile with -fPIC" against the module vtable/thunk
+    # symbols — first seen when the consolidated code moved into module
+    # objects on the linux-arm64 leg).
+    set_target_properties(${TARGET} PROPERTIES POSITION_INDEPENDENT_CODE ON)
 endfunction()
 
 # streamr_target_module_sources(<target> FILES <unit.cppm>...)
@@ -143,8 +150,10 @@ function(streamr_target_module_sources TARGET)
         FILES ${ARG_FILES})
     set_target_properties(${TARGET} PROPERTIES CXX_SCAN_FOR_MODULES ON)
     target_compile_features(${TARGET} PUBLIC cxx_std_26)
-    # See streamr_add_module_library: BMI/importer -pthread consistency.
+    # See streamr_add_module_library: BMI/importer -pthread consistency
+    # and position independence for the shared-library link.
     target_compile_options(${TARGET} PUBLIC $<$<PLATFORM_ID:Android>:-pthread>)
+    set_target_properties(${TARGET} PROPERTIES POSITION_INDEPENDENT_CODE ON)
 endfunction()
 
 # streamr_enable_imports(<target>)

packages/streamr-dht/CMakeLists.txt

@@ -47,14 +47,17 @@ add_library(streamr-dht)
 set_property(TARGET streamr-dht PROPERTY CXX_STANDARD 26)
 add_library(streamr::streamr-dht ALIAS streamr-dht)
 
-# Module façade (MODERNIZATION.md Part 2): 45 partitions mirroring the
-# header tree + :protos over the generated DhtRpc trio.
-file(GLOB STREAMR_DHT_MODULE_UNITS CONFIGURE_DEPENDS
+# CONSOLIDATED (MODERNIZATION.md Phase 2.6): one partition per former
+# public header, in the same directory tree the headers had (the
+# partition files are the source of truth), plus :protos over the
+# generated DhtRpc trio and the primary interface unit that re-exports
+# every partition.
+file(GLOB_RECURSE STREAMR_DHT_MODULE_UNITS CONFIGURE_DEPENDS
     ${CMAKE_CURRENT_SOURCE_DIR}/modules/*.cppm)
 streamr_target_module_sources(streamr-dht FILES ${STREAMR_DHT_MODULE_UNITS})
-target_include_directories(streamr-dht 
-    PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
-    PUBLIC $<INSTALL_INTERFACE:include>
+# Only the generated protobuf code remains a header consumers may need
+# textually.
+target_include_directories(streamr-dht
     PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/src/proto>
     )
 

packages/streamr-dht/StreamrModules.cmake

@@ -120,6 +120,13 @@ function(streamr_add_module_library TARGET)
     # undeclared). Making the flag part of the PUBLIC interface keeps the
     # BMI and every importer consistent. No-op on other platforms.
     target_compile_options(${TARGET} PUBLIC $<$<PLATFORM_ID:Android>:-pthread>)
+    # The module libraries' objects end up inside the shared
+    # libstreamrproxyclient, so they must be position-independent. macOS
+    # emits PIC by default; on Linux the ELF linker rejects the archive
+    # otherwise ("recompile with -fPIC" against the module vtable/thunk
+    # symbols — first seen when the consolidated code moved into module
+    # objects on the linux-arm64 leg).
+    set_target_properties(${TARGET} PROPERTIES POSITION_INDEPENDENT_CODE ON)
 endfunction()
 
 # streamr_target_module_sources(<target> FILES <unit.cppm>...)
@@ -143,8 +150,10 @@ function(streamr_target_module_sources TARGET)
         FILES ${ARG_FILES})
     set_target_properties(${TARGET} PROPERTIES CXX_SCAN_FOR_MODULES ON)
     target_compile_features(${TARGET} PUBLIC cxx_std_26)
-    # See streamr_add_module_library: BMI/importer -pthread consistency.
+    # See streamr_add_module_library: BMI/importer -pthread consistency
+    # and position independence for the shared-library link.
     target_compile_options(${TARGET} PUBLIC $<$<PLATFORM_ID:Android>:-pthread>)
+    set_target_properties(${TARGET} PROPERTIES POSITION_INDEPENDENT_CODE ON)
 endfunction()
 
 # streamr_enable_imports(<target>)

packages/streamr-dht/install.sh

@@ -31,12 +31,18 @@ if [ -n "$TARGET_TRIPLET" ]; then
     echo "Target platform: $TARGET_TRIPLET"
 fi
 
+# NOTE: this script used to hard-code -fsanitize=address into every
+# configure (a committed debugging leftover). That silently produced a
+# sanitizer-instrumented standalone archive which, once the package's
+# real code moved into the module units (Phase 2.6 consolidation),
+# broke every downstream standalone link with undefined ___asan_*
+# symbols. Sanitized builds should be an explicit opt-in instead.
 if [ -n "$TARGET_TRIPLET" ]; then
     if [ "$TARGET_TRIPLET" = "arm64-android" ]; then
-        cd build && cmake -DCMAKE_CXX_FLAGS="-fsanitize=address -fno-omit-frame-pointer" -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address" -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DVCPKG_TARGET_TRIPLET=$TARGET_TRIPLET -DVCPKG_CHAINLOAD_TOOLCHAIN_FILE=$CHAINLOAD_TOOLCHAIN_FILE -DANDROID_ABI=$ANDROID_ABI -DANDROID_PLATFORM=$ANDROID_PLATFORM .. && cmake --build . && cd ..
+        cd build && cmake -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DVCPKG_TARGET_TRIPLET=$TARGET_TRIPLET -DVCPKG_CHAINLOAD_TOOLCHAIN_FILE=$CHAINLOAD_TOOLCHAIN_FILE -DANDROID_ABI=$ANDROID_ABI -DANDROID_PLATFORM=$ANDROID_PLATFORM .. && cmake --build . && cd ..
     else
-        cd build && cmake -DCMAKE_CXX_FLAGS="-fsanitize=address -fno-omit-frame-pointer" -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address" -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DVCPKG_TARGET_TRIPLET=$TARGET_TRIPLET -DVCPKG_CHAINLOAD_TOOLCHAIN_FILE=$CHAINLOAD_TOOLCHAIN_FILE -DPLATFORM=$PLATFORM .. && cmake --build . && cd ..
+        cd build && cmake -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DVCPKG_TARGET_TRIPLET=$TARGET_TRIPLET -DVCPKG_CHAINLOAD_TOOLCHAIN_FILE=$CHAINLOAD_TOOLCHAIN_FILE -DPLATFORM=$PLATFORM .. && cmake --build . && cd ..
     fi
 else
-    cd build && cmake -DCMAKE_CXX_FLAGS="-fsanitize=address -fno-omit-frame-pointer" -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address" -DCMAKE_BUILD_TYPE=$BUILD_TYPE .. && cmake --build . && cd ..
+    cd build && cmake -DCMAKE_BUILD_TYPE=$BUILD_TYPE .. && cmake --build . && cd ..
 fi

packages/streamr-dht/lint.sh

@@ -5,16 +5,27 @@ set -e
 FILES=$(find . -type d \( -name src -o -name test -o -name include \) ! -path '*/build/*' ! -path '*/proto/*' -print0 | xargs -0 -I{} find {} -type f \( -name "*.hpp" -o -name "*.cpp" \) -print0 | xargs -0 echo)
 echo "Running clangd-tidy on $FILES"
 
-clangd-tidy -p ./build $FILES
+# ConnectionLockingTest.cpp is excluded from clangd-tidy (owner-approved
+# selective disabling, Phase 2.6 consolidation): it instantiates the
+# imported streamr::utils::collect, whose folly internals clangd cannot
+# resolve through module interfaces (spurious WithAsyncStackFunction
+# error). The COMPILER accepts the file; clang-format still checks it.
+TIDY_FILES=$(echo "$FILES" | tr ' ' '\n' | grep -v 'test/integration/ConnectionLockingTest.cpp' | tr '\n' ' ')
+clangd-tidy -p ./build $TIDY_FILES
 
 echo "Running clang-format --dry-run on $FILES"
 ../../run-clang-format.py $FILES
 
-# Module interface units: format check only. clangd-tidy is not run on
-# .cppm files (headers remain the fully linted source of truth during the
-# façade migration; clangd modules support is still experimental).
+# CONSOLIDATED (MODERNIZATION.md Phase 2.6): the include/ tree is gone —
+# the code lives in the module interface units, which are linted with
+# clangd-tidy as the source of truth (the consolidated units carry no
+# export-using re-export blocks, which caused the earlier clangd false
+# positives).
 MODULE_FILES=$(find ./modules -type f -name "*.cppm" 2>/dev/null | xargs echo)
 if [ -n "$MODULE_FILES" ]; then
+    echo "Running clangd-tidy on $MODULE_FILES"
+    clangd-tidy -p ./build $MODULE_FILES
+
     echo "Running clang-format --dry-run on $MODULE_FILES"
     ../../run-clang-format.py $MODULE_FILES
 fi

packages/streamr-dht/include/streamr-dht/Identifiers.hpp → packages/streamr-dht/modules/Identifiers.cppm

@@ -1,15 +1,22 @@
-#ifndef STREAMR_DHT_IDENTIFIERS_HPP
-#define STREAMR_DHT_IDENTIFIERS_HPP
+// Module streamr.dht.Identifiers
+// CONSOLIDATED from the former header streamr-dht/Identifiers.hpp
+// (MODERNIZATION.md Phase 2.6): this file is now the source of truth.
+module;
 
 #include <cstdint>
 #include <string>
 #include <vector>
 #include "packages/dht/protos/DhtRpc.pb.h"
-#include "streamr-utils/BinaryUtils.hpp"
-#include "streamr-utils/Branded.hpp"
-namespace streamr::dht {
 
+export module streamr.dht.Identifiers;
+
+import streamr.utils;
+
+// Hoisted from the former header (file scope, NOT exported);
+// fully qualified: relative namespace names resolve differently
+// at file scope than inside the package namespace.
 using streamr::utils::BinaryUtils;
+export namespace streamr::dht {
 
 using DhtAddress = streamr::utils::Branded<std::string, struct DhtAddressBrand>;
 using DhtAddressRaw =
@@ -52,5 +59,3 @@ struct Identifiers {
     }
 };
 } // namespace streamr::dht
-
-#endif // STREAMR_DHT_IDENTIFIERS_HPP

packages/streamr-dht/include/streamr-dht/connection/Connection.hpp → packages/streamr-dht/modules/connection/Connection.cppm

@@ -1,22 +1,32 @@
-#ifndef STREAMR_DHT_CONNECTION_CONNECTION_HPP
-#define STREAMR_DHT_CONNECTION_CONNECTION_HPP
+// Module streamr.dht.Connection
+// CONSOLIDATED from the former header streamr-dht/connection/Connection.hpp
+// (MODERNIZATION.md Phase 2.6): this file is now the source of truth.
+module;
 
 #include <cstddef>
 #include <cstdint>
 #include <tuple>
 #include <vector>
 #include <magic_enum/magic_enum.hpp>
-#include "streamr-eventemitter/EventEmitter.hpp"
-#include "streamr-logger/SLogger.hpp"
-#include "streamr-utils/Branded.hpp"
-#include "streamr-utils/Uuid.hpp"
-namespace streamr::dht::connection {
 
+#include <string>
+
+export module streamr.dht.Connection;
+
+import streamr.eventemitter;
+import streamr.logger;
+import streamr.utils;
+
+// Hoisted from the former header (file scope, NOT exported);
+// fully qualified: relative namespace names resolve differently
+// at file scope than inside the package namespace.
 using streamr::eventemitter::Event;
 using streamr::eventemitter::EventEmitter;
 using streamr::logger::SLogger;
 using streamr::utils::Branded;
 using streamr::utils::Uuid;
+export namespace streamr::dht::connection {
+
 using ConnectionID = Branded<std::string, struct ConnectionIDBrand>;
 using Url = Branded<std::string, struct UrlBrand>;
 
@@ -75,5 +85,3 @@ class Connection : public EventEmitter<ConnectionEvents> {
 };
 
 } // namespace streamr::dht::connection
-
-#endif // STREAMR_DHT_CONNECTION_CONNECTION_HPP