ci(swift-sdk): prune orphaned FFI header subdirs in build_ios.sh#3666
Conversation
…s.sh cargo doesn't reclaim include/<crate>/ when a crate leaves the dep tree, so on runners that preserve target/ across runs the orphan gets bundled into the xcframework and clang's umbrella-header check fails. Drive umbrella content and a prune step from one ordered INCLUDED_CRATES list so the script stays correct as crates come and go, and assert each crate's header exists before emitting the umbrella. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe iOS build script now uses a configurable ChangesiOS Build Script – Dynamic Crate Header Inclusion
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
✅ Review complete (commit 21ff9e3) |
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "75d627d7a59a810436018b19554d1a05491cd085c00540f9c300f6605fc9059b"
)Xcode manual integration:
|
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
PR is a small, well-scoped CI fix to packages/swift-sdk/build_ios.sh that prunes orphan cbindgen header subdirs before xcframework creation. No FFI surface, ownership, or ABI changes. One nitpick about silent drift in the opposite direction worth surfacing.
Reviewed commit: 21ff9e3
💬 1 nitpick(s)
Issue being fixed or feature implemented
The Swift SDK CI job (
.github/workflows/swift-sdk-build.yml, jobSwift SDK and Example build (warnings as errors)) has been failing on PRs that mergev3.1-devwith:The self-hosted macOS ARM64 runner preserves
target/across CI runs (introduced by #3632,git clean -ffdx -e target/ -e target/**). Recently #3644 removed thedash-spv-fficrate from the unified FFI build and dropped its#includefrom the umbrella header.cargonever reclaimstarget/<triple>/<profile>/include/<crate>/when a crate leaves the dependency tree, so the orphandash-spv-ffi/dash-spv-ffi.hsits in the cached include directory,xcodebuild -create-xcframework -headersbundles it into the framework, and Clang'sumbrella headercheck fails.What was done?
Made
packages/swift-sdk/build_ios.shself-correcting against this class of stale-state failure.INCLUDED_CRATESlist at the top of the script — the canonical set of FFI crates whose cbindgen output ships inDashSDKFFI.xcframework. Ordering matters because earlier headers define types referenced by later ones.inject_modulemapnow:INCLUDED_CRATES(catches orphans from removed/renamed crates), with safe quoting and a[[ -d "$dir" ]] || continueguard.INCLUDED_CRATEShas its expected header on disk — converts the silent drift case ("array references a crate Cargo.toml doesn't pull in") into a clear script-level error before Clang sees it.DashSDKFFI.hfrom the same list via aprintfloop, replacing the previous hardcoded heredoc.module.modulemapheredoc and the opaque-struct rewrite loop are untouched.The list is the one source of truth: a future crate removal becomes a one-line edit, and the script self-heals the runner's cache on the next run.
How Has This Been Tested?
Validated locally before pushing.
bash -n packages/swift-sdk/build_ios.shclean.inject_modulemapagainst the real staletarget/aarch64-apple-ios-sim/release/include/on this workstation (which contained the samedash-spv-ffi/orphan the runner has): the orphan was pruned, generatedDashSDKFFI.hcame out byte-identical to the post-#3644heredoc shape,module.modulemapunchanged.dash-network/) to simulate aCargo.toml/array drift: existence assertion fired withMissing header: …andexit 1. No silent failure path into Clang.On CI (this PR): the first run will exercise the bug — the runner's cached
target/still contains thedash-spv-ffi/subdir from prior builds, the prune step will remove it (look for→ pruned orphan header dir: dash-spv-ffiin the logs), the umbrella check passes, and the runner's cache is permanently repaired for every subsequent PR. Re-runs on the same SHA will be a no-op for the prune step.Out-of-scope: not switching the modulemap from
umbrella header "X.h"toumbrella .— that would mask the orphan rather than fix it, and would lose the explicit#includeorder the framework relies on for transitive type visibility.Breaking Changes
None. The generated umbrella header is byte-identical to the previous hardcoded heredoc on the no-orphan path, and the script is invoked the same way.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit