[maitake-sync]: `critical-section` feature shouldn't set `portable-atomic/critical-section`
In the Cargo.toml:
https://github.com/hawkw/mycelium/blob/main/maitake-sync/Cargo.toml#L29
However, `portable-atomic`'s docs give this guidance:
> It is usually discouraged to always enable this feature in libraries that depend on portable-atomic.
>
> Enabling this feature will prevent the end user from having the chance to take advantage of other (potentially) efficient implementations (implementations provided by unsafe-assume-single-core feature mentioned above, implementation proposed in [#60](https://github.com/taiki-e/portable-atomic/issues/60), etc.). Also, targets that are currently unsupported may be supported in the future.
>
> The recommended approach for libraries is to leave it up to the end user whether or not to enable this feature. (However, it may make sense to enable this feature by default for libraries specific to a platform where other implementations are known not to work.)
Unfortunately, `esp-hal` with the `esp32c3` feature active sets the `portable-atomic/unsafe-assume-single-core` feature, rather than the `portable-atomic/critical-section` feature:
https://github.com/esp-rs/esp-hal/blob/main/esp-hal/Cargo.toml#L202-L210
These features are incompatible, and lead to a compilation error:
```
error: you may not enable `critical-section` feature and `portable_atomic_unsafe_assume_single_core` cfg (`unsafe-assume-single-core` feature) at the same time
--> /Users/james/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/portable-atomic-1.11.1/src/lib.rs:430:1
|
430 | / compile_error!(
431 | | "you may not enable `critical-section` feature and `portable_atomic_unsafe_assume_single_core` cfg (`unsafe-assume-single-core` feature) at ...
432 | | );
| |_^
```
This causes problems for `bbqueue` and `ergot`, which both aim to be compatible with non-atomic platforms, specifically including the `esp32c3`. CC @okhsunrog:
* https://github.com/jamesmunns/ergot/pull/184
* https://github.com/jamesmunns/bbqueue/pull/123
I do *personally* think `esp-hal` is in the wrong here as well, but that leaves us with what to do with `maitake-sync`. I think the options are:
1. cut a breaking change that no longer activates the `p-a/c-s` feature when the `critical-section` feature is enabled
2. introduce a new `diet-critical-section` feature, that does not activate the `p-a/c-s` feature, maybe add some kind of warning to get people to switch over for `0.2`, in the future just make this an alias for `critical-section` again for `0.3` releases
3. yolo-remove the `p-a/c-s` activation in a minor release, maybe note in the docs that people need to handle it on their own? This is probably not Cool and Good behavior tho.
I'm not certain the ideal UX to ensure that `p-a` has a backend available, we might need to give docs guidance that on non-atomic targets, users need to select a `p-a` impl.
1 条评论