-
Notifications
You must be signed in to change notification settings - Fork 137
macOS and iOS: move -ObjC flag to build.rs
#847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The `livekit` crate poses a devex challenge for those using this crate on macOS and iOS due to developers having to set these flags. Placing this flag in the .config.toml only works if you're building the crate from the `rust-sdks` repo but does not get added if a programmer has a rust project that depends on the livekit rust-sdk crate. A better alternative would be to place the flags within the build.rs of the `livekit` crate. By doing so, developers that depend on the livekit crate do not need to worry about setting this flag for macOS and iOS. Fix this by moving this flag from the `config.toml` to the `build.rs`.
📝 WalkthroughWalkthroughThis change relocates the Objective-C linking flag configuration from static Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@livekit/build.rs`:
- Around line 1-13: Update the license header in livekit/build.rs to match the
repo's standard Rust header template: replace the current block comment with the
exact multi-line header used across other Rust files including the SPDX
identifier (SPDX-License-Identifier: Apache-2.0), the canonical Apache-2.0
notice text and the correct year/owner formatting (LiveKit, Inc.). Ensure the
header text and SPDX line exactly match other Rust source files so Hawkeye
recognizes it.
- Around line 15-20: The build script currently emits cargo:rustc-link-arg
inside main (in build.rs) which does not propagate to dependent crates; update
the packaging by adding links = "livekit" to Cargo.toml and change the build
script (the main function in build.rs) to print a custom metadata key instead
(e.g., println!("cargo:objc_link_arg=-ObjC")) so downstream crates can pick it
up via the DEP_* env var, then document that dependents must read
DEP_LIVEKIT_OBJC_LINK_ARG in their build.rs and re-emit it as
cargo:rustc-link-arg (or re-add the arg themselves); optionally note the
alternative of using cargo:rustc-link-arg-bin or cargo:rustc-link-arg-cdylib if
only final binaries/cdylibs need the flag.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
.cargo/config.tomllivekit/build.rs
💤 Files with no reviewable changes (1)
- .cargo/config.toml
🧰 Additional context used
🪛 GitHub Actions: License Check
livekit/build.rs
[error] 1-1: Hawkeye check failed: Found header missing in files: ["/home/runner/work/rust-sdks/rust-sdks/livekit/build.rs"]
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Build (x86_64-apple-darwin)
- GitHub Check: Build (aarch64-apple-ios-sim)
- GitHub Check: Build (x86_64-linux-android)
- GitHub Check: Build (aarch64-linux-android)
- GitHub Check: Build (aarch64-apple-darwin)
- GitHub Check: Build (aarch64-unknown-linux-gnu)
- GitHub Check: Build (armv7-linux-androideabi)
- GitHub Check: Build (aarch64-pc-windows-msvc)
- GitHub Check: Build (x86_64-unknown-linux-gnu)
- GitHub Check: Build (aarch64-apple-ios)
- GitHub Check: Build (x86_64-pc-windows-msvc)
- GitHub Check: Test (x86_64-apple-darwin)
- GitHub Check: Test (x86_64-pc-windows-msvc)
- GitHub Check: Test (x86_64-unknown-linux-gnu)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Copyright 2026 LiveKit, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix license header format to satisfy Hawkeye check.
CI reports the header is missing for this file; the current header likely doesn’t match the repo’s required template. Please align it with the standard header used in other Rust files (often includes an SPDX line and exact wording/year format).
📝 Possible minimal adjustment (if SPDX is expected)
+// SPDX-License-Identifier: Apache-2.0
// Copyright 2026 LiveKit, Inc.📝 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.
| // Copyright 2026 LiveKit, Inc. | |
| // | |
| // Licensed under the Apache License, Version 2.0 (the "License"); | |
| // you may not use this file except in compliance with the License. | |
| // You may obtain a copy of the License at | |
| // | |
| // http://www.apache.org/licenses/LICENSE-2.0 | |
| // | |
| // Unless required by applicable law or agreed to in writing, software | |
| // distributed under the License is distributed on an "AS IS" BASIS, | |
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |
| // See the License for the specific language governing permissions and | |
| // limitations under the License. | |
| // SPDX-License-Identifier: Apache-2.0 | |
| // Copyright 2026 LiveKit, Inc. | |
| // | |
| // Licensed under the Apache License, Version 2.0 (the "License"); | |
| // you may not use this file except in compliance with the License. | |
| // You may obtain a copy of the License at | |
| // | |
| // http://www.apache.org/licenses/LICENSE-2.0 | |
| // | |
| // Unless required by applicable law or agreed to in writing, software | |
| // distributed under the License is distributed on an "AS IS" BASIS, | |
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |
| // See the License for the specific language governing permissions and | |
| // limitations under the License. |
🧰 Tools
🪛 GitHub Actions: License Check
[error] 1-1: Hawkeye check failed: Found header missing in files: ["/home/runner/work/rust-sdks/rust-sdks/livekit/build.rs"]
🤖 Prompt for AI Agents
In `@livekit/build.rs` around lines 1 - 13, Update the license header in
livekit/build.rs to match the repo's standard Rust header template: replace the
current block comment with the exact multi-line header used across other Rust
files including the SPDX identifier (SPDX-License-Identifier: Apache-2.0), the
canonical Apache-2.0 notice text and the correct year/owner formatting (LiveKit,
Inc.). Ensure the header text and SPDX line exactly match other Rust source
files so Hawkeye recognizes it.
| fn main() { | ||
| match std::env::var("CARGO_CFG_TARGET_OS").as_deref() { | ||
| Ok("macos") | Ok("ios") => println!("cargo:rustc-link-arg=-ObjC"), | ||
| Ok(_) => return, | ||
| Err(e) => panic!("{e}"), | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Cargo cargo:rustc-link-arg propagate dependency build script downstream crates
💡 Result:
Short answer: No — cargo:rustc-link-arg affects only the crate whose build script emitted it; it is not propagated to downstream (dependent) crates. See the Cargo build-script docs for rustc-link-arg (applies when building that package's supported targets) and the metadata section (only key=value metadata is passed to immediate dependents — not rustc-link-arg instructions). [1][2]
Sources:
- cargo build-script reference — rustc-link-arg description. [1]
- cargo build-script reference — build-script metadata passed to dependents (only key=value, immediate dependents). [2]
🌐 Web query:
Cargo library crate pass linker flags to dependents -ObjC
💡 Result:
Short answer: You can't force-add a generic linker flag like -ObjC from a library crate so it automatically applies to downstream dependents. Cargo's build-script linker-arg outputs are only applied to the crate being built (and to certain target kinds), not propagated to arbitrary dependents. (rustwiki.org)
Options/workarounds:
- Document the requirement and ask downstream crates (or the final binary) to add the flag in their build script: println!("cargo:rustc-link-arg=-ObjC");. This is the simplest reliable approach. (docs.rs)
- If you control the final crate type (e.g., produce a cdylib or the final binary), emit the appropriate rustc-link-arg variant from your build script (cargo:rustc-link-arg-cdylib or rustc-link-arg when building the final binary). Those apply to those target kinds. (docs.rs)
- Use a small helper crate that exposes a build-time configuration (via its links field + a convention for propagating values) and have dependents call that helper from their build.rs to re-emit the link flags. Some projects use this pattern to surface native/link options to dependents. (Cargo does let you emit cfg/env values from a dependency which dependents can read and re-emit.) (docs.rs)
If you want, tell me whether the consumer is a binary, cdylib, or rlib and I’ll give the exact build.rs lines to emit.
Citations:
- 1: https://rustwiki.org/en/cargo/reference/build-scripts.html?utm_source=openai
- 2: https://docs.rs/cargo-build/latest/cargo_build/fn.rustc_link_arg.html?utm_source=openai
- 3: https://docs.rs/cargo-build/latest/cargo_build/fn.rustc_link_arg_cdylib.html?utm_source=openai
- 4: https://docs.rs/embuild/latest/embuild/build/struct.CfgArgs.html?utm_source=openai
🌐 Web query:
Cargo build script instructions propagation rust-link-arg library dependencies
💡 Result:
Short answer: Cargo does not automatically propagate cargo:rustc-link-arg (or other rustc-* instructions) from a library crate to its dependents. To have downstream crates receive linker args, expose them as build-script metadata (via package.links and/or custom metadata keys) and have dependents’ build scripts read DEP__* environment variables and re-emit the appropriate cargo:rustc-link-arg (or rustc-link-lib / rustc-link-search) for their own build.
Steps (minimal example):
- In the library crate (Cargo.toml):
- set a links name so Cargo will pass metadata:
[package]
links = "mylib" - in build.rs emit metadata keys:
println!("cargo:MY_LINKER_ARG=-Wl,--foo");
// and emit any rustc-link-lib / rustc-link-search for the library itself if needed:
println!("cargo:rustc-link-lib=static=mylib");
- In a dependent crate’s build.rs:
- read the env var created by Cargo: DEP_<UPPERCASE_LINKS>_MY_LINKER_ARG
let v = std::env::var("DEP_MYLIB_MY_LINKER_ARG").ok(); - if present, re-emit it as a rustc link arg for that crate:
println!("cargo:rustc-link-arg={}", v.unwrap());
This ensures the final binary/library being built gets the linker flag. (Metadata is only passed to immediate dependents, not transitive dependents.)
References: Cargo build-script docs (explains cargo:rustc-link-arg and metadata / DEP_* variables and links key). [1][2].
Sources:
[1] Cargo build scripts reference — rustc-link-arg / rustc-cdylib-link-arg.
[2] Cargo build scripts reference — package.links and DEP__* metadata.
cargo:rustc-link-arg does not propagate to dependent crates—the current implementation will not reach downstream users.
cargo:rustc-link-arg applies only to the crate whose build script emits it; Cargo does not forward it to dependents. To ensure -ObjC reaches downstream crates, use the links field + metadata pattern:
- Add
links = "livekit"toCargo.toml - Emit custom metadata from
build.rsinstead ofcargo:rustc-link-arg:println!("cargo:objc_link_arg=-ObjC"); - Document that downstream crates must read
DEP_LIVEKIT_OBJC_LINK_ARGin their build script and re-emit it ascargo:rustc-link-arg, or add it themselves if they directly link the native code.
Alternatively, if all dependents are final binaries/cdylibs, use cargo:rustc-link-arg-bin or cargo:rustc-link-arg-cdylib respectively (though these still apply only to that specific build, not transitively).
🤖 Prompt for AI Agents
In `@livekit/build.rs` around lines 15 - 20, The build script currently emits
cargo:rustc-link-arg inside main (in build.rs) which does not propagate to
dependent crates; update the packaging by adding links = "livekit" to Cargo.toml
and change the build script (the main function in build.rs) to print a custom
metadata key instead (e.g., println!("cargo:objc_link_arg=-ObjC")) so downstream
crates can pick it up via the DEP_* env var, then document that dependents must
read DEP_LIVEKIT_OBJC_LINK_ARG in their build.rs and re-emit it as
cargo:rustc-link-arg (or re-add the arg themselves); optionally note the
alternative of using cargo:rustc-link-arg-bin or cargo:rustc-link-arg-cdylib if
only final binaries/cdylibs need the flag.
The
livekitcrate poses a devex challenge for those using this crate on macOS and iOS due to developers having to set these flags. Placing this flag in the .config.toml only works if you're building the crate from therust-sdksrepo but does not get added if a programmer has a rust project that depends on the livekit rust-sdk crate. A better alternative would be to place the flags within the build.rs of thelivekitcrate. By doing so, developers that depend on the livekit crate do not need to worry about setting this flag for macOS and iOS.Fix this by moving this flag from the
config.tomlto thebuild.rs.Fixes: #795
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.