-
Notifications
You must be signed in to change notification settings - Fork 138
put examples in root Cargo workspace #731
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
Conversation
|
@Be-ing, it looks like this change is causing the test jobs to fail. |
|
yep will look into that |
7089937 to
e82f26d
Compare
|
That took a bit more work than I anticipated, but tests are passing now. |
ladvoc
left a 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.
Overall, this looks great—just a few minor comments.
1ca712a to
e7aa07d
Compare
|
Everything looks good now, except build is failing on Android. From a quick look at the logs, it looks like it isn't related to this PR—I will address that so we can get this merged. |
|
That might be related to this PR if this PR accidentally changed the Cargo feature for TLS that gets selected on Android. |
cfde24f to
4b80220
Compare
instead of their own workspace. This avoids rebuilding dependencies and wasting space with multiple target directories.
This is required now that the examples are in the root workspace. Otherwise, each seperate example would enable conflicting features on livekit-runtime. Avoid this by using Cargo features in one crate to select between runtimes.
like all the other examples
There's no need to specify these in the workspace's Cargo.toml. Cargo reads the versions from each crate's Cargo.toml.
Different examples used different backends, which doesn't work great in a workspace.
e3b4ba1 to
b2f4956
Compare
libc++abi does not need to be manually linked. Doing so breaks linking when failing to find pthread.
e71f284 to
123cb2a
Compare
It fails to link for aarch64 Android:
= note: ld.lld: error: undefined symbol: __arm_tpidr2_save
>>> referenced by rotate_sme.cc
>>> rotate_sme.o:(TransposeWxH_SME) in archive /home/runner/work/rust-sdks/rust-sdks/target/aarch64-linux-android/release/deps/libwebrtc_sys-8a55d18259959b0a.rlib
123cb2a to
da4a060
Compare
|
I got Android builds to work again. The wgpu_room example fails to link on aarch64 Android with a strange error: I don't know what to make of that error; it might take a deep dive into how libwebrtc.a is built for Android (which I don't plan to do) to resolve. I think just skipping building that one crate is fine for now. Phew, that was a hairy yak to shave 🪒 |
|
Agreed, skipping that example for Android is fine. |
ladvoc
left a 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.
LGTM ✅
|
Hey @Be-ing, if you're up for another contribution, we have a new CI build issue for Android—it appears to be different than the one you fixed on this PR: Example run: https://github.com/livekit/rust-sdks/actions/runs/18541840496 |
|
I don't know what might have caused that considering that CI for the commit before it passed and the commit that failed to build doesn't seem to touch anything build related... |
Removed by #731, but required to publish to crates.io.
instead of their own workspace. This avoids rebuilding dependencies and wasting space with multiple target directories.