Compile all features in ci - #1393
Merged
Merged
Conversation
Adds a job that runs `cargo hack --feature-powerset --depth 2 check` over `livekit`, `livekit-api`, and the ten workspace crates beneath them: 209 feature combinations in total. Feature bugs do not show up in the default-feature build — an optional `dep:` used unconditionally, or a `?/` forward that silently no-ops, only breaks for callers who pick a particular feature set. Nothing else in CI exercises those paths today. All 209 combinations currently pass, so this starts green. Depth 2 is a deliberate ceiling. It covers pairwise feature interactions, where nearly all such bugs live, and keeps the job affordable: `livekit` alone would go from 67 combinations to 232 at depth 3. Runs on one platform (linux arm64) because feature resolution does not vary by target — the point is to exercise the feature graph, not the target matrix. Scheduled nightly rather than per-PR, plus on Cargo.toml and Cargo.lock changes, which are what can alter feature resolution. `target/` is deliberately not cached so the reported time reflects a cold run, and the job prints the powerset time next to a plain default-feature `cargo check` for comparison. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Expands the feature-combination job from linux arm64 to the same eleven targets builds.yml covers, and drops the nightly schedule plus the Cargo.toml path filter so it runs on every commit. Measured at 14 min end to end on ubuntu-24.04-arm, which is on par with the existing build and test jobs, so per-commit is affordable. Matrix jobs run in parallel, so the cost is the slowest target rather than the sum of eleven. One platform was never enough. Feature *resolution* does not vary by target, but what the resolved features then compile does: cfg-gated code, the webrtc-sys C++ bridge and the per-platform TLS backends all differ, so a combination can be green on linux and broken on android or iOS. That is exactly the class of bug this job exists to catch. Android goes through cargo-ndk, which supplies the NDK clang the webrtc-sys C++ bridge needs; it execs `cargo <args>`, so it wraps `hack` the same way it wraps `build`. fail-fast is off so one broken target does not hide the other ten, and a concurrency group cancels superseded runs rather than leaving eleven jobs per push running. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All three Android targets failed the same 8 of 209 combinations: every one that enables plain `native-tls`. The cause is openssl-sys, which cannot find an OpenSSL installation to link against when cross-compiling to Android and fails its build script outright. This is not a feature-wiring bug, so there is nothing in the SDK to fix. Android is a rustls-only platform here — ffi-builds.yml already ships all three Android targets as `rustls-tls-webpki-roots`, so a `native-tls` Android build is not a configuration anyone ships. Excluding the feature takes Android from 209 combinations to 172. It keeps `native-tls-vendored` covered (33 combinations), since that builds OpenSSL from source and works, and the excluded combinations are still exercised on the eight non-Android targets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…werset
Splits feature checking by where combinatorial coverage earns its cost,
as an alternative to the full 209-combination powerset:
- Leaf crates keep the full `--feature-powerset --depth 2` (83
combinations). Their feature sets are small, so exhaustive coverage
is cheap, and they are where the `dep:`/`?/` plumbing bugs live.
- livekit, livekit-api, livekit-ffi and livekit-uniffi get 26
hand-picked combinations. The powerset over the top crates is where
the time went — `livekit` alone is 65 of 209 combinations and ~40%
of wall time — and most of those are combinations nobody can
select: two TLS backends at once, or an internal `__lk-*` feature
paired with something unrelated. Every entry here is drawn from
what the manifests document or what ffi-builds.yml ships.
This also covers livekit-ffi and livekit-uniffi, which the powerset
workflow does not touch at all.
The tradeoff is deliberate and narrows coverage on the top crates: a
feature pair that only breaks in one of the dropped combinations will
not be caught. Both workflows run for now so the numbers can be
compared on the same commit; one should be deleted once that is
settled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…erset Measured cold on a 14-core machine, checking the same crates: full powerset (209 combos) 359s leaf powerset + curated livekit/api 242s -33% the same, plus livekit-ffi/livekit-uniffi 345s -4% The first version of the curated workflow was no faster than the powerset it was meant to replace — on CI it was slower on 9 of 11 targets. Two reasons, both visible above. livekit-ffi and livekit-uniffi were the larger one: 103s of a 345s budget for two crates, because every TLS change rebuilds livekit underneath livekit-ffi, and the powerset workflow never built them at all. They move to the nightly job here. Their shipped configurations are still covered per-commit — builds.yml builds the whole workspace on every PR, and ffi-builds.yml builds each shipped per-platform TLS variant on push to main — so what nightly adds back is livekit-ffi's non-default TLS variants on PRs. The other was a misread on my part: `livekit` is 65 of 209 combinations and ~40% of wall time, but those combinations are mostly ~2s cache hits. The cost attributed to them was the one-time cold build of the dependency graph, which does not go away when the combinations do — it just moves into the leaf phase. The leaf-only powerset is already 55-80% of the full 209. So: feature-combinations-curated.yml runs per-commit on all eleven targets (leaf powerset + 17 documented livekit/livekit-api combinations), and feature-combinations.yml keeps the exhaustive 209 plus the ffi crates, nightly, as the backstop for what the subset skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1egoman
commented
Sep 2, 2026
Comment on lines
+64
to
+74
| LEAF_PACKAGES: >- | ||
| -p livekit-signaling | ||
| -p livekit-net | ||
| -p livekit-region | ||
| -p livekit-protocol | ||
| -p livekit-datatrack | ||
| -p livekit-data-stream | ||
| -p livekit-common | ||
| -p libwebrtc | ||
| -p webrtc-sys | ||
| -p livekit-token |
Contributor
Author
There was a problem hiding this comment.
Note for reviewers - Is this a good list of "leaf packages"? Is there a better way to determine these programatically? If not, I may also need to add a note to agents.md to keep this list up to date.
Contributor
Author
There was a problem hiding this comment.
(Added the AGENTS.md note in cc27308 - still would be interested in alternative solutions though!)
Contributor
There was a problem hiding this comment.
I suspect when you tried this originally without limiting the package list all examples were also being compiled (since they are members of the workspace). How about doing powerset with the addition of the --ignore-private flag? This excludes any crate with publish = false in its manifest.
Contributor
Author
There was a problem hiding this comment.
I think moving to the Cargo.toml driven approach, this becomes less of an issue since packages which don't specify either powerset or curated will be excluded, so I will leave it as is.
Comment on lines
+213
to
+243
| # --- livekit ------------------------------------------------------- | ||
| livekit| | ||
| livekit|--no-default-features | ||
| livekit|--no-default-features --features native | ||
| livekit|--no-default-features --features native,native-tls | ||
| livekit|--no-default-features --features native,native-tls-vendored | ||
| livekit|--no-default-features --features native,rustls-tls-native-roots | ||
| livekit|--no-default-features --features native,rustls-tls-webpki-roots | ||
| livekit|--no-default-features --features native,rustls-tls-webpki-roots,glib-main-loop | ||
| livekit|--no-default-features --features native,__lk-internal | ||
| # --- livekit-api --------------------------------------------------- | ||
| livekit-api| | ||
| livekit-api|--no-default-features | ||
| livekit-api|--no-default-features --features access-token | ||
| livekit-api|--no-default-features --features webhooks | ||
| livekit-api|--no-default-features --features services,native-tls | ||
| livekit-api|--no-default-features --features services,rustls-tls-webpki-roots | ||
| livekit-api|--no-default-features --features services,rustls-tls-native-roots | ||
| livekit-api|--no-default-features --features signal-client-native,rustls-tls-webpki-roots | ||
| livekit-api|--features rustls-tls-webpki-roots | ||
| # livekit-ffi and livekit-uniffi are deliberately NOT here. Measured | ||
| # cold, their 8 combinations cost 103s of a 345s total — 30% of the | ||
| # budget for two crates, because each TLS change rebuilds livekit | ||
| # underneath livekit-ffi. Dropping them takes the job from 345s to | ||
| # 242s, which is what makes per-commit affordable. Their shipped | ||
| # configurations are still covered: builds.yml builds the whole | ||
| # workspace (livekit-ffi at default features) on every PR, and | ||
| # ffi-builds.yml builds every shipped per-platform TLS variant on | ||
| # push to main. The residual gap is livekit-ffi's non-default TLS | ||
| # variants on PRs, which the nightly full powerset covers. | ||
| EOF |
Contributor
Author
There was a problem hiding this comment.
Note to reviewers - here is the list of "interesting" feature combinations currently being tested.
I could probably cut this down even further? access-token should be no longer used.
The curated per-commit workflow is the better trade, so the nightly
209-combination job is not worth the runner time. Verified green on all
eleven targets (run 33651960004) at 7.7-17.4 min per job, with the
feature-checking steps themselves down ~30% against the full powerset.
Removing it gives up two things, both deliberately:
- The exhaustive backstop. A feature pair that only breaks in one of
the combinations the curated list skips is now caught nowhere. The
header comment records the one-liner to run the full 209 by hand
when a feature bug needs chasing.
- Feature-combination coverage for livekit-ffi and livekit-uniffi,
which only ever lived in this workflow. Their shipped configurations
are still built — builds.yml builds the workspace on every PR, and
ffi-builds.yml builds each shipped per-platform TLS variant on push
to main — so what is lost is livekit-ffi's non-default TLS variants
on PRs. That cost 103s of a 345s budget to cover, which is what
kept it out of the per-commit job in the first place.
Also folds the rationale that lived in the deleted file (the Android
openssl-sys exclusion) into the surviving workflow rather than leaving
the comment pointing at a file that no longer exists.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both halves of feature-combinations-curated.yml are maintained by hand,
and neither fails loudly when it goes stale:
- A new feature on livekit or livekit-api is invisible to the job
until it is added to the curated COMBOS list, because those two
crates get a hand-picked list rather than a powerset.
- A new workspace crate is invisible until it is added to
LEAF_PACKAGES, because cargo-hack only varies the features of
packages named with -p. The crate still gets built as a dependency,
which makes it easy to assume it is covered when it is not.
Also records the constraints that are not obvious from reading the
workflow: why depth stays at 2, why openssl-sys features have to be
excluded on Android, why livekit-ffi and livekit-uniffi are left out,
and that --print-command-list enumerates combinations without building.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1egoman
marked this pull request as ready for review
September 2, 2026 18:50
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Both from review feedback on #1393, and both real gaps. livekit-token-source is a published crate (0.1.2) whose five features are bare pass-throughs to livekit-net — exactly the forwarding this job exists to check — and it was unchecked. It was missed because the scope was framed as "livekit and livekit-api plus what they depend on", and livekit-token-source is a sibling rather than a dependency: nothing in the SDK path pulls it in, only examples/token_source. The header now states the rule as every publishable workspace crate that has features, and calls out the sibling case, so the next such crate is not missed the same way. Leaf combinations go from 83 to 104, which costs 9s: the combinations are TLS variants already compiled elsewhere in the job. livekit-api's `services,native-tls-vendored` was the one TLS backend of the four not covered, while the livekit equivalent was — an asymmetric omission rather than a deliberate one. `services` is what enables dep:reqwest, so this is the combination where the `reqwest?/native-tls-vendored` forward actually becomes live. Costs 1s, since livekit already compiles vendored OpenSSL earlier in the run. Deliberately not fixing yuv-sys, whose `jpeg` feature is also unchecked: it would pull the libyuv C build into a job that is otherwise pure cargo check, needs system libjpeg on all eleven targets, and is not on the client SDK path. Verified both locally before pushing: token-source's 21 combinations and the new livekit-api combination all pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ladvoc
approved these changes
Sep 4, 2026
1egoman
requested review from
MaxHeimbrock,
cloudwebrtc,
jhugman and
lukasIO
as code owners
September 8, 2026 21:06
1egoman
added
the
internal
jq.exe uses native windows line endings which the script wasn't taking into account properly.
1egoman
force-pushed
the
add-all-features-complication
branch
2 times, most recently
from
September 9, 2026 20:35
e9c27d1 to
7486053
Compare
Contributor
Author
|
I've had to do some fighting with ci to get it to NOT update protobufs. I don't want this change to include any rust, as it should be purely a ci / Therefore because I cancelled the "generate protobufs" job, ci won't pass when I merge this, but it's a false negative. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Today, there is nothing verifying that all feature combinations build. This pull request aims to fix this.
Initial approach
Initially, I aimed to use cargo-hack with
--feature-powersetto build all feature combinations across all crates. This ended up being far too slow, though:Second approach: targeted feature subset
I want to run this on every commit so I looked a bit at how other popular crates do this - tokio, hyper, etc. They seem to test a fixed set of features each ci run rather than all combinations to keep the ci run time down. However, somewhat suprisingly, it did not:
The big reason why it turned out was that I hadn't realized, but initial test wasn't actually building
livekit-ffiorlivekit-uniffi. So had those been included, the run time would have been even slower.Approach three: approach two but omitting
livekit-ffiSo after rerunning approach two, only this time without
livekit-ffi, the check times went way, way down:The slowest (windows) is 16 minutes, which is about as long as the longest ci build takes today. So this should not add any additional time to ci.
This third approach is what I am suggesting and what this pull request contains. All three have pros and cons though, so if others think a different approach is better I'd be interested in hearing your perspective.
TODO
add-all-features-complicationpush trigger