Skip to content

Conversation

@atlv24
Copy link
Contributor

@atlv24 atlv24 commented Aug 25, 2025

Objective

  • We currently specify transitive feature dependencies in two places: bevy and bevy_internal Cargo.tomls
  • This means they get out of sync often, accumulate unnecessary duplication, and sometimes forget certain transitive deps.

Solution

  • Standardize on bevy_internal. Why: this makes it impossible to use it incorrectly if you depend on bevy_internal directly for some reason. If we standardized on bevy Cargo.toml holding these, it would mean that they could be bypassed by depending on bevy_internal. Not sure why someone would do that, but this feels right.
  • Move the few transitive feature dependency specifications that are still in bevy to bevy_internal
  • clean up a lot of duplicates
  • add a few missing dependencies
  • add top level bevy_mesh, bevy_camera, bevy_light, and bevy_shader features.

Testing

  • this stuff is hard to test automatically or comprehensively. Add default_no_render #20741 might make it easy to have a no-render test suite we can maintain coverage for, but other than that its just manual verification.

…nal and clean up duplicates and add missing dependencies
@atlv24 atlv24 added C-Dependencies A change to the crates that Bevy depends on S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 25, 2025
@alice-i-cecile alice-i-cecile added A-Cross-Cutting Impacts the entire engine X-Contentious There are nontrivial implications that should be thought through labels Aug 25, 2025
@mockersf
Copy link
Member

mostly ok, but feels strange to have bevy_light, bevy_camera and bevy_mesh in optional features

@atlv24
Copy link
Contributor Author

atlv24 commented Aug 25, 2025

Yeah, I noticed that too. It's autogenerated by a build script though, and there seem to already be other wrongly optional features there (for example bevy_image). If i move it manually to default, CI fails. I'm gonna look into why the build script is doing weird things at some point. For now what we could do is list them as default features, so that they appear in the correct place, but its not really necessary because they're enabled transitively.

@atlv24 atlv24 removed the X-Contentious There are nontrivial implications that should be thought through label Aug 25, 2025
Copy link
Member

@tychedelia tychedelia left a comment

Choose a reason for hiding this comment

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

Seems a lot cleaner / more straightforward!

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 26, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 26, 2025
Merged via the queue into bevyengine:main with commit c24491e Aug 26, 2025
38 checks passed
Zeophlite pushed a commit to Zeophlite/bevy that referenced this pull request Aug 27, 2025
# Objective

- We currently specify transitive feature dependencies in two places:
bevy and bevy_internal Cargo.tomls
- This means they get out of sync often, accumulate unnecessary
duplication, and sometimes forget certain transitive deps.

## Solution

- Standardize on bevy_internal. Why: this makes it impossible to use it
incorrectly if you depend on bevy_internal directly for some reason. If
we standardized on bevy Cargo.toml holding these, it would mean that
they could be bypassed by depending on bevy_internal. Not sure why
someone would do that, but this feels right.
- Move the few transitive feature dependency specifications that are
still in bevy to bevy_internal
- clean up a lot of duplicates
- add a few missing dependencies
- add top level bevy_mesh, bevy_camera, bevy_light, and bevy_shader
features.


## Testing

- this stuff is hard to test automatically or comprehensively. bevyengine#20741
might make it easy to have a no-render test suite we can maintain
coverage for, but other than that its just manual verification.
@atlv24 atlv24 deleted the ad/feature-cleanup branch September 9, 2025 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Cross-Cutting Impacts the entire engine C-Dependencies A change to the crates that Bevy depends on S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants