-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Add benchmarks and compile_fail
tests back to workspace
#16858
Add benchmarks and compile_fail
tests back to workspace
#16858
Conversation
Custom profiles must be in the workspace root, so it was raising a warning. I opted to delete it entirely, since I'm not sure LTO is a good idea for benchmarks anyways.
I take it the RA bug you mentioned is now fixed? |
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.
Yay! And good comments too.
No, not yet unfortunately, and it's also surfaced in the CI failure. There are a few solutions, but I don't like any of them:
I think this is an underlying symptom of a larger problem: feature flags are difficult to use correctly and consistently. We don't have a system for defining how and when they should be made, and it's difficult to detect when they're enabled in our examples because we tunnel features through |
This comment was marked as outdated.
This comment was marked as outdated.
This reverts commit e4c29fb.
I hate this hack, but it's the only one that seems to work.
…#16858) # Objective - Our benchmarks and `compile_fail` tests lag behind the rest of the engine because they are not in the Cargo workspace, so not checked by CI. - Fixes bevyengine#16801, please see it for further context! ## Solution - Add benchmarks and `compile_fail` tests to the Cargo workspace. - Fix any leftover formatting issues and documentation. ## Testing - I think CI should catch most things! ## Questions <details> <summary>Outdated issue I was having with function reflection being optional</summary> The `reflection_types` example is failing in Rust-Analyzer for me, but not a normal check. ```rust error[E0004]: non-exhaustive patterns: `ReflectRef::Function(_)` not covered --> examples/reflection/reflection_types.rs:81:11 | 81 | match value.reflect_ref() { | ^^^^^^^^^^^^^^^^^^^ pattern `ReflectRef::Function(_)` not covered | note: `ReflectRef<'_>` defined here --> /Users/bdeep/dev/bevy/bevy/crates/bevy_reflect/src/kind.rs:178:1 | 178 | pub enum ReflectRef<'a> { | ^^^^^^^^^^^^^^^^^^^^^^^ ... 188 | Function(&'a dyn Function), | -------- not covered = note: the matched value is of type `ReflectRef<'_>` help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown | 126 ~ ReflectRef::Opaque(_) => {}, 127 + ReflectRef::Function(_) => todo!() | ``` I think it is because the following line is feature-gated: https://github.com/bevyengine/bevy/blob/cc0f6a8db43581755bd84302072c7b97ea51bc0f/examples/reflection/reflection_types.rs#L117-L122 My theory for why this is happening is because the benchmarks enabled `bevy_reflect`'s `function` feature, which gets merged with the rest of the features when RA checks the workspace, but the `#[cfg(...)]` gate in the example isn't detecting it: https://github.com/bevyengine/bevy/blob/cc0f6a8db43581755bd84302072c7b97ea51bc0f/benches/Cargo.toml#L19 Any thoughts on how to fix this? It's not blocking, since the example still compiles as normal, but it's just RA and the command `cargo check --workspace --all-targets` appears to fail. </summary>
Objective
compile_fail
tests lag behind the rest of the engine because they are not in the Cargo workspace, so not checked by CI.Solution
compile_fail
tests to the Cargo workspace.Testing
Questions
Outdated issue I was having with function reflection being optional
The
reflection_types
example is failing in Rust-Analyzer for me, but not a normal check.I think it is because the following line is feature-gated:
bevy/examples/reflection/reflection_types.rs
Lines 117 to 122 in cc0f6a8
My theory for why this is happening is because the benchmarks enabled
bevy_reflect
'sfunction
feature, which gets merged with the rest of the features when RA checks the workspace, but the#[cfg(...)]
gate in the example isn't detecting it:bevy/benches/Cargo.toml
Line 19 in cc0f6a8
Any thoughts on how to fix this? It's not blocking, since the example still compiles as normal, but it's just RA and the command
cargo check --workspace --all-targets
appears to fail.