Consolidation step 2 of 8: trackerless-network code moves into the module [androidbuild] [iosbuild]#42
Merged
Conversation
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
ptesavol
force-pushed
the
consolidate/c2-trackerless-network
branch
from
fde4ad3 to
3a71303
Compare
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Second step of the module consolidation: every former public header becomes its OWN module partition file, keeping the old directory tree and short file names - include/streamr-trackerless-network/logic/X.hpp becomes modules/logic/X.cppm (git detects the renames, preserving file history). The partition files are the source of truth; the include/ tree is deleted. Each partition contains `export module streamr.trackerlessnetwork:X;` with: - only that header's third-party/std/generated includes in its global module fragment, - `import :Dep;` for the old intra-package include edges, - `import streamr.<pkg>;` for sibling streamr packages, - the header's convenience using-declarations hoisted to non-exported file scope (fully qualified - relative namespace names resolve differently at file scope), and - the code in `export namespace` blocks (export-using of module-linkage entities is ill-formed, verified empirically). The transition-stage :all partition is deleted; the primary interface unit re-exports every partition. Gotcha (hit twice): a partition must textually include what its own code uses. The former headers received <string>'s operator+ and folly::coro::blockingWait transitively from neighboring headers; entities reached only through an imported module's global module fragment are not reliably reachable. Also: ProxyClientTsIntegrationTest.cpp (the last file textually including a package header) now imports, with module scanning and the direct links imports require; lint.sh runs clangd-tidy on all module units (full analysis coverage, zero suppressions - the consolidated units have no export-using re-export blocks, which caused the earlier clangd false positives). Verified: full Release build; tn tests 12 unit + 1 integration; proxy client (importer) 15/15; standalone builds of both packages; package lint green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ptesavol
force-pushed
the
consolidate/c2-trackerless-network
branch
from
3a71303 to
4588f94
Compare
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
github-actions
Bot
added
the
ci
ptesavol
added a commit
that referenced
this pull request
Jul 4, 2026
… events to late listeners (#43) On the outgoing side the connector starts socket I/O inside createConnection(), but the endpoint's listeners on the returned PendingConnection are only registered afterwards (addEndpoint -> changeToConnectingState). When the main thread is descheduled in that window for longer than one localhost websocket + streamr handshake round-trip - realistic only on a loaded 2-core CI runner - onHandshakeCompleted() runs first: it disarms the 15 s connect watchdog and then emits Connected on a fire-and-forget emitter with zero listeners. The emission is lost, no timeout remains armed, and the endpoint sits in the connecting state forever with the lock-request RPC buffered (the ConnectionLockingTest.CanLockConnections stall on ubuntu-latest, PR #42 CI). IPendingConnection now extends ReplayEventEmitter, so a late listener receives the latest Connected/Disconnected instead of nothing. To make that airtight across threads, emit() now performs the latest-event store and the handler snapshot atomically with respect to on() (both under mEventHandlersMutex): a listener registered concurrently with an emit gets exactly one of live dispatch or replay, never neither, never both. This also closes the pre-existing unsynchronized read of mEventHandlers in createExecutingEmitLoopHandlersMap(). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second step of the module consolidation, restructured per your direction: every former header file becomes its own module partition file — one to one, never one merged unit.
The structure
Each of the thirteen public headers of the trackerless-network package is now its own file
modules/streamr.trackerlessnetwork-<Name>.cppm, containing a module partition named after it. Each partition carries:import :<Name>;for every dependency it previously expressed as an include of a sibling header (the old include graph becomes the partition import graph — this is why the earlier dependency-cycle work mattered);import streamr.<package>;for each sibling streamr package it uses;The old merged transition file is gone, the primary interface unit re-exports every partition, and the header directory is deleted — the partition files are the source of truth.
A gotcha worth knowing for the remaining packages (found twice here)
A partition must textually include what its own code uses. Two headers had relied on includes inherited transitively from neighboring headers (
<string>'s concatenation operator, folly'sblockingWait); with imports, entities reached only through another module's preamble are not reliably visible. The compiler catches these immediately and the fix is adding the honest include — an improvement in include hygiene, incidentally.The linting outcome — better than before
The full code-analysis step now runs on all fifteen module files of this package. The false positives we feared came from the re-export blocks that consolidation eliminates, so coverage is complete with zero suppressions — the selective-disabling permission has not been needed.
Verification
🤖 Generated with Claude Code