Add pkg-config and CMake C smoke cells#9
Conversation
The C smoke client hand-links the prebuilt libmoq with its own -I/-L/-l and
hardcoded system libs, so it never reads the moq.pc or lib/cmake/moq package
that the release tarball also ships. That left a whole class of packaging
breakage invisible: a wrong includedir/libdir or a missing framework in moq.pc
(the FFmpeg --enable-libmoq consumer path) passes the smoke test today.
Add two more subscriber cells alongside `c`, mirroring the
js-vite/js-esbuild/js-jsdelivr "same client, different delivery" pattern:
- c-pkgconfig: resolves cflags/libs via `pkg-config --cflags --static --libs
moq`. --static pulls Libs.private (the system frameworks/libs), so a broken
moq.pc fails the build.
- c-cmake: builds via find_package(moq) against the shipped CMake package,
importing the moq::moq target.
All three produce the same subscribe-only binary from the same tarball and run
identically. Split c_prepare into a shared, idempotent libmoq_fetch plus three
build helpers so the download happens once and each variant fails only its own
cell. Wire pkg-config + cmake into the CI deps and add the cells to the matrix
and the `full` recipe.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 49 minutes and 6 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThe PR expands C smoke-test coverage from one build variant to three. A new 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`just check` hardcoded `smoke.sh freshness.sh token.sh`, missing the two
extensionless bash wrappers under clients/docker/, and drove shfmt with CLI
flags. Switch to moq's setup: a committed .editorconfig carries the shfmt style
(indent 4, switch_case_indent, ignore vendored trees), and `just check`
discovers every shell file with `shfmt -f .` for both shfmt and shellcheck.
This surfaced pre-existing drift the local-only `just check` never caught (CI
doesn't run it): under the dev shell's shfmt 3.13.1 the scripts weren't clean.
Reformat them to be shfmt-clean (the compact `cmd || { ...; }` guards expand to
multi-line blocks) and silence freshness.sh's shellcheck findings -- SC2001 via
a parameter expansion, two intentional literal-$ greps via inline SC2016
disables.
The .editorconfig adds a clients/docker/* glob so the shebang-only wrappers get
the same style as the *.sh scripts.
Verified: shfmt --diff, shellcheck, and actionlint all clean over the discovered
set; rust -> c,c-pkgconfig,c-cmake still PASS after the reformat.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Adds two new C subscriber cells to the smoke matrix —
c-pkgconfigandc-cmake— alongside the existingc. All three build the same subscribe-onlyclients/c/subscribe.cagainst the same prebuilt libmoq tarball; they differ only in how the build discovers the library:c-I/-L/-l(unchanged)c-pkgconfigpkg-config --cflags --static --libs moqincludedir/libdiror a missing framework inmoq.pcc-cmakefind_package(moq)→moq::moqlib/cmake/moqpackage configThis mirrors the existing
js-vite/js-esbuild/js-jsdelivr"same client, different delivery" pattern.Why
The C client hand-links libmoq with its own
-I/-L/-lflags and hardcoded system libs, so it never reads thelib/pkgconfig/moq.pcorlib/cmake/moq/files that the release tarball also ships. That leaves a whole class of packaging breakage invisible to the smoke test — e.g. a wrongincludedir/libdiror a missing-framework CoreServicesinmoq.pc, the kind of thing moq#1758 fixed. Those are the FFmpeg--enable-libmoq/ CMake-consumer paths the harness had no equivalent of.The
--staticflag on the pkg-config build is the key bit: libmoq is a static archive, so--staticpulls inLibs.private(the system frameworks/libs), meaning a missing framework genuinely fails the build.Notes for reviewers
c_prepareis split into a shared, idempotentlibmoq_fetch(one download for all three variants) plusc_build_direct/c_build_pkgconfig/c_build_cmake. Each variant fails only its own cell; a failed download marks all three broken (same as the js group).clients/c/CMakeLists.txtis driven byc_build_cmake(CMAKE_PREFIX_PATH→ tarball root); it's not meant to be built standalone.pkg-config+cmaketo the Linux and macOS dep installs, andc-pkgconfig,c-cmaketo the matrix + thejust fullrecipe.cargo buildtarget tree where moq#1758's path bug actually lived. Closing that gap would mean building libmoq from source in the harness — a bigger change, arguably outside this repo's "test the public packages" charter. Flagging as a possible follow-up.Verification
bash -n smoke.shclean.libmoq-v0.3.5, aarch64):rust → c,c-pkgconfig,c-cmakeall PASS.shellcheck/shfmtlocally (not installed); CI'sjust checkwill lint.