Skip to content

put examples in root Cargo workspace - #731

Merged
ladvoc merged 15 commits into
livekit:mainfrom
Be-ing:examples_workspace
Oct 14, 2025
Merged

put examples in root Cargo workspace#731
ladvoc merged 15 commits into
livekit:mainfrom
Be-ing:examples_workspace

Conversation

@Be-ing

@Be-ing Be-ing commented Oct 9, 2025

Copy link
Copy Markdown
Contributor

instead of their own workspace. This avoids rebuilding dependencies and wasting space with multiple target directories.

@ladvoc

ladvoc commented Oct 10, 2025

Copy link
Copy Markdown
Contributor

@Be-ing, it looks like this change is causing the test jobs to fail.

@Be-ing

Be-ing commented Oct 10, 2025

Copy link
Copy Markdown
Contributor Author

yep will look into that

@Be-ing
Be-ing force-pushed the examples_workspace branch 2 times, most recently from 7089937 to e82f26d Compare October 14, 2025 04:28
@Be-ing

Be-ing commented Oct 14, 2025

Copy link
Copy Markdown
Contributor Author

That took a bit more work than I anticipated, but tests are passing now.

@ladvoc
ladvoc self-requested a review October 14, 2025 06:49

@ladvoc ladvoc left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread .github/workflows/tests.yml
Comment thread examples/basic_room/src/main.rs
Comment thread examples/send_bytes/Cargo.toml
Comment thread Cargo.toml
@Be-ing
Be-ing force-pushed the examples_workspace branch 6 times, most recently from 1ca712a to e7aa07d Compare October 14, 2025 07:56
@ladvoc

ladvoc commented Oct 14, 2025

Copy link
Copy Markdown
Contributor

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.

@ladvoc ladvoc self-assigned this Oct 14, 2025
@Be-ing

Be-ing commented Oct 14, 2025

Copy link
Copy Markdown
Contributor Author

That might be related to this PR if this PR accidentally changed the Cargo feature for TLS that gets selected on Android.

@Be-ing
Be-ing force-pushed the examples_workspace branch 5 times, most recently from cfde24f to 4b80220 Compare October 14, 2025 18:50
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.
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.
@Be-ing
Be-ing force-pushed the examples_workspace branch from e3b4ba1 to b2f4956 Compare October 14, 2025 21:59
libc++abi does not need to be manually linked. Doing so breaks
linking when failing to find pthread.
@Be-ing
Be-ing force-pushed the examples_workspace branch 3 times, most recently from e71f284 to 123cb2a Compare October 14, 2025 22:58
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
@Be-ing
Be-ing force-pushed the examples_workspace branch from 123cb2a to da4a060 Compare October 14, 2025 23:07
@Be-ing

Be-ing commented Oct 14, 2025

Copy link
Copy Markdown
Contributor Author

I got Android builds to work again. The wgpu_room example fails to link on aarch64 Android with a strange error:

  = 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

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 🪒

@ladvoc
ladvoc self-requested a review October 14, 2025 23:44
@ladvoc

ladvoc commented Oct 14, 2025

Copy link
Copy Markdown
Contributor

Agreed, skipping that example for Android is fine.

@ladvoc ladvoc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✅

@ladvoc
ladvoc merged commit f471d99 into livekit:main Oct 14, 2025
16 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 14, 2025
@Be-ing
Be-ing deleted the examples_workspace branch October 15, 2025 02:07
This was referenced Oct 15, 2025
@ladvoc

ladvoc commented Oct 16, 2025

Copy link
Copy Markdown
Contributor

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:

cargo:warning=Could not find directory of OpenSSL installation, and this `-sys` crate cannot proceed without this knowledge. If OpenSSL is installed and this crate had trouble finding it, you can set the `OPENSSL_DIR` environment variable for the compilation process. See stderr section below for further information.

Example run: https://github.com/livekit/rust-sdks/actions/runs/18541840496

@Be-ing

Be-ing commented Oct 16, 2025

Copy link
Copy Markdown
Contributor Author

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...

ladvoc added a commit that referenced this pull request Oct 22, 2025
Removed by #731, but required to publish to crates.io.
@ladvoc ladvoc mentioned this pull request Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants