Build the C ABI from the root and in CI; drop the committed __pycache__ - #8
Merged
Merged
Conversation
Three small hygiene fixes found in a cross-repo consistency sweep.
**The C ABI was never compiled by anything automated.** tools/capi builds
standalone (`cmake -B build_capi -S tools/capi`), which is what the ctypes
bridge does on first import, but the root CMakeLists had no way to build it and
no CI job touched it. So the verification layer -- the thing the notebooks drive
the shipping C++ through, and therefore the thing every measured claim rests on
-- could break and nobody would find out until someone re-ran a notebook by
hand. Adds a TAP_DSP_BUILD_CAPI option, matching the BUILD_CAPI option every
sibling library already carries, and turns it ON in the build-test matrix so the
ABI is compiled on Linux, Windows and macOS on every push. The standalone path
is untouched: tools/capi still declares its own project and pulls the repo root
in when tap::dsp does not already exist.
This is safe on all three platforms: dsptap_capi.h already selects
__declspec(dllexport) on Windows and visibility("default") elsewhere, and the
capi target does not link tap_dsp_warnings, so TAP_DSP_WERROR does not apply to
it.
**notebooks/__pycache__/dsptap_py.cpython-311.pyc was committed** -- a build
artifact of the ctypes bridge, and the only such file in the family. Removed,
and __pycache__/ added to .gitignore, which did not cover it.
Verified in exactly the configuration CI will now run
(-DTAP_DSP_WERROR=ON -DTAP_DSP_BUILD_CAPI=ON, Release): configure and build
clean with zero errors, libdsptap_capi.so produced, ctest 99/99 passing.
Worth recording: this CI gap is family-wide, not specific to this repo. All six
library repos default their BUILD_CAPI option OFF and no workflow in any of them
enables it, so none of their C ABIs is compiled by CI either. This commit closes
it here; the same one-flag change would close it in AmbiTap, MuTap,
SampleRateTap, RatioTap and TapTools.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JhhQ93r2E1QTnCx46YfX8j
This was referenced Jul 28, 2026
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.
What this changes
TAP_DSP_BUILD_CAPIoption to the rootCMakeLists.txt, matching theBUILD_CAPIoption every sibling library already carriesbuild-testCI matrix, so the C ABI is compiled on Linux, Windows and macOS on every pushnotebooks/__pycache__/dsptap_py.cpython-311.pycand adds__pycache__/to.gitignoreWhy
The C ABI was never compiled by anything automated.
tools/capibuilds standalone (cmake -B build_capi -S tools/capi), which is what the ctypes bridge does on first import — but the root build had no way to compile it and no CI job touched it. So the verification layer, the thing notebooks drive the shipping C++ through and therefore the thing every measured claim rests on, could break silently until someone re-ran a notebook by hand.The committed
.pycwas a build artifact of the bridge and the only such file in the family;.gitignoredidn't cover it.Verification
Built in exactly the configuration CI will now run —
-DCMAKE_BUILD_TYPE=Release -DTAP_DSP_WERROR=ON -DTAP_DSP_BUILD_CAPI=ON:Cross-platform safety checked by inspection rather than assumed:
dsptap_capi.halready selects__declspec(dllexport)on Windows and__attribute__((visibility("default")))elsewhere, and the capi target does not linktap_dsp_warnings, soTAP_DSP_WERRORdoesn't apply to it. The standalone path is untouched —tools/capistill declares its own project and pulls the repo root in whentap::dspdoesn't already exist.Notes for the reviewer
The audit item that led here was partly wrong, and the real finding is bigger. I had flagged this repo's capi as "orphaned from the root build," implying it was the outlier. It isn't: the standalone-only arrangement is deliberate and documented, and the capi CMakeLists already handles both modes. What's actually true is that all six library repos default
BUILD_CAPIto OFF and no workflow in any of them enables it — so none of their C ABIs is compiled by CI either. This PR closes the gap here; the identical one-flag change would close it in AmbiTap, MuTap, SampleRateTap, RatioTap and TapTools. Happy to do that sweep if you want it.One thing I deliberately did not do: the capi target isn't warning-checked, since it doesn't link
tap_dsp_warnings. Adding that would be a genuine improvement but could surface warnings and turn CI red, so it belongs in its own change rather than riding along here.Generated by Claude Code