feat(tgb): add AllTargetsFileHashes column 22 support - #285
Merged
yushan8 merged 6 commits intoAug 20, 2026
Conversation
yushan8
force-pushed
the
yushan/all-targets-config-v2
branch
from
August 19, 2026 16:49
dc9447b to
45cd0c3
Compare
5 tasks
yushan8
changed the title
yushan8
marked this pull request as ready for review
August 19, 2026 21:55
sywhang
approved these changes
Aug 20, 2026
Add TGB support for the AllTargetsFileHashes sidecar so the AllTargetsFiles feature works regardless of graph_format. Column 22 stores a length-prefixed sequence of (key, value) string pairs, zstd-compressed. Old readers skip unknown column IDs; new readers on old blobs get nil — no format version bump needed. The TGB comparison path in the controller now checks AllTargetsFileHashes via a cheap single-column decode before the full columnar diff, matching the gob path's behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Verify that AllTargetsFileHashes survives TGB encode/decode via both full Decode and the lazy Reader.AllTargetsFileHashes() accessor. Covers present, absent, and empty-map cases. Also fixes canonicalise() to preserve AllTargetsFileHashes so existing round-trip tests remain correct when graphs carry the sidecar. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
MergeChunks was not merging AllTargetsFileHashes from metadata chunks, so the field was silently dropped before TGB encoding. This caused the AllTargetsFiles trigger to never fire on the TGB comparison path. Also adds controller-level TGB tests for both trigger and no-trigger cases, verifying end-to-end that AllTargetsFileHashes survives the full write → read → compare pipeline. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Map iteration order is random in Go, so the same logical AllTargetsFileHashes could produce different byte sequences across runs, violating the deterministic computation property and causing spurious cache misses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- gofmt alignment in colNames map and a test's Params struct literal - DecodeGraph now reuses Reader.AllTargetsFileHashes() instead of duplicating the column-decode logic inline
Makes it clearer that all targets in the graph are reported as changed when the trigger fires, not just the ones that happen to have different hashes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
yushan8
force-pushed
the
yushan/all-targets-config-v2
branch
from
August 20, 2026 03:57
681fe6f to
c51d4b7
Compare
yushan8
added a commit
that referenced
this pull request
Aug 20, 2026
…r file change (#286) ## Summary - Adds `RepositoryConfig.AllTargetsFiles`: a list of repo-relative file paths that, when their content hash differs between two revisions, cause `GetChangedTargets` to report every target in the second revision as changed (distance 0) - File hashes are computed from `git ls-tree` during graph computation in `targethasher.FromProto`, stored as sidecar data in `Metadata.AllTargetsFileHashes`, and compared at diff time in the controller - Intended for files that affect the build globally but are invisible to the target graph (toolchain definitions, CI config, Bazel wrapper scripts) - Gob-format graphs carry this field automatically via gob serialization ### Data flow ``` git ls-tree → HashConfig.KnownSourceHashes → targethasher.FromProto filters by AllTargetsFiles → Result.AllTargetsFileHashes → mapper → entity.Metadata.AllTargetsFileHashes → stored in graph blob (gob auto-serialization) → read back at comparison time in controller → if any hash differs → all targets reported as changed ``` ### Example config ```yaml repository: - remote: "git@github.com:org/repo.git" all_targets_files: - .bazelrc - .bazelversion - tools/bazel - rules/go/sdk.bzl ``` ## Test plan - [x] `allTargetsFileChanged` unit tests — nil/empty/matching/differing hash scenarios (table-driven) - [x] `compareTargetGraphs` gob-path test — trigger fires → all targets changed - [x] `compareTargetGraphs` gob-path test — matching hashes → normal diff - [x] Mapper propagation tests — Result → Metadata, including via ResultToGraphChunks - [x] All existing tests pass **Stack:** TGB format support in PR #285 --------- Co-authored-by: Claude Sonnet 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
2 participants
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.
Summary
Adds TGB (Target Graph Binary) format support for
AllTargetsFileHashes, stacked on #286.ALL_TARGETS_FILE_HASHES) — length-prefixed key-value string pairs, zstd compressedReader.AllTargetsFileHashes()lazy accessor decodes only column 22 (cheap, no full graph decode)MergeChunkspropagates AllTargetsFileHashes from metadata chunks before TGB encodingFiles changed
internal/tgb/format.go— column ID 22internal/tgb/encode.go—encodeStringMap+ column emissioninternal/tgb/reader.go—decodeStringMap+AllTargetsFileHashes()accessor +DecodeGraphintegrationinternal/tgbdiff/adapt.go—MergeChunkspropagation fixcontroller/getchangedtargets.go—allTargetsChangedFromTGB(), TGB path wiringcontroller/getchangedtargets_tgb_test.go— trigger/no-trigger testsinternal/tgb/tgb_test.go— round-trip testsTest plan
Stack: depends on #286