Skip to content

Commit 390f0f3

Browse files
aiutoclaude
andcommitted
[ABLD-464] Add dbg_symbol.md design record
Preserves the ABLD-464 context and design rationale (both candidate strategies, platform semantics, and the macOS codesign-ordering issue found while implementing this PR) alongside the code, since the original planning doc lives only on the local machine that planned it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent df86c35 commit 390f0f3

1 file changed

Lines changed: 114 additions & 0 deletions

File tree

dbg_symbol.md

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
# ABLD-464: Debug-symbol ("dbg") packages for Bazel builds
2+
3+
Context and design record for [ABLD-464](https://datadoghq.atlassian.net/browse/ABLD-464)
4+
("Process for creating 'dbg' packages"). Kept alongside the code so the rationale
5+
survives past the PR description.
6+
7+
## Problem
8+
9+
Omnibus produces stripped release binaries plus a separate "-dbg" package/archive
10+
containing the removed debug symbols, for post-mortem debugging of crash dumps from
11+
stripped production binaries. Bazel packaging (`packages/`, built on `rules_pkg`) has
12+
no equivalent yet — `packages/AGENTS.md` and `packages/installer/MIGRATION_PLAN.md`
13+
both flag this as an open gap.
14+
15+
Requirements from the ticket:
16+
- A normal `bazel build` of a target keeps debug symbols (dev/test use).
17+
- A packaging build produces stripped binaries.
18+
- A sibling packaging target produces *only* the debug symbols, no other files.
19+
- No shadow dependency tree duplicated top-down to every binary/library target.
20+
- Works for Go, C, and Rust objects; works on Linux, macOS, Windows.
21+
- Callable from `rules_pkg` `pkg_files` construction.
22+
23+
## Two strategies, two PRs
24+
25+
The ticket names two candidate strategies. Rather than pick one up front, we
26+
implemented both as independent PRs in separate worktrees of this repo, so they can be
27+
compared in review and in CI before committing to one for the real migration:
28+
29+
- **Plan A — provider/rule-based** (this repo, branch `aiuto/454-a`): a new
30+
`dd_strip_debug` rule wraps each binary/library's own build rule directly, adding a
31+
`DdStripInfo` provider and `OutputGroupInfo(stripped=..., debug=...)`. No aspect is
32+
needed for the core case — the default output stays unstripped, and a `pkg_files`
33+
transform just checks whether its `srcs[i]` carries `DdStripInfo`.
34+
See `bazel/rules/dd_strip/`, `bazel/toolchains/dd_strip/`,
35+
`bazel/rules/dd_packaging/dd_pkg_files_stripped.bzl`.
36+
- **Plan B — packaging-time filter** (`/Users/tony.aiuto/ws/datadog-agent`, branch
37+
`aiuto/454-b`): a new `dd_pkg_strip_transform` rule intercepts files as they're
38+
assembled into a `pkg_files`/`PackageFilesInfo` tree and strips/splits them there,
39+
with no cooperation needed from the binary's own rule. This is what lets it work
40+
directly against today's `prebuilt_file.bzl`-backed product binaries
41+
(`@agent_binary//:agent`, etc.), which aren't real Bazel `go_binary`/`cc_binary`
42+
targets yet — Plan A can't wrap a provider around those until that migration lands.
43+
44+
Both use the same platform semantics, matching omnibus's `Stripper`
45+
(`omnibus-ruby/lib/omnibus/stripper.rb`):
46+
- **Linux**: `objcopy --only-keep-debug``.dbg`, `strip --strip-debug
47+
--strip-unneeded` in place, `objcopy --add-gnu-debuglink`.
48+
- **macOS**: `strip` the shipped copy, `dsymutil` produces a `.dSYM` bundle. No prior
49+
art for this in omnibus or this repo (omnibus skips stripping on macOS entirely) —
50+
flagged as needing build-team confirmation before either PR merges.
51+
- **Windows**: debug artifact = the *unstripped original* binary (not split DWARF),
52+
matching omnibus's `windows_symbol_stripping_file` semantics, since this repo's
53+
Go/Rust/mingw toolchains don't reliably produce standalone PDBs.
54+
55+
Both PRs independently build their own copy of the `bazel/toolchains/dd_strip`
56+
toolchain rather than one depending on the other (explicit choice, made so both could
57+
start immediately) — deduping them is a deliberate follow-up once one strategy is
58+
chosen.
59+
60+
Full original design doc (file lists, verification steps, trade-off analysis):
61+
`/Users/tony.aiuto/.claude/plans/we-are-going-to-flickering-pizza.md` (local to the
62+
machine this was planned on, not checked in — this file is the durable summary).
63+
64+
## Known issue: macOS codesign ordering
65+
66+
`dd_cc_packaged` (`bazel/rules/dd_packaging/dd_cc_packaged.bzl`) runs `dd_strip_debug`
67+
**after** `rewrite_rpath`. `rewrite_rpath`'s macOS implementation
68+
(`bazel/toolchains/rpath_rewriter/rewrite_with_install_name_tool.sh`) ends with:
69+
70+
```sh
71+
# Re-sign with an ad-hoc signature after modification as install_name_tool invalidates
72+
# any existing code signature.
73+
/usr/bin/codesign --sign - --force "$OUTPUT"
74+
```
75+
76+
`install_name_tool` invalidates any existing signature, so `rewrite_rpath` re-signs
77+
ad-hoc as its last step. `dd_strip_debug` then runs `strip`/`objcopy`/`dsymutil` on
78+
that *already re-signed* output — and stripping/objcopy also invalidates a Mach-O code
79+
signature, but **`dd_strip_debug` does not re-sign afterward**. The result: on macOS,
80+
the final `.stripped` binary that actually ships out of `dd_cc_packaged` carries a
81+
stale/invalid ad-hoc signature.
82+
83+
This was chosen deliberately to match omnibus's ordering ("strip is the last finalize
84+
step"), but omnibus's ordering rationale is Linux-centric (objcopy debuglink chains)
85+
and doesn't account for macOS's codesign-after-every-mutation requirement.
86+
87+
Two ways to fix, not yet decided:
88+
1. Add a `codesign --sign - --force` re-sign step to `dd_strip_debug`'s macOS driver,
89+
after the strip step, so it always leaves a valid ad-hoc signature — keeps
90+
omnibus's "strip last" ordering.
91+
2. Reorder so `dd_strip_debug` runs *before* `rewrite_rpath` on macOS specifically,
92+
since `rewrite_rpath` already re-signs as its last step — avoids adding a second
93+
codesign invocation, but only applies to the `dd_cc_packaged` call chain (Plan A);
94+
for Plan B's flattened packaging-time model there's no equivalent single choke
95+
point to reorder around, so option 1 (re-sign after strip) is likely the more
96+
portable fix across both PRs.
97+
98+
This affects Plan A directly (found during its implementation/testing). Plan B should
99+
be checked for the same issue if/when it starts producing real signed macOS binaries
100+
through `dd_cc_packaged`-style consumers.
101+
102+
## Other open items (both PRs)
103+
104+
- Linux `objcopy` path is unverified locally — both PRs were implemented and tested on
105+
macOS arm64 with no Linux exec platform available; needs CI or a Linux sandbox.
106+
- `packages/agent/product/BUILD.bazel`'s real binaries come from `prebuilt_file.bzl`
107+
(built by `dda`, not a real Bazel target) — Plan A can only demonstrate against
108+
`//cmd/agent:agent` and `rtloader` directly, not the full product bundle, until that
109+
migration lands. Plan B has no such blocker.
110+
- Plan A: `dsymutil` produced an empty `.dSYM` for at least one pure-Go binary during
111+
testing — needs investigation (Go's DWARF layout vs. dsymutil's expectations).
112+
- Whatever builds release binaries outside Bazel (`dda inv agent.build` / omnibus)
113+
must not pre-strip them, or there's nothing left for either strategy to split.
114+
Out of scope for both PRs; flagged as a cross-cutting dependency.

0 commit comments

Comments
 (0)