Skip to content

Allow partial support for bevy_log in no_std #18782

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

Merged
merged 4 commits into from
Apr 11, 2025

Conversation

bushrat011899
Copy link
Contributor

Objective

Solution

  • Moved LogPlugin into its own file gated behind a new tracing feature.
  • Used log instead of tracing where possible.
  • Exposed a new tracing feature in bevy which enables bevy_log/tracing.
  • Gated LogPlugin from DefaultPlugins on tracing feature.

Testing

  • CI

Migration Guide

  • If you were previously using bevy_log with default features disabled, enable the new std and tracing features.
  • If you were using bevy with the default features disabled, enable the new tracing feature.

Notes

Almost all of the diffs in this PR come from moving LogPlugin into its own file. This just makes the PR less noisy, since the alternative is excessive #[cfg(feature = "tracing")] directives all over the plugin.

@bushrat011899 bushrat011899 added C-Feature A new feature, making something new possible M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide A-Utils Utility functions and types X-Contentious There are nontrivial implications that should be thought through D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward O-Embedded Weird hardware and no_std platforms labels Apr 10, 2025
@bushrat011899 bushrat011899 added this to the 0.16 milestone Apr 10, 2025
@bushrat011899 bushrat011899 requested a review from Shatur April 10, 2025 00:02
Copy link
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Please review the instructions for writing migration guides, then expand or revise the content in the migration guides directory to reflect your changes.

@Shatur
Copy link
Contributor

Shatur commented Apr 10, 2025

Maybe depend on bevy_log for other internal Bevy crates? 🤔

@bushrat011899
Copy link
Contributor Author

Maybe depend on bevy_log for other internal Bevy crates? 🤔

Definitely could do that, but I'd save that for a follow-up, this is already pretty lengthy for a last-minute RC update.

@hukasu
Copy link
Contributor

hukasu commented Apr 10, 2025

i commented on the issue, but tracing seems to work on no_std, unless liballoc is a blocker
https://docs.rs/tracing/latest/tracing/#crate-feature-flags

Edit: liballoc seems to be just extern crate alloc; which is already use in many places

@bushrat011899
Copy link
Contributor Author

i commented on the issue, but tracing seems to work on no_std, unless liballoc is a blocker https://docs.rs/tracing/latest/tracing/#crate-feature-flags

Edit: liballoc seems to be just extern crate alloc; which is already use in many places

The issue is actually to do with no_atomic rather than no_std, but right now Bevy considers them synonymous. Trying to consider them as separate features further fragments the codebase for little gain, since getting no_atomic support in a no_std context is comparatively trivial.

I already have no_atomic PRs for tracing opened which would close the gap and allow tracing on all platforms (e.g., thumbv6m-none-eabi), just waiting for further review before they're merged.

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 10, 2025
Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

Ok I think this is the move for now, given the current approach. I think the "virality" of both std and web is a maintenance liability and an "ecosystem UX" liability. Needing to know what dependencies are "std" or "web" is something we should avoid thrusting on users (and ourselves) whenever possible.

I've written out a potential path forward here: #18799

@bushrat011899 bushrat011899 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-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Apr 10, 2025
@cart cart added this pull request to the merge queue Apr 10, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 10, 2025
Co-Authored-By: François Mockers <[email protected]>
@cart cart added this pull request to the merge queue Apr 11, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 11, 2025
@mockersf mockersf enabled auto-merge April 11, 2025 01:29
@mockersf mockersf added this pull request to the merge queue Apr 11, 2025
Merged via the queue into bevyengine:main with commit ac52cca Apr 11, 2025
36 checks passed
mockersf added a commit that referenced this pull request Apr 11, 2025
- Fixes #18781

- Moved `LogPlugin` into its own file gated behind a new `tracing`
feature.
- Used `log` instead of `tracing` where possible.
- Exposed a new `tracing` feature in `bevy` which enables
`bevy_log/tracing`.
- Gated `LogPlugin` from `DefaultPlugins` on `tracing` feature.

- CI

---

- If you were previously using `bevy_log` with default features
disabled, enable the new `std` and `tracing` features.
- If you were using `bevy` with the default features disabled, enable
the new `tracing` feature.

Almost all of the diffs in this PR come from moving `LogPlugin` into its
own file. This just makes the PR less noisy, since the alternative is
excessive `#[cfg(feature = "tracing")]` directives all over the plugin.

---------

Co-authored-by: François Mockers <[email protected]>
cart added a commit to cart/bevy that referenced this pull request Apr 11, 2025
cart added a commit to cart/bevy that referenced this pull request Apr 12, 2025
@cBournhonesque
Copy link
Contributor

cBournhonesque commented Apr 14, 2025

The move from log to tracing is a pretty big breaking change, right?
I was using

use bevy::prelude::*;
use tracing::*;

everywhere in my library for convenience, and now this causes conflicts because bevy's prelude switched from using tracing::debug! to log::debug!

Even if I enable the tracing feature, it only seems to add debug_span!, error_span!, etc. to the bevy prelude, not debug!, error! from tracing crate.

If tracing feature is enabled, should the bevy::log::debug! macro be using the macro from tracing instead of log?

Used log instead of tracing where possible.

That's a pretty change as the macros from tracing and log have different capabilities/syntax. I wouldn't expect a PR to enable no_std for bevy_log to have such a big side-effect

github-merge-queue bot pushed a commit that referenced this pull request Apr 14, 2025
…18816)

This reverts commit ac52cca.

Fixes #18815

# Objective

#18782 resulted in using `log` macros instead of `tracing` macros (in
the interest of providing no_std support, specifically no_atomic
support). That tradeoff isn't worth it, especially given that tracing is
likely to get no_atomic support.

## Solution

Revert #18782
mockersf pushed a commit that referenced this pull request Apr 14, 2025
…18816)

This reverts commit ac52cca.

Fixes #18815

the interest of providing no_std support, specifically no_atomic
support). That tradeoff isn't worth it, especially given that tracing is
likely to get no_atomic support.

Revert #18782
jf908 pushed a commit to jf908/bevy that referenced this pull request May 12, 2025
# Objective

- Fixes bevyengine#18781

## Solution

- Moved `LogPlugin` into its own file gated behind a new `tracing`
feature.
- Used `log` instead of `tracing` where possible.
- Exposed a new `tracing` feature in `bevy` which enables
`bevy_log/tracing`.
- Gated `LogPlugin` from `DefaultPlugins` on `tracing` feature.

## Testing

- CI

---

## Migration Guide

- If you were previously using `bevy_log` with default features
disabled, enable the new `std` and `tracing` features.
- If you were using `bevy` with the default features disabled, enable
the new `tracing` feature.

## Notes

Almost all of the diffs in this PR come from moving `LogPlugin` into its
own file. This just makes the PR less noisy, since the alternative is
excessive `#[cfg(feature = "tracing")]` directives all over the plugin.

---------

Co-authored-by: François Mockers <[email protected]>
jf908 pushed a commit to jf908/bevy that referenced this pull request May 12, 2025
…18782)" (bevyengine#18816)

This reverts commit ac52cca.

Fixes bevyengine#18815

# Objective

bevyengine#18782 resulted in using `log` macros instead of `tracing` macros (in
the interest of providing no_std support, specifically no_atomic
support). That tradeoff isn't worth it, especially given that tracing is
likely to get no_atomic support.

## Solution

Revert bevyengine#18782
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Utils Utility functions and types C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide O-Embedded Weird hardware and no_std platforms S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add no_std support to bevy_log
7 participants