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