esp-rs / esp-hal

no_std Hardware Abstraction Layers for ESP32 microcontrollers
https://docs.esp-rs.org/esp-hal/
Apache License 2.0
752 stars 209 forks source link

When the latest versions of esp-hal and slint are present simultaneously, a compilation error occurs #1434

Closed Song-aff closed 6 months ago

Song-aff commented 7 months ago

When the latest versions of esp-hal and slint are present simultaneously, a compilation error occurs:

[dependencies]
esp-hal = { version = "0.16.1", features = ["esp32c3"] }
slint ={version = "1.5.1", default-features = false, features = [
    "compat-1-2",
    "unsafe-single-threaded",
    "libm",
    "renderer-software"]}

The error message is as follows:

cargo build
   Compiling portable-atomic v1.6.0
error: you may not enable feature `critical-section` and cfg(portable_atomic_unsafe_assume_single_core) at the same time
   --> /Users/admin/.cargo/registry/src/mirrors.tuna.tsinghua.edu.cn-df7c3c540f42cdbd/portable-atomic-1.6.0/src/lib.rs:409:1
    |
409 | / compile_error!(
410 | |     "you may not enable feature `critical-section` and cfg(portable_...
411 | | );
    | |_^

error: could not compile `portable-atomic` (lib) due to 1 previous error
bjoernQ commented 7 months ago

Seems like slint unconditionally enables portable-atomic/critical-section here: https://github.com/slint-ui/slint/blob/master/internal/core/Cargo.toml#L58

While for C3 we enable portable-atomic/unsafe-assume-single-core here: https://github.com/esp-rs/esp-hal/blob/da3375bbe47dcb659353b909fbc556a74bb64cda/esp-hal/Cargo.toml#L109

Given this comment in the portable-atomic:

Therefore, for better performance, if all the critical-section implementation for your target does is disable interrupts, prefer using unsafe-assume-single-core feature instead.

I think for C3 we do the right thing here. Ideally Slint would have this a (default) feature to have a way to opt-out of this

Song-aff commented 7 months ago

Seems like slint unconditionally enables portable-atomic/critical-section here: https://github.com/slint-ui/slint/blob/master/internal/core/Cargo.toml#L58

While for C3 we enable portable-atomic/unsafe-assume-single-core here:

https://github.com/esp-rs/esp-hal/blob/da3375bbe47dcb659353b909fbc556a74bb64cda/esp-hal/Cargo.toml#L109

Given this comment in the portable-atomic:

Therefore, for better performance, if all the critical-section implementation for your target does is disable interrupts, prefer using unsafe-assume-single-core feature instead.

I think for C3 we do the right thing here. Ideally Slint would have this a (default) feature to have a way to opt-out of this

thank you. Before the official issue in Slint is resolved, would you recommend me to fork the codebase and modify it to use portable-atomic/critical-section for C3, and then reference it?

bjoernQ commented 7 months ago

Before the official issue in Slint is resolved, would you recommend me to fork the codebase and modify it to use portable-atomic/critical-section for C3, and then reference it?

That is probably a good workaround for now

MabezDev commented 6 months ago

This is a slint issue, as per https://github.com/taiki-e/portable-atomic 's README, libraries should not enable critical-section etc features themselves.

MabezDev commented 6 months ago

Closing this, but please file an issue against slint, I'm sure they'd be happy to resolve this on their end :).