Skip to content

build: separate build dirs for sanitizer and coverage builds#23

Merged
RedFox20 merged 4 commits into
masterfrom
feat/sanitizer-build-dirs
Jun 1, 2026
Merged

build: separate build dirs for sanitizer and coverage builds#23
RedFox20 merged 4 commits into
masterfrom
feat/sanitizer-build-dirs

Conversation

@battlesnake
Copy link
Copy Markdown
Collaborator

Problem

All sanitizer flavors write to the same platform build dir (packages/<pkg>/linux, windows, …). self.sanitize isn't part of the build path, so switching between asan / tsan / ubsan / none reuses one dir with a different sanitizer than it was configured for. run_config() then sees the sanitizer marker changed and forces a full CMake reconfigure every switch — slow, and occasionally a stale-mix build failure. Coverage (mama coverage) had the exact same problem: it landed in plain linux and collided with both plain and sanitizer builds.

Fix

Suffix the build dir per flavor via one build_dir_suffix() helper, reusing the existing CLI short tokens for sanitizers and adding a -coverage marker that composes cleanly in front of any sanitizer suffix:

flavor dir
(none) linux (unchanged)
coverage linux-coverage
address linux-asan
thread linux-tsan
undefined linux-ubsan
leak linux-lsan
coverage + address linux-coverage-asan

(and the same for every platform: windows-asan, android-tsan, linux-coverage, …)

The helper is applied in the two places that pick the per-platform name: platform_build_dir_name() and dependency_chain._save_mama_cmake's get_build_dir_defines (kept in sync per the existing WARNING comment, so dependency mama.cmake include paths match the suffixed dep dirs). No coverage and no sanitizer ⇒ no suffix ⇒ existing caches stay valid. Each flavor now keeps its own dir + CMakeCache.txt, so switching between already-built flavors no longer reconfigures.

Coverage

mama coverage build now lands in linux-coverage instead of sharing linux. The marker is prepended to the sanitizer suffix, so mama coverage asan buildlinux-coverage-asan, keeping the previously-reviewed sanitizer dir names byte-identical (linux-asan, etc.). Plain mama build is unchanged (linux), so existing non-coverage caches stay valid.

Verification

tests/test_build_dir/ asserts the per-sanitizer names, the new coverage-only (linux-coverage) and coverage+sanitizer (linux-coverage-asan) names, and that no-coverage/no-sanitizer is unchanged. Full existing suite is green — 87 tests (prior 85 + 2 new coverage cases).

Ran the patched mama against a throwaway CMake project: mama coverage build, mama asan build, and mama coverage asan build create separate linux-coverage / linux-asan / linux-coverage-asan dirs; the MAMA_BUILD define in the generated mama.cmake matches the on-disk dir for each; re-running each already-built flavor skips CMake configure (no churn); plain build still uses linux.

Back-compat note

First build of each flavor after upgrading mama configures once into its new suffixed dir (the old shared linux dir keeps the plain no-coverage/no-sanitizer build). Expected, one-time.

Supersedes #22 (moved off fork to a branch on this repo).

LOC breakdown

prod tests
sanitizer dirs (8f07b02) 18 33
coverage dirs (this change) 3 13
branch total vs master 18 46

Mark Kuckian and others added 2 commits May 29, 2026 08:22
All sanitizer flavors shared one platform build dir (eg 'linux'), so
switching asan<->tsan<->none forced a full CMake reconfigure (and the
occasional stale-mix build failure). Suffix the build dir per sanitizer
('linux-asan', 'linux-tsan', 'linux-ubsan', 'linux-lsan') in the two
funnels that pick it. No sanitizer keeps the old name, so existing
caches stay valid; first build of each flavor after upgrade configures
once into its new dir.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Coverage builds shared a build dir with plain (and sanitizer) builds,
forcing a full reconfigure whenever you switched flavors. Extend
build_dir_suffix() to prepend a '-coverage' marker, composing with the
existing sanitizer suffix: plain -> linux, coverage -> linux-coverage,
asan -> linux-asan (unchanged), coverage+asan -> linux-coverage-asan.
Both consumers (platform_build_dir_name and dependency_chain's
MAMA_BUILD define) flow through the same helper, so dep include paths
stay consistent.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread mama/build_config.py Outdated
Coverage builds add '-coverage' and sanitizer builds add a further
suffix, eg 'linux-coverage', 'linux-asan' or 'linux-coverage-asan'.
"""
# WARNING: This needs to be in sync with dependency_chain.py: _save_mama_cmake !!!
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment got spliced

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, a clanker got spliced for that mistake

The comment about sync with dependency_chain.py was accidentally left on
the public wrapper after the implementation was split into the private
helper. Move it to the method whose body actually needs to stay in sync.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread mama/dependency_chain.py Outdated
# gets the defines for a single platform and architecture
# must match platform_build_dir_name(): same sanitizer suffix as the actual build dir
def get_build_dir_defines(build_dir):
build_dir += c.build_dir_suffix()
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fishy and doesn't seem to belong here, the whole suffix definition should be done inside BuildConfig (single source of truth). This would also remove the unnecessary comment must match platform_build_dir_name() -- because in reality even Claude will not often read this properly.

build_dir = c.build_dir_with_suffix(build_dir)

This would delegate the single source of truth into BuildConfig class, and BuildConfig.platform_build_dir_name() can also use it

@battlesnake
Copy link
Copy Markdown
Collaborator Author

Concrete motivation to land this: it would've caught KrattWorks/rtpvideo#4 — an unaligned *reinterpret_cast<const uint16_t*>(ptr) read in BitStream that mama ubsan build silently ran without -fsanitize (it reused the non-sanitized linux dir, exactly the stale-mix described above). With per-flavor build dirs mama ubsan actually applies the flag, and that read then trips the alignment check.

…ith_suffix()

Replace the split build_dir_suffix() + manual += in dependency_chain.py
with a build_dir_with_suffix(build_dir) method on BuildConfig. Both
platform_build_dir_name() and get_build_dir_defines() now call the same
method, removing the "must match" guard comment and the WARNING.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@RedFox20 RedFox20 merged commit 0020147 into master Jun 1, 2026
1 check passed
@RedFox20 RedFox20 deleted the feat/sanitizer-build-dirs branch June 1, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants