fix(build): //base belongs in public_deps where it is in the public interface #57
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/base-public-deps"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
A build-graph defect that only a unit test could see — found by the build I fired to prove #56's tests pass.
What happened
That build didn't get to running tests. It failed to compile one:
:cb_wire_envelopelisted//basein privatedeps. Butcb_wire_envelope.hincludesbase/values.hin its public interface (ProbeResultPayloadholds abase::DictValue), so anything including that header needs//base's include dirs transitively — and//baseis what pulls in partition_alloc's generated headers.The source_set itself compiled fine; only dependents broke. With no dependent building, nothing could observe it. That's gn's
depsvspublic_depsdistinction exactly: a dep whose headers appear in your own public headers is part of your interface.It was a class, not an instance
I scanned every
source_setincapture/for "public header includesbase/AND//baseis a private dep". Nineteen targets, including:Note
pointer_stateandinput_dispatch. Their test binaries —cloud_browser_pointer_state_unittests,cloud_browser_input_dispatch_unittests— are two of the five that have never been built in any lane. This is very likely why: whoever last tried hit the same wall and dropped the target rather than the dep. I can't prove that from here (no record of the attempt), but the shape matches.Nothing was broken in production: the worker links all of these transitively either way. The defect is only visible to a target that depends on one of them in isolation — i.e. a unit test with narrow deps. That's why it survived while the tests stayed unbuilt.
Verified
t7 build on this branch, with
cb_wire_envelope_unittestsadded to the targets:It compiles and links where it previously died.
One caveat I want to be explicit about: STEP 7 in that build still only invoked the two hardcoded binaries, so the wire-envelope assertions have still not executed. That's not this PR's job — #56 is what teaches STEP 7 to derive its list. The two changes are complementary and both are needed: this one makes the test compile, #56 makes it run. Neither alone closes the gap.
The build I fired to prove cb_wire_envelope_unittests passes did not get that far — it failed to COMPILE the test, with: FAILED: cb_wire_envelope_test.o ../../base/memory/raw_ptr_exclusion.h:11:10: fatal error: 'partition_alloc/pointers/raw_ptr_exclusion.h' file not found Root cause: :cb_wire_envelope listed //base in private `deps`. But cb_wire_envelope.h includes "base/values.h" in its PUBLIC interface (ProbeResultPayload holds a base::DictValue), so any target that includes that header needs //base's include dirs transitively — and //base is what pulls in partition_alloc's generated headers. The source_set itself compiled fine (it sees //base directly); only DEPENDENTS broke. With no dependent building, nothing could observe it. That is gn's deps-vs-public_deps distinction exactly: a dep whose headers appear in your own public headers is part of your interface, not an implementation detail. Then the obvious question — is this one target or a class? Scanned every source_set in capture/ for "public header includes base/ AND //base is a private dep". NINETEEN targets, including: pointer_state, input_dispatch_ime, input_dispatch_touch, input_dispatch_drag, input_dispatch_clipboard, cursor_client, cursor_xy_join, embedder, framesink_capture, stats_relay, clipboard_relay, file_upload_relay, active_webcontents_resolver, cb_audio_lifecycle, cb_signaling_ws_client, cb_offerer_driver, cb_dc_host, cb_signaling_reconnect, cb_wire_envelope Note which names appear there: pointer_state and input_dispatch. Their test binaries — cloud_browser_pointer_state_unittests, cloud_browser_input_dispatch_unittests — are two of the five that have never been built in any lane. This is very likely why: whoever last tried to add them hit this same wall and dropped the target rather than the dep. I cannot prove that from here (no record of the attempt), but the shape matches. All nineteen moved to public_deps. The worker binary links them all transitively either way, which is why nothing was broken in production — the defect is only visible to a target that depends on ONE of them in isolation, i.e. a unit test. That is the whole point of unit tests having narrow deps, and it is why this stayed hidden while the tests stayed unbuilt. Verified locally: brace/bracket balance on both BUILD.gn files, no target left with //base in both deps and public_deps, re-scan reports zero remaining affected targets, make verify + cxx-include-lint clean. NOT verified: that it compiles. A t7 build follows — that is the point. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>