Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add portable-atomic Feature #91

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add portable-atomic Feature #91

wants to merge 4 commits into from

Conversation

bushrat011899
Copy link

@bushrat011899 bushrat011899 commented Dec 29, 2024 •
edited
Loading

Objective

Description

Added portable-atomic feature, which brings in portable-atomic and portable-atomic-util as a compatibility shim for Arc support on atomically challenged platforms, such as thumbv6m-none-eabi.

On platforms with targer_has_atomic = "ptr" (typical), this feature does nothing.

Testing

  • cargo check --no-default-features --target thumbv6m-none-eabi --features portable-atomic,portable-atomic/fallback,portable-atomic/critical-section

Notes

This is my first attempt at contributing to this project. Please let me know if there's anything I can do to assist with reviewing this change!

@bushrat011899
Copy link
Author

bushrat011899 commented Dec 29, 2024 •
edited
Loading

CI issue is caused by enabling all features for testing, which enables the portable-atomic compatibility support. I'm unsure how this project would like to handle this kind of issue.

@bushrat011899 bushrat011899 mentioned this pull request Dec 29, 2024
Merged
github-merge-queue bot pushed a commit to bevyengine/bevy that referenced this pull request Dec 29, 2024
@bushrat011899
Add no_std support to bevy_input (#16995)
46af466 
# Objective

- Contributes to #15460

## Solution

- Added the following features:
  - `std` (default)
  - `smol_str` (default)
  - `portable-atomic`
  - `critical-section`
  - `libm`
- Fixed an existing issue where `bevy_reflect` wasn't properly feature
gated.

## Testing

- CI

## Notes

- There were some minor issues with `bevy_math` and `bevy_ecs` noticed
in this PR which I have also resolved here. I can split these out if
desired, but I've left them here for now as they're very small changes
and I don't consider this PR itself to be very controversial.
- `libm`, `portable-atomic`, and `critical-section` are shortcuts to
enable the relevant features in dependencies, making the usage of this
crate on atomically challenged platforms possible and simpler.
- `smol_str` is gated as it doesn't support atomically challenged
platforms (e.g., Raspberry Pi Pico). I have an issue and a
[PR](rust-analyzer/smol_str#91) to discuss this
upstream.
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
@bushrat011899 @ecoskey
Add no_std support to bevy_input (bevyengine#16995)
8734b66 
# Objective

- Contributes to bevyengine#15460

## Solution

- Added the following features:
  - `std` (default)
  - `smol_str` (default)
  - `portable-atomic`
  - `critical-section`
  - `libm`
- Fixed an existing issue where `bevy_reflect` wasn't properly feature
gated.

## Testing

- CI

## Notes

- There were some minor issues with `bevy_math` and `bevy_ecs` noticed
in this PR which I have also resolved here. I can split these out if
desired, but I've left them here for now as they're very small changes
and I don't consider this PR itself to be very controversial.
- `libm`, `portable-atomic`, and `critical-section` are shortcuts to
enable the relevant features in dependencies, making the usage of this
crate on atomically challenged platforms possible and simpler.
- `smol_str` is gated as it doesn't support atomically challenged
platforms (e.g., Raspberry Pi Pico). I have an issue and a
[PR](rust-analyzer/smol_str#91) to discuss this
upstream.
@bushrat011899
Copy link
Author

Resolved CI issue by checking for target_has_atomic = "ptr", a critical condition for Arc to be available from alloc::sync. I then used this check to add appropriate From implementations for portable_atomic_util::Arc and alloc::sync::Arc as available. Additionally, I've ensured smol_str will always use alloc::sync::Arc if it is available, even when the portable-atomic feature is enabled.

I have confirmed this adds support for smol_str to platforms without support for atomic pointers. This can be quickly verified using something like:

cargo check --target thumbv6m-none-eabi --no-default-features --features portable-atomic,portable-atomic/unsafe-assume-single-core

@bushrat011899
Copy link
Author

Finally, I've put portable-atomic behind a target dependency, so on existing targets there is zero observable change. I believe this could be a minor update with no breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for thumbv6m-none-eabi via portable-atomic
1 participant
@bushrat011899