UniFFI trial #750
UniFFI trial #750
Conversation
ladvoc
force-pushed
the
ladvoc/uniffi-trial
branch
from
48449a0 to
41bd64c
Compare
| pub struct Claims { | ||
| pub exp: u64, | ||
| pub iss: String, | ||
| pub nbf: u64, |
There was a problem hiding this comment.
[nitpick] can we use SystemTime instead?
Makes clear what comes from uniffi
Allows client to access the version specified in Cargo.toml
* Unverified token * CR: Names * CR: Test token
ladvoc
force-pushed
the
ladvoc/uniffi-trial
branch
from
a028a03 to
3faab50
Compare
|
The I'd also commit (local) cc @1egoman |
# Conflicts: # Cargo.lock
|
I'll try to complete the Swift distribution path in the meantime. |
|
Here's more or less everything we need for local distribution, the tricky part is: [tasks.swift-generate-manifest.env]
SPM_URL = { value = "https://dummy.com" }
SPM_CHECKSUM = { value = "1234" }Let's assume the following workflow:
optionally:
Alternatively, we can skip |
|
I made a manual pre-release here: https://github.com/livekit/livekit-uniffi-xcframework @ladvoc if you're able to finish the release pipeline here, that would be cool (artifact is OK as mentioned above). |
livekit-uniffi/Makefile.toml
Outdated
| --platforms maccatalyst \ | ||
| --platforms visionos \ | ||
| --platforms tvos \ | ||
| --lib-type dynamic \ |
There was a problem hiding this comment.
And one more thing @ladvoc static release is still ~40MB whereas dylib ~1.8MB.
Dynamic one won't pass AppStore validation (we'd need to wrap it in a Framework).
90426: Invalid Swift Support. The SwiftSupport folder is missing. Rebuild your app using the current public (GM) version of Xcode and resubmit it.
I'd rather go for stripping the static one further, any ideas?
There was a problem hiding this comment.
iOS in --release profile:
Ar Archive - 47 MB
There was a problem hiding this comment.
Okay, looks like DEAD_CODE_STRIPPING does the job and the bundle size increase is negligible (0.1 MB), moved back to static 729bc10
It also passes 🍏 validation 👍
I'm still open to optimization ideas 😄
livekit-uniffi/Makefile.toml
Outdated
| script = """ | ||
| # Rename modules inside the xcframework to match the module name (fix for Swift >6.2) | ||
| # Workaroud for cargo-swift v0.10.0 not supporting ffi_module_name | ||
| find "${SPM_NAME}/Rust${SPM_NAME}.xcframework" -depth -type d -name "Rust${SPM_NAME}" -exec bash -c 'mv "$1" "${1%Rust'${SPM_NAME}'}livekit_uniffiFFI"' _ {} \; || true |
There was a problem hiding this comment.
Trying to fix it here antoniusnaumann/cargo-swift#87
Applies a patch to fix newer Swift versions 6.2+ failing when the module name does not match folder name inside xcframework. `cargo-swift` fix in progress, livekit/rust-sdks#750 (comment) tests https://github.com/livekit/client-sdk-swift/actions/runs/20334177943/job/58416504807
📝 WalkthroughWalkthroughAdds a new workspace member Changes
Sequence DiagramssequenceDiagram
participant Client as Client (Node/Python/Swift)
participant UniFFI as UniFFI Binding Layer
participant Rust as livekit-uniffi (Rust)
participant LiveKitAPI as livekit-api
Client->>UniFFI: token_generate(options, credentials)
UniFFI->>Rust: call token_generate
Rust->>LiveKitAPI: construct & sign token
LiveKitAPI-->>Rust: return JWT
Rust-->>UniFFI: return JWT
UniFFI-->>Client: return JWT
sequenceDiagram
participant Client as Client (Node/Python/Swift)
participant UniFFI as UniFFI Binding Layer
participant RustLogger as Rust Logger (log_forward)
participant Channel as MPSC Channel
Client->>UniFFI: log_forward_bootstrap(level)
UniFFI->>RustLogger: init global logger
RustLogger->>Channel: create sender/receiver
Note over RustLogger,Channel: Rust emits logs -> sent to Channel
Client->>UniFFI: log_forward_receive()
UniFFI->>Channel: poll receiver
Channel-->>UniFFI: Option<LogForwardEntry>
UniFFI-->>Client: deliver entry or None
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@livekit-uniffi/python_test/main.py`:
- Around line 44-52: The receive_log_messages loop using log_forward_receive can
block forever because the Rust channel may never close; update
receive_log_messages (or the caller) to avoid hanging by adding a timeout around
await log_forward_receive (e.g., with asyncio.wait_for) or by running the
receiver concurrently with the main token operations and cancelling it
afterwards; specifically modify the async function receive_log_messages and the
asyncio.run invocation so that log_forward_receive calls time out (handle
asyncio.TimeoutError and treat it as "no more logs") or run receive_log_messages
as a background task and cancel/join it after the token operations complete.
In `@livekit-uniffi/support/node/download-lib.ts.tera`:
- Around line 45-54: generateLibraryName currently switches on
'windows'/'macos'/'linux' but targetPlatform() returns Rust triple fragments
(e.g. 'pc-windows-msvc', 'apple-darwin', 'unknown-linux-gnu'), so no case
matches; update generateLibraryName to normalize or pattern-match the triples
returned by targetPlatform() (e.g. use startsWith/includes checks like
startsWith('pc-windows') or includes('darwin')/includes('linux') or map the
known triples to the three targets) and return the correct filename for each,
and add a default branch that throws/returns a clear error; reference
generateLibraryName and targetPlatform to locate and change the logic.
🧹 Nitpick comments (9)
livekit-uniffi/README.md (1)
19-21: Add language specifier to fenced code blocks.The shell command blocks should specify a language for proper syntax highlighting and to satisfy markdown linting.
📝 Suggested fix
-``` +```bash cargo make swift-package```diff -``` +```bash cargo make node-package</details> Also applies to: 26-28 </blockquote></details> <details> <summary>livekit-uniffi/support/node/download-lib.ts.tera (1)</summary><blockquote> `119-126`: **Consider adding type assertions for CLI arguments.** The `parseArgs` values are typed as `string | boolean | undefined`, but `downloadLib` expects specific platform/arch types. Consider adding runtime validation or type assertions to catch invalid CLI inputs early. <details> <summary>♻️ Suggested improvement</summary> ```diff +function assertValidPlatform(value: unknown): asserts value is TargetPlatform { + const valid = ['pc-windows-msvc', 'apple-darwin', 'unknown-linux-gnu']; + if (typeof value !== 'string' || !valid.includes(value)) { + throw new Error(`Invalid platform: ${value}`); + } +} + +function assertValidArch(value: unknown): asserts value is TargetArch { + const valid = ['aarch64', 'x86_64']; + if (typeof value !== 'string' || !valid.includes(value)) { + throw new Error(`Invalid arch: ${value}`); + } +} if (require.main === module) { // ... const { values } = parseArgs({ args: process.argv.slice(2), options }); + if (values.platform) assertValidPlatform(values.platform); + if (values.arch) assertValidArch(values.arch); downloadLib( values.platform, // ...livekit-uniffi/src/log_forward.rs (1)
102-102: Consider adding context to theunwrap()for future debugging.While the
unwrap()is currently safe (the receiver can't be dropped while theLoggerexists inOnceCell), adding anexpect()with a message would help if the design ever changes.♻️ Suggested improvement
- self.tx.send(record).unwrap(); + self.tx.send(record).expect("Log receiver unexpectedly dropped");livekit-uniffi/node_test/index.ts (1)
67-68: Handle async errors frommain()to avoid silent failures.Top-level promise rejections won’t be surfaced clearly; add a catch handler.
🛠️ Proposed fix
if (require.main === module) { - main(); + main().catch((err) => { + console.error(err); + process.exit(1); + }); }livekit-uniffi/node_test/package.json (1)
5-7: Consider cleaning up the placeholder test script.The
npm testscript in this package is not currently invoked in CI or any automation (tests are run directly vianpx tsx index.tsper the README). However, if someone manually runsnpm testin this directory, it will unconditionally fail. Either replace it with"test": "tsx index.ts"or remove it entirely to avoid confusion.livekit-uniffi/python_test/main.py (1)
21-21: Consider explicit imports instead of star import.While star imports are acceptable for test files, using explicit imports would silence the static analysis warnings and make dependencies clearer:
from livekit_uniffi import ( log_forward_bootstrap, log_forward_receive, LogForwardFilter, build_version, ApiCredentials, token_generate, token_verify, TokenOptions, )livekit-uniffi/Cargo.toml (1)
1-11: Consider reordering sections for conventional layout.Placing
[lib]before[package]is valid but unconventional. The typical Cargo.toml order is[package]first, followed by[lib],[[bin]], dependencies, and profiles. This is a minor style preference.livekit-uniffi/support/node/package.json.tera (1)
12-17: Consider settingmainto CJS for broader compatibility.The
mainfield points to the ESM entry (dist/index.mjs), but conventionallymainshould point to CommonJS for tools that don't supportexports. Consider:"main": "dist/index.js", "module": "dist/index.mjs",However, with
engines.node >= 18and properexportsfield, this may not be necessary for your target audience.livekit-uniffi/src/access_token.rs (1)
184-185: Address TODO: Remove test logging or the TODO comment.The TODO indicates this debug log was added for testing log forwarding. Either remove the log statement if it's not needed in production, or remove the TODO comment if the logging should be kept.
Do you want me to help clean up these debug log statements across
token_generateandtoken_verify?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locklivekit-uniffi/node_test/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
Cargo.tomllicenserc.tomllivekit-api/src/access_token.rslivekit-api/src/test_token.txtlivekit-uniffi/.gitignorelivekit-uniffi/Cargo.tomllivekit-uniffi/Makefile.tomllivekit-uniffi/README.mdlivekit-uniffi/bindgen.rslivekit-uniffi/node_test/.gitignorelivekit-uniffi/node_test/index.tslivekit-uniffi/node_test/package.jsonlivekit-uniffi/python_test/main.pylivekit-uniffi/src/access_token.rslivekit-uniffi/src/build_info.rslivekit-uniffi/src/lib.rslivekit-uniffi/src/log_forward.rslivekit-uniffi/support/node/download-lib.ts.teralivekit-uniffi/support/node/package.json.teralivekit-uniffi/support/node/tsconfig.jsonlivekit-uniffi/support/node/tsup.config.tslivekit-uniffi/support/swift/Package.swift.teralivekit-uniffi/support/swift/[email protected]
🧰 Additional context used
🧬 Code graph analysis (1)
livekit-uniffi/python_test/main.py (3)
livekit-uniffi/src/log_forward.rs (2)
log_forward_bootstrap(28-32)log_forward_receive(41-44)livekit-uniffi/src/build_info.rs (1)
build_version(17-19)livekit-uniffi/src/access_token.rs (2)
token_generate(180-230)token_verify(238-250)
🪛 Gitleaks (8.30.0)
livekit-api/src/test_token.txt
[high] 1-1: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 markdownlint-cli2 (0.18.1)
livekit-uniffi/README.md
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
26-26: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.13)
livekit-uniffi/python_test/main.py
21-21: from livekit_uniffi import * used; unable to detect undefined names
(F403)
25-25: log_forward_bootstrap may be undefined, or defined from star imports
(F405)
25-25: LogForwardFilter may be undefined, or defined from star imports
(F405)
28-28: build_version may be undefined, or defined from star imports
(F405)
30-30: ApiCredentials may be undefined, or defined from star imports
(F405)
30-30: Possible hardcoded password assigned to argument: "secret"
(S106)
32-32: token_generate may be undefined, or defined from star imports
(F405)
33-33: TokenOptions may be undefined, or defined from star imports
(F405)
38-38: token_verify may be undefined, or defined from star imports
(F405)
46-46: log_forward_receive may be undefined, or defined from star imports
(F405)
🔇 Additional comments (25)
livekit-uniffi/node_test/.gitignore (1)
1-1: LGTM!Standard
.gitignoreentry for Node.js projects.livekit-uniffi/.gitignore (1)
1-2: LGTM!Correctly excludes generated artifacts and packaged bindings from version control.
livekit-api/src/test_token.txt (1)
1-1: Test token is appropriate for unit testing.The Gitleaks warning is a false positive—this is a test fixture with dummy credentials (
myapikeyas issuer,identityas subject) and a far-future expiration date (year 2100). The token is used for testingClaims::from_unverifiedand related token parsing functionality.licenserc.toml (1)
16-16: LGTM!The negation pattern correctly ensures
Makefile.tomlis checked for license headers despite the*.tomlexclusion. Excludinglivekit-uniffi/packagesappropriately skips generated artifacts.Also applies to: 21-21
livekit-uniffi/src/build_info.rs (1)
15-19: LGTM!Clean implementation using compile-time
env!macro to expose the crate version through UniFFI. The#[uniffi::export]attribute correctly generates cross-language bindings.Cargo.toml (1)
8-8: LGTM!The new
livekit-unifficrate is properly added to the workspace members.livekit-uniffi/Makefile.toml (2)
24-26: Placeholder download URL needs to be updated before release.The
CDYLIB_DOWNLOAD_BASE_URLpoints to an example URL. Ensure this is updated to the actual release artifact location before merging or enabling release builds.
293-299:sed -i ''syntax is acceptable for this macOS-only manual task.The
swift-packagetask is a manual developer workflow documented in the README for local Swift package generation, not part of CI automation. Since XCFramework/Swift package building is inherently macOS-focused, the BSD sed syntaxsed -i ''is appropriate and doesn't require portability fixes.If this task were automated in CI on multiple platforms, portability would be a concern. As-is, no action needed.
livekit-uniffi/support/swift/[email protected] (1)
1-41: LGTM!The Swift 6.0 manifest template properly handles development vs release profiles with appropriate binaryTarget configurations. The visionOS platform support (.v2) is correctly included.
livekit-uniffi/support/swift/Package.swift.tera (1)
8-13: visionOS is missing from platform list, unlike the Swift 6.0 template.The
[email protected]includes.visionOS(.v2)but this template omits it. Since visionOS support was added in Swift 5.9, this may be intentional (to support older Xcode versions) or an oversight.livekit-uniffi/src/log_forward.rs (2)
41-44:try_lock()may silently returnNoneunder lock contention.If multiple consumers call
log_forward_receive()concurrently and one holds the lock, others will returnNoneimmediately (interpreted as "forwarding ended"). This could cause premature termination of log polling loops in client code.Consider whether this is the intended behavior, or if you want to use
.lock().awaitto wait for the mutex (though this changes semantics for single-consumer scenarios).
27-32: LGTM!The bootstrap function correctly handles idempotent initialization while allowing subsequent level changes, as discussed in prior review comments.
livekit-uniffi/bindgen.rs (1)
15-19: LGTM for the bindgen entrypoint.livekit-uniffi/support/node/tsconfig.json (1)
2-13: LGTM for the TypeScript compiler settings.livekit-uniffi/support/node/tsup.config.ts (1)
3-15: LGTM for the tsup build configuration.livekit-uniffi/src/lib.rs (1)
15-26: LGTM for the module wiring.livekit-uniffi/node_test/index.ts (1)
57-63: The review comment is incorrect. The concern about empty strings terminating the loop does not apply here.The function
logForwardReceive()returnsOption<LogForwardEntry>(TypeScript:LogForwardEntry | null), not a string. The checkif (!message)correctly tests whether the entire object isnullto detect loop termination. Empty content in the message field insideLogForwardEntryhas no effect on the loop condition, so the current code handles the intended termination signal correctly.livekit-api/src/access_token.rs (2)
154-167: LGTM!The
from_unverifiedimplementation correctly disables signature validation while retainingexpandnbftime validation, which is appropriate for inspecting token contents without the secret. The method name clearly communicates the security implications.
398-441: LGTM!Comprehensive test coverage for
from_unverified:
- Parses pre-existing token with room configuration
- Parses freshly generated token
- Correctly handles malformed token with altered signature
Good validation of the intended behavior.
livekit-uniffi/Cargo.toml (1)
28-34: LGTM!The release profile is well-configured for FFI distribution:
panic = "abort"prevents undefined behavior from unwinding across FFI boundaries- Size optimizations (
opt-level = "z",lto,strip) are appropriate for distributable binarieslivekit-uniffi/support/node/package.json.tera (1)
34-44: Potential trailing comma issue in generated JSON.The template iterates over
devDependenciesand adds a trailing comma after each entry (line 37), which is followed by hardcoded entries. However, ifdevDependenciesis empty, the first hardcoded entry ("tsup") won't have a preceding comma separator from the loop, and the JSON will still be valid.The current pattern works but is fragile. If
devDependencieshas entries, the trailing comma from line 37 correctly precedes"tsup". Consider explicitly handling the comma separator for clarity:💡 Alternative approach
"devDependencies": { {% for package_name, package_version in devDependencies %} "{{package_name | addslashes}}": "{{package_version | addslashes}}", {%- endfor %} "tsup": "^8.4.0", ...The current approach works because of the trailing comma in the loop, but ensure the template is tested with both empty and populated
devDependencies.livekit-uniffi/src/access_token.rs (4)
21-29: LGTM!Correct use of
#[uniffi::remote(Error)]to expose the existingAccessTokenErrorfromlivekit_api. Theflat_errorattribute is appropriate for cross-language error handling.
107-139: LGTM!Using
u64for timestamps (exp,nbf) is appropriate for FFI - it's more portable across target languages thanSystemTime. Theusize as u64conversion at lines 126 and 128 is safe on both 32-bit (widening) and 64-bit (no-op) platforms.
237-250: LGTM!The verification logic correctly handles optional credentials and converts the result to the UniFFI-compatible
Claimstype. Note: the TODO on line 242-243 should also be addressed (same as noted fortoken_generate).
252-262: LGTM!Excellent documentation with a clear security warning. The implementation correctly delegates to the API layer's
from_unverifiedand converts to the UniFFI-compatible type.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| async def receive_log_messages(): | ||
| while True: | ||
| message = await log_forward_receive() | ||
| if message is None: | ||
| print("Log forwarding ended") | ||
| break | ||
| print(f"Log from Rust: {message}") | ||
|
|
||
| asyncio.run(receive_log_messages()) |
There was a problem hiding this comment.
Log receive loop may block indefinitely.
The async loop waits for log messages but has no mechanism to exit after the token operations complete. Unless the Rust log channel is explicitly closed, log_forward_receive() will block forever waiting for more messages, causing the test script to hang.
Consider adding a timeout or running the log receiver concurrently with the main operations:
💡 Suggested approach using asyncio.wait_for or concurrent execution
async def receive_log_messages():
import asyncio
try:
while True:
message = await asyncio.wait_for(log_forward_receive(), timeout=1.0)
if message is None:
print("Log forwarding ended")
break
print(f"Log from Rust: {message}")
except asyncio.TimeoutError:
print("No more log messages (timeout)")🧰 Tools
🪛 Ruff (0.14.13)
46-46: log_forward_receive may be undefined, or defined from star imports
(F405)
🤖 Prompt for AI Agents
In `@livekit-uniffi/python_test/main.py` around lines 44 - 52, The
receive_log_messages loop using log_forward_receive can block forever because
the Rust channel may never close; update receive_log_messages (or the caller) to
avoid hanging by adding a timeout around await log_forward_receive (e.g., with
asyncio.wait_for) or by running the receiver concurrently with the main token
operations and cancelling it afterwards; specifically modify the async function
receive_log_messages and the asyncio.run invocation so that log_forward_receive
calls time out (handle asyncio.TimeoutError and treat it as "no more logs") or
run receive_log_messages as a background task and cancel/join it after the token
operations complete.
| function generateLibraryName(platform: TargetPlatform) { | ||
| switch (platform) { | ||
| case "windows": | ||
| return `${PACKAGE_NAME}.dll`; | ||
| case "macos": | ||
| return `lib${PACKAGE_NAME}.dylib`; | ||
| case "linux": | ||
| return `lib${PACKAGE_NAME}.so`; | ||
| } | ||
| } |
There was a problem hiding this comment.
Platform string mismatch will cause generateLibraryName to never match.
The targetPlatform() function returns Rust target triple fragments ('pc-windows-msvc', 'apple-darwin', 'unknown-linux-gnu'), but generateLibraryName() switches on different strings ('windows', 'macos', 'linux'). This will cause all cases to fall through and return undefined.
🐛 Proposed fix
function generateLibraryName(platform: TargetPlatform) {
switch (platform) {
- case "windows":
+ case "pc-windows-msvc":
return `${PACKAGE_NAME}.dll`;
- case "macos":
+ case "apple-darwin":
return `lib${PACKAGE_NAME}.dylib`;
- case "linux":
+ case "unknown-linux-gnu":
return `lib${PACKAGE_NAME}.so`;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function generateLibraryName(platform: TargetPlatform) { | |
| switch (platform) { | |
| case "windows": | |
| return `${PACKAGE_NAME}.dll`; | |
| case "macos": | |
| return `lib${PACKAGE_NAME}.dylib`; | |
| case "linux": | |
| return `lib${PACKAGE_NAME}.so`; | |
| } | |
| } | |
| function generateLibraryName(platform: TargetPlatform) { | |
| switch (platform) { | |
| case "pc-windows-msvc": | |
| return `${PACKAGE_NAME}.dll`; | |
| case "apple-darwin": | |
| return `lib${PACKAGE_NAME}.dylib`; | |
| case "unknown-linux-gnu": | |
| return `lib${PACKAGE_NAME}.so`; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@livekit-uniffi/support/node/download-lib.ts.tera` around lines 45 - 54,
generateLibraryName currently switches on 'windows'/'macos'/'linux' but
targetPlatform() returns Rust triple fragments (e.g. 'pc-windows-msvc',
'apple-darwin', 'unknown-linux-gnu'), so no case matches; update
generateLibraryName to normalize or pattern-match the triples returned by
targetPlatform() (e.g. use startsWith/includes checks like
startsWith('pc-windows') or includes('darwin')/includes('linux') or map the
known triples to the three targets) and return the correct filename for each,
and add a default branch that throws/returns a clear error; reference
generateLibraryName and targetPlatform to locate and change the logic.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@livekit-uniffi/Makefile.toml`:
- Around line 25-27: The CDYLIB_DOWNLOAD_BASE_URL is set to the example release
URL and must be guarded so release builds don't embed a non-existent example
asset; update the Makefile.toml logic that sets CDYLIB_DOWNLOAD_BASE_URL (and
the downloader generation code referenced around the other block at the 226-236
region) to require an explicit environment variable (e.g.
CDYLIB_DOWNLOAD_BASE_URL or CDYLIB_DOWNLOAD_BASE_URL_RELEASE) or only use the
example value when a non-release flag is present (e.g. when PROFILE != "release"
or when a DEV_DOWNLOADS=true env is set); modify the downloader generation step
to fail fast with a clear error if the URL is unset in release mode, and
reference CDYLIB_DOWNLOAD_BASE_URL in both places to ensure they use the guarded
value.
- Around line 281-324: The task config currently hard-sets SPM_URL and
SPM_CHECKSUM in [tasks.swift-generate-manifest.env], which unconditionally
overrides CI-provided values; remove the unconditional env entries and instead
handle defaults and validation inside the swift-generate-manifest script: in the
script for the task (swift-generate-manifest) only assign a dummy default to
SPM_URL/SPM_CHECKSUM when the env var is empty (e.g. if [ -z "$SPM_URL" ]; then
SPM_URL="https://dummy.com"; fi), and after generating the manifests fail fast
in release CI (detect via CI flag or BUILD_TYPE) if the values are still the
dummy placeholders (exit 1 with an error message). Update references in the
script and keep the unique symbols SPM_URL, SPM_CHECKSUM, and task names
swift-generate-manifest and swift-move-to-packages to locate the changes.
- Around line 107-117: The locate-lib task constructs LIB_PATH using a LIB_NAME
that always includes a "lib" prefix which is incorrect on Windows; update the
locate-lib script to set LIB_NAME conditionally based on
CARGO_MAKE_RUST_TARGET_OS (or use CARGO_MAKE_RUST_TARGET_OS value): when the OS
is "windows" use "${CARGO_MAKE_CRATE_FS_NAME}.${LIB_EXT}" (no "lib" prefix) and
for other OSes use "lib${CARGO_MAKE_CRATE_FS_NAME}.${LIB_EXT}", then build
LIB_PATH as before and export LIB_PATH; adjust the script inside the locate-lib
duckscript block to branch on CARGO_MAKE_RUST_TARGET_OS and set LIB_NAME
accordingly so bindgen and downstream tasks find the correct DLL name on
Windows.
- Around line 185-224: The node packaging tasks node-move-code-into-src,
node-copy-support-files, and node-link-lib use the POSIX `@shell` runner with
mv/cp/ln which will fail on Windows; update these tasks to either switch their
script_runner to `@duckscript` and rewrite the commands using cross-platform
duckscript steps (matching how the build task uses `@duckscript`), or add explicit
platform conditions (e.g., condition = { platforms = ["linux", "macos"] }) to
each of node-move-code-into-src, node-copy-support-files, and node-link-lib so
they do not run on Windows; ensure node-generate-manifest and
node-generate-downloader remain unchanged unless they also need platform gating.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
livekit-uniffi/Makefile.toml
🔇 Additional comments (2)
livekit-uniffi/Makefile.toml (2)
34-103: Build matrix and tier‑3 handling look solid.No issues found here.
146-157: Kotlin/Python bindgen task wiring looks good.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| # Base URL for downloading release builds of the cdylib. | ||
| # Currently, it is assumed the archives will be named "ffi-<platform>-<arch>.zip". | ||
| CDYLIB_DOWNLOAD_BASE_URL = "https://github.com/livekit/example/releases/download/example/build" |
There was a problem hiding this comment.
Prevent release Node packages from using the example download base URL.
If CDYLIB_DOWNLOAD_BASE_URL stays on the example value, release downloaders will point at a non‑existent asset. Add a release‑only guard (or require an explicit env var) before downloader generation.
🔧 Suggested fix
+[tasks.node-verify-download-url]
+private = true
+condition = { profiles = ["release"] }
+script_runner = "@shell"
+script = """
+if [ "${CDYLIB_DOWNLOAD_BASE_URL}" = "https://github.com/livekit/example/releases/download/example/build" ]; then
+ echo "CDYLIB_DOWNLOAD_BASE_URL must be set for release" >&2
+ exit 1
+fi
+"""
+
[tasks.node-package-flow]
private = true
dependencies = [
"node-bindgen",
"node-move-code-into-src",
"node-generate-manifest",
+ "node-verify-download-url",
"node-generate-downloader",
"node-copy-support-files",
"node-format",
"node-link-lib"
]Also applies to: 226-236
🤖 Prompt for AI Agents
In `@livekit-uniffi/Makefile.toml` around lines 25 - 27, The
CDYLIB_DOWNLOAD_BASE_URL is set to the example release URL and must be guarded
so release builds don't embed a non-existent example asset; update the
Makefile.toml logic that sets CDYLIB_DOWNLOAD_BASE_URL (and the downloader
generation code referenced around the other block at the 226-236 region) to
require an explicit environment variable (e.g. CDYLIB_DOWNLOAD_BASE_URL or
CDYLIB_DOWNLOAD_BASE_URL_RELEASE) or only use the example value when a
non-release flag is present (e.g. when PROFILE != "release" or when a
DEV_DOWNLOADS=true env is set); modify the downloader generation step to fail
fast with a clear error if the URL is unset in release mode, and reference
CDYLIB_DOWNLOAD_BASE_URL in both places to ensure they use the guarded value.
| [tasks.locate-lib.env.LIB_EXT] | ||
| source = "${CARGO_MAKE_RUST_TARGET_OS}" | ||
| mapping = { linux = "so", macos = "dylib", windows = "dll" } | ||
|
|
||
| [tasks.locate-lib] | ||
| private = true | ||
| script_runner = "@duckscript" | ||
| script = """ | ||
| path = join_path ${TARGET_DIR} ${TARGET} release/ ${LIB_NAME}.${LIB_EXT} | ||
| set_env LIB_PATH "${path}" | ||
| """ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the Makefile.toml to understand the full context
fd "Makefile.toml" --type fRepository: livekit/rust-sdks
Length of output: 89
🏁 Script executed:
# Read the Makefile.toml to understand the full context
cat -n livekit-uniffi/Makefile.tomlRepository: livekit/rust-sdks
Length of output: 11552
🏁 Script executed:
# Let me verify Windows Rust cdylib naming conventions by checking Rust documentation
# and looking at how the crate is actually built
fd "Cargo.toml" livekit-uniffi --type fRepository: livekit/rust-sdks
Length of output: 86
🏁 Script executed:
# Read the Cargo.toml to understand the crate configuration
cat -n livekit-uniffi/Cargo.tomlRepository: livekit/rust-sdks
Length of output: 1102
🏁 Script executed:
# Let me search for any existing usage or documentation about this issue in the codebase
rg "LIB_PATH" livekit-uniffi/Repository: livekit/rust-sdks
Length of output: 294
🏁 Script executed:
# Let me verify the exact Rust cdylib naming convention with a quick check
web_searchRepository: livekit/rust-sdks
Length of output: 111
🌐 Web query:
Rust cdylib naming convention Windows Linux macOS
💡 Result:
Short answer — cdylib filenames follow the platform's native dynamic-library naming:
- Linux / other ELF: lib.so. [1]
- macOS: lib.dylib. [1]
- Windows: .dll (with an optional import .lib for MSVC linking). [1][2]
Notes: cdylib is the crate type that emits a C-compatible dynamic library for those native formats; Cargo/rustc choose the platform-specific prefix/suffix automatically. [1][3]
Sources:
[1] Rust Reference — Linkage / crate types.
[2] Rustonomicon — FFI / calling Rust from C.
[3] RFC 1510 (cdylib description).
Fix Windows DLL naming in locate-lib task.
On Windows, Rust cdylib artifacts are named <crate>.dll without the lib prefix, but the current code unconditionally uses lib${CARGO_MAKE_CRATE_FS_NAME}, causing LIB_PATH to point to a non-existent file and breaking bindgen and all dependent tasks on Windows builds.
🔧 Suggested fix
[tasks.locate-lib]
private = true
script_runner = "@duckscript"
script = """
-path = join_path ${TARGET_DIR} ${TARGET} release/ ${LIB_NAME}.${LIB_EXT}
+if eq ${CARGO_MAKE_RUST_TARGET_OS} "windows"
+ lib_name = ${CARGO_MAKE_CRATE_FS_NAME}
+else
+ lib_name = "lib${CARGO_MAKE_CRATE_FS_NAME}"
+end
+path = join_path ${TARGET_DIR} ${TARGET} release/ ${lib_name}.${LIB_EXT}
set_env LIB_PATH "${path}"
"""🤖 Prompt for AI Agents
In `@livekit-uniffi/Makefile.toml` around lines 107 - 117, The locate-lib task
constructs LIB_PATH using a LIB_NAME that always includes a "lib" prefix which
is incorrect on Windows; update the locate-lib script to set LIB_NAME
conditionally based on CARGO_MAKE_RUST_TARGET_OS (or use
CARGO_MAKE_RUST_TARGET_OS value): when the OS is "windows" use
"${CARGO_MAKE_CRATE_FS_NAME}.${LIB_EXT}" (no "lib" prefix) and for other OSes
use "lib${CARGO_MAKE_CRATE_FS_NAME}.${LIB_EXT}", then build LIB_PATH as before
and export LIB_PATH; adjust the script inside the locate-lib duckscript block to
branch on CARGO_MAKE_RUST_TARGET_OS and set LIB_NAME accordingly so bindgen and
downstream tasks find the correct DLL name on Windows.
| [tasks.node-move-code-into-src] | ||
| private = true | ||
| script_runner = "@shell" | ||
| script = """ | ||
| mkdir -p ${PACKAGES_DIR}/${LANG}/src | ||
| mv ${PACKAGES_DIR}/${LANG}/*.ts ${PACKAGES_DIR}/${LANG}/src | ||
| """ | ||
|
|
||
| [tasks.node-generate-manifest] | ||
| private = true | ||
| extend = "tera" | ||
| args = [ | ||
| "--env", | ||
| "-t", "${SUPPORT_DIR}/${LANG}/package.json.tera", | ||
| "-o", "${PACKAGES_DIR}/${LANG}/package.json", | ||
| "${PACKAGES_DIR}/${LANG}/package.json" | ||
| # Provide original package.json as input | ||
| ] | ||
|
|
||
| [tasks.node-copy-support-files] | ||
| private = true | ||
| script_runner = "@shell" | ||
| script = """ | ||
| cp ${SUPPORT_DIR}/${LANG}/tsconfig.json ${PACKAGES_DIR}/${LANG}/tsconfig.json | ||
| cp ${SUPPORT_DIR}/${LANG}/tsup.config.ts ${PACKAGES_DIR}/${LANG}/tsup.config.ts | ||
| """ | ||
|
|
||
| [tasks.node-generate-downloader] | ||
| private = true | ||
| condition = { profiles = ["release"] } | ||
| extend = "tera" | ||
| args = ["--env-only", "-t", "${SUPPORT_DIR}/${LANG}/download-lib.ts.tera", "-o", "${PACKAGES_DIR}/${LANG}/download-lib.ts"] | ||
|
|
||
| [tasks.node-link-lib] | ||
| private = true | ||
| condition = { profiles = ["development"] } | ||
| script_runner = "@shell" | ||
| script = """ | ||
| ln -sf ${LIB_PATH} ${PACKAGES_DIR}/${LANG}/src | ||
| """ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | head -20Repository: livekit/rust-sdks
Length of output: 513
🏁 Script executed:
find . -name "Makefile.toml" -o -name ".github" -type d | head -10Repository: livekit/rust-sdks
Length of output: 101
🏁 Script executed:
# Check the full Makefile.toml to understand context better
wc -l livekit-uniffi/Makefile.tomlRepository: livekit/rust-sdks
Length of output: 93
🏁 Script executed:
cat -n livekit-uniffi/Makefile.toml | head -50Repository: livekit/rust-sdks
Length of output: 2009
🏁 Script executed:
cat -n livekit-uniffi/Makefile.toml | sed -n '180,230p'Repository: livekit/rust-sdks
Length of output: 1711
🏁 Script executed:
# Search for platform-specific conditions or duckscript usage in Makefile.toml
grep -n "condition\|duckscript\|win\|platform" livekit-uniffi/Makefile.tomlRepository: livekit/rust-sdks
Length of output: 1156
🏁 Script executed:
# Check if Windows CI is configured in workflows
cat .github/workflows/builds.ymlRepository: livekit/rust-sdks
Length of output: 4073
🏁 Script executed:
# Search for Node-related workflows or CI configuration
find .github/workflows -name "*.yml" -o -name "*.yaml" | xargs grep -l "node\|npm\|uniffi" 2>/dev/nullRepository: livekit/rust-sdks
Length of output: 45
🏁 Script executed:
# Check the ffi-builds workflow which might be related to Node packaging
cat .github/workflows/ffi-builds.yml | head -100Repository: livekit/rust-sdks
Length of output: 3566
🏁 Script executed:
# Search for references to node-package task in Makefile
grep -n "node-package\|node-bindgen" livekit-uniffi/Makefile.tomlRepository: livekit/rust-sdks
Length of output: 199
🏁 Script executed:
# Check if there's any documentation about Windows support or platform handling
grep -n "windows\|win32\|Windows" livekit-uniffi/Makefile.tomlRepository: livekit/rust-sdks
Length of output: 413
🏁 Script executed:
# Check the node-package-flow task and its dependencies
cat -n livekit-uniffi/Makefile.toml | sed -n '226,280p'Repository: livekit/rust-sdks
Length of output: 1906
🏁 Script executed:
# Look for any workflow that triggers node-package task
grep -r "node-package\|node.package" .github/workflows/Repository: livekit/rust-sdks
Length of output: 43
🏁 Script executed:
# Check if there's a separate Node build/release workflow
ls -la .github/workflows/ | grep -i nodeRepository: livekit/rust-sdks
Length of output: 43
🏁 Script executed:
# Search for any duckscript usage for file operations to understand patterns
grep -n "duckscript\|@shell" livekit-uniffi/Makefile.toml | head -20Repository: livekit/rust-sdks
Length of output: 335
🏁 Script executed:
# Search all workflows for invocation of node-package or cargo make calls
grep -r "node-package\|cargo.*make\|cargo-make" .github/workflows/Repository: livekit/rust-sdks
Length of output: 43
🏁 Script executed:
# Look at the full Makefile to understand when node-package is called
cat -n livekit-uniffi/Makefile.toml | sed -n '1,50p'Repository: livekit/rust-sdks
Length of output: 2009
🏁 Script executed:
# Check release workflow which might build Node packages
cat .github/workflows/release.yml 2>/dev/null | head -100Repository: livekit/rust-sdks
Length of output: 2825
🏁 Script executed:
# Look at CargoMake documentation or see how `@shell` behaves - check nearby tasks
cat -n livekit-uniffi/Makefile.toml | sed -n '280,310p'Repository: livekit/rust-sdks
Length of output: 1588
🏁 Script executed:
# Check if there's any shell configuration or Windows handling in cargo-make config
cat livekit-uniffi/Makefile.toml | grep -i "shell\|windows\|@" | head -30Repository: livekit/rust-sdks
Length of output: 763
🏁 Script executed:
# Look for any .cargo/config.toml that might set shell behavior
cat .cargo/config.toml 2>/dev/nullRepository: livekit/rust-sdks
Length of output: 497
🏁 Script executed:
# Check if swift-move-to-packages (which also uses `@shell` with POSIX commands) has platform conditions
cat -n livekit-uniffi/Makefile.toml | sed -n '294,320p'Repository: livekit/rust-sdks
Length of output: 1244
Use duckscript or platform-specific conditions for file operations in Node packaging tasks.
The tasks node-move-code-into-src, node-copy-support-files, and node-link-lib (lines 185–224) use @shell runner with POSIX-only commands: mv, cp, and ln -sf. These will fail on Windows since the commands don't exist in cmd.exe or PowerShell.
The project explicitly supports Windows targets (x86_64-pc-windows-msvc, aarch64-pc-windows-msvc), and these tasks have no platform conditions to prevent execution on Windows. Consider converting to @duckscript (like the build task at line 41) or adding condition = { platforms = ["linux", "macos"] } if Node packaging is not intended for Windows.
🤖 Prompt for AI Agents
In `@livekit-uniffi/Makefile.toml` around lines 185 - 224, The node packaging
tasks node-move-code-into-src, node-copy-support-files, and node-link-lib use
the POSIX `@shell` runner with mv/cp/ln which will fail on Windows; update these
tasks to either switch their script_runner to `@duckscript` and rewrite the
commands using cross-platform duckscript steps (matching how the build task uses
`@duckscript`), or add explicit platform conditions (e.g., condition = { platforms
= ["linux", "macos"] }) to each of node-move-code-into-src,
node-copy-support-files, and node-link-lib so they do not run on Windows; ensure
node-generate-manifest and node-generate-downloader remain unchanged unless they
also need platform gating.
| [tasks.swift-generate-manifest.env] | ||
| SPM_URL = { value = "https://dummy.com" } | ||
| SPM_CHECKSUM = { value = "1234" } | ||
|
|
||
| [tasks.swift-generate-manifest] | ||
| private = true | ||
| install_crate = { crate_name = "tera-cli", binary = "tera", test_arg = "--version" } | ||
| script_runner = "@shell" | ||
| script = """ | ||
| tera --env --file ${SUPPORT_DIR}/${LANG}/Package.swift.tera > ${SPM_NAME}/Package.swift | ||
| tera --env --file ${SUPPORT_DIR}/${LANG}/[email protected] > ${SPM_NAME}/[email protected] | ||
| """ | ||
|
|
||
| [tasks.swift-move-to-packages] | ||
| private = true | ||
| script_runner = "@shell" | ||
| script = """ | ||
| # Patch the generated XCFramework and Swift sources to match the framework name | ||
| # This ensures compatibility with Swift 6.2+ which requires the module name to match the framework name | ||
|
|
||
| FFI_NAME="livekit_uniffiFFI" | ||
|
|
||
| # 1. Rename header files inside the XCFramework | ||
| find "${SPM_NAME}/${XCFRAMEWORK_NAME}.xcframework" -name "${FFI_NAME}.h" -exec bash -c 'mv "$1" "$(dirname "$1")/'"${XCFRAMEWORK_NAME}"'.h"' _ {} \\; | ||
|
|
||
| # 2. Patch the modulemaps inside the XCFramework | ||
| find "${SPM_NAME}/${XCFRAMEWORK_NAME}.xcframework" -name "module.modulemap" -exec sed -i '' "s/${FFI_NAME}/${XCFRAMEWORK_NAME}/g" {} + | ||
|
|
||
| # 3. Patch the generated Swift sources | ||
| find "${SPM_NAME}/Sources" -name "*.swift" -exec sed -i '' "s/${FFI_NAME}/${XCFRAMEWORK_NAME}/g" {} + | ||
|
|
||
| out_dir="${PACKAGES_DIR}/${LANG}" | ||
| rm -rf "${out_dir}" | ||
| mkdir -p "${out_dir}" | ||
| mv "${SPM_NAME}" "${out_dir}" | ||
| """ | ||
|
|
||
| [tasks.swift-package-flow] | ||
| private = true | ||
| dependencies = [ | ||
| "swift-xcframework", | ||
| "swift-generate-manifest", | ||
| "swift-move-to-packages" | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "Makefile.toml" -type fRepository: livekit/rust-sdks
Length of output: 178
🏁 Script executed:
# Check if the file exists and look at the relevant sections
grep -n "SPM_URL\|SPM_CHECKSUM" -r . --include="Makefile.toml" -A 2 -B 2Repository: livekit/rust-sdks
Length of output: 416
🏁 Script executed:
# Check for CI configuration to see if these values are set
find . -name "*.yml" -o -name "*.yaml" | head -20Repository: livekit/rust-sdks
Length of output: 375
🏁 Script executed:
# Check the release workflow to see if SPM_URL and SPM_CHECKSUM are provided
cat -n ./.github/workflows/release.yml | head -100Repository: livekit/rust-sdks
Length of output: 3525
🏁 Script executed:
# Check ffi-builds workflow
cat -n ./.github/workflows/ffi-builds.yml | head -150Repository: livekit/rust-sdks
Length of output: 6582
🏁 Script executed:
# Get more context around the problematic lines in Makefile.toml
sed -n '275,330p' ./livekit-uniffi/Makefile.tomlRepository: livekit/rust-sdks
Length of output: 1841
🏁 Script executed:
# Search for where SPM_URL and SPM_CHECKSUM are used in the tera templates
grep -r "SPM_URL\|SPM_CHECKSUM" ./livekit-uniffi/support --include="*.tera" -A 2 -B 2Repository: livekit/rust-sdks
Length of output: 1063
Hard-coded dummy SPM_URL and SPM_CHECKSUM values override CI-provided values.
The environment variables are unconditionally set in the task configuration, which causes the dummy values to override any CI-provided environment variables and generates invalid Package.swift manifests in release builds. Default only when unset and fail fast during release if dummy values persist.
🔧 Suggested fix
[tasks.swift-generate-manifest.env]
-SPM_URL = { value = "https://dummy.com" }
-SPM_CHECKSUM = { value = "1234" }
+SPM_URL = { value = "https://dummy.com", condition = { env_not_set = ["SPM_URL"] } }
+SPM_CHECKSUM = { value = "1234", condition = { env_not_set = ["SPM_CHECKSUM"] } }
+[tasks.swift-verify-spm-env]
+private = true
+condition = { profiles = ["release"] }
+script_runner = "@shell"
+script = """
+if [ "${SPM_URL}" = "https://dummy.com" ] || [ "${SPM_CHECKSUM}" = "1234" ]; then
+ echo "SPM_URL/SPM_CHECKSUM must be set for release" >&2
+ exit 1
+fi
+"""
+
[tasks.swift-package-flow]
private = true
dependencies = [
"swift-xcframework",
+ "swift-verify-spm-env",
"swift-generate-manifest",
"swift-move-to-packages"
]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [tasks.swift-generate-manifest.env] | |
| SPM_URL = { value = "https://dummy.com" } | |
| SPM_CHECKSUM = { value = "1234" } | |
| [tasks.swift-generate-manifest] | |
| private = true | |
| install_crate = { crate_name = "tera-cli", binary = "tera", test_arg = "--version" } | |
| script_runner = "@shell" | |
| script = """ | |
| tera --env --file ${SUPPORT_DIR}/${LANG}/Package.swift.tera > ${SPM_NAME}/Package.swift | |
| tera --env --file ${SUPPORT_DIR}/${LANG}/[email protected] > ${SPM_NAME}/[email protected] | |
| """ | |
| [tasks.swift-move-to-packages] | |
| private = true | |
| script_runner = "@shell" | |
| script = """ | |
| # Patch the generated XCFramework and Swift sources to match the framework name | |
| # This ensures compatibility with Swift 6.2+ which requires the module name to match the framework name | |
| FFI_NAME="livekit_uniffiFFI" | |
| # 1. Rename header files inside the XCFramework | |
| find "${SPM_NAME}/${XCFRAMEWORK_NAME}.xcframework" -name "${FFI_NAME}.h" -exec bash -c 'mv "$1" "$(dirname "$1")/'"${XCFRAMEWORK_NAME}"'.h"' _ {} \\; | |
| # 2. Patch the modulemaps inside the XCFramework | |
| find "${SPM_NAME}/${XCFRAMEWORK_NAME}.xcframework" -name "module.modulemap" -exec sed -i '' "s/${FFI_NAME}/${XCFRAMEWORK_NAME}/g" {} + | |
| # 3. Patch the generated Swift sources | |
| find "${SPM_NAME}/Sources" -name "*.swift" -exec sed -i '' "s/${FFI_NAME}/${XCFRAMEWORK_NAME}/g" {} + | |
| out_dir="${PACKAGES_DIR}/${LANG}" | |
| rm -rf "${out_dir}" | |
| mkdir -p "${out_dir}" | |
| mv "${SPM_NAME}" "${out_dir}" | |
| """ | |
| [tasks.swift-package-flow] | |
| private = true | |
| dependencies = [ | |
| "swift-xcframework", | |
| "swift-generate-manifest", | |
| "swift-move-to-packages" | |
| ] | |
| [tasks.swift-generate-manifest.env] | |
| SPM_URL = { value = "https://dummy.com", condition = { env_not_set = ["SPM_URL"] } } | |
| SPM_CHECKSUM = { value = "1234", condition = { env_not_set = ["SPM_CHECKSUM"] } } | |
| [tasks.swift-generate-manifest] | |
| private = true | |
| install_crate = { crate_name = "tera-cli", binary = "tera", test_arg = "--version" } | |
| script_runner = "@shell" | |
| script = """ | |
| tera --env --file ${SUPPORT_DIR}/${LANG}/Package.swift.tera > ${SPM_NAME}/Package.swift | |
| tera --env --file ${SUPPORT_DIR}/${LANG}/[email protected] > ${SPM_NAME}/[email protected] | |
| """ | |
| [tasks.swift-move-to-packages] | |
| private = true | |
| script_runner = "@shell" | |
| script = """ | |
| # Patch the generated XCFramework and Swift sources to match the framework name | |
| # This ensures compatibility with Swift 6.2+ which requires the module name to match the framework name | |
| FFI_NAME="livekit_uniffiFFI" | |
| # 1. Rename header files inside the XCFramework | |
| find "${SPM_NAME}/${XCFRAMEWORK_NAME}.xcframework" -name "${FFI_NAME}.h" -exec bash -c 'mv "$1" "$(dirname "$1")/'"${XCFRAMEWORK_NAME}"'.h"' _ {} \\; | |
| # 2. Patch the modulemaps inside the XCFramework | |
| find "${SPM_NAME}/${XCFRAMEWORK_NAME}.xcframework" -name "module.modulemap" -exec sed -i '' "s/${FFI_NAME}/${XCFRAMEWORK_NAME}/g" {} + | |
| # 3. Patch the generated Swift sources | |
| find "${SPM_NAME}/Sources" -name "*.swift" -exec sed -i '' "s/${FFI_NAME}/${XCFRAMEWORK_NAME}/g" {} + | |
| out_dir="${PACKAGES_DIR}/${LANG}" | |
| rm -rf "${out_dir}" | |
| mkdir -p "${out_dir}" | |
| mv "${SPM_NAME}" "${out_dir}" | |
| """ | |
| [tasks.swift-verify-spm-env] | |
| private = true | |
| condition = { profiles = ["release"] } | |
| script_runner = "@shell" | |
| script = """ | |
| if [ "${SPM_URL}" = "https://dummy.com" ] || [ "${SPM_CHECKSUM}" = "1234" ]; then | |
| echo "SPM_URL/SPM_CHECKSUM must be set for release" >&2 | |
| exit 1 | |
| fi | |
| """ | |
| [tasks.swift-package-flow] | |
| private = true | |
| dependencies = [ | |
| "swift-xcframework", | |
| "swift-verify-spm-env", | |
| "swift-generate-manifest", | |
| "swift-move-to-packages" | |
| ] |
🤖 Prompt for AI Agents
In `@livekit-uniffi/Makefile.toml` around lines 281 - 324, The task config
currently hard-sets SPM_URL and SPM_CHECKSUM in
[tasks.swift-generate-manifest.env], which unconditionally overrides CI-provided
values; remove the unconditional env entries and instead handle defaults and
validation inside the swift-generate-manifest script: in the script for the task
(swift-generate-manifest) only assign a dummy default to SPM_URL/SPM_CHECKSUM
when the env var is empty (e.g. if [ -z "$SPM_URL" ]; then
SPM_URL="https://dummy.com"; fi), and after generating the manifests fail fast
in release CI (detect via CI flag or BUILD_TYPE) if the values are still the
dummy placeholders (exit 1 with an error message). Update references in the
script and keep the unique symbols SPM_URL, SPM_CHECKSUM, and task names
swift-generate-manifest and swift-move-to-packages to locate the changes.
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.