slint-ui / slint

Slint is a declarative GUI toolkit to build native user interfaces for Rust, C++, or JavaScript apps.
https://slint.dev
Other
16.94k stars 568 forks source link

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

Open Song-aff opened 5 months ago

Song-aff commented 5 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
Song-aff commented 5 months ago

https://github.com/esp-rs/esp-hal/issues/1434#issuecomment-2055667647 I also raised this question in ESP-HAL

ogoffart commented 5 months ago

Looks like we shouldn't unconditionally require critical-section in https://github.com/slint-ui/slint/blob/a61ca93cd121b080d6caf9e30e1336a04b7d1f07/internal/core/Cargo.toml#L58 and maybe also in once_cell, but we then need to enable it in some case. I wonder what's best here.

KortanZ commented 4 months ago

Any plan to fix this? It's really annoying that need to patch esp-hal as a workaround in order to use slint. Really thankful if this could be fixed 🙂.

ogoffart commented 3 months ago

Not sure how to fix this. We can't just remove the features = ["critical-section"] from our Cargo.toml, otherwise things will stop compiling. Even if we add unsafe-single-threaded = ["portable-atomic/unsafe-assume-single-core"] in our own Cargo.toml this won't work on stm32

KortanZ commented 3 months ago

@ogoffart what if we follow the portable-atomic crate recommendation? Leave the end user to determine which feature to use, features = ["critical-section"] or features = ["unsafe-assume-single-core"]. We remove features = ["critical-section"], and add an slint feature let's say critical-section, and bind portable-atomic/critical-section. Full change may like:

unsafe-single-threaded = ["portable-atomic/unsafe-assume-single-core"]
critical-section = ["portable-atomic/critical-section"]

And user could choose one of them to use.

Update: There may some HAL use critical-seciton in protable-atomic, but user may still need use unsafe-single-threaded in slint. So we may need change like:

unsafe-single-threaded = []
portable-atomic-single-core = ["portable-atomic/unsafe-assume-single-core"]
portable-atomic-critical-section = ["portable-atomic/critical-section"]

A breaking and clumsy change, but I can't see any other solution. : (

ogoffart commented 3 months ago

I guess you're right. We can make it non-breaking by using the compat-1-2 feature.

In api/rs/slint/Cargo.toml

unsafe-single-threaded = []
portable-atomic-single-core = ["portable-atomic/unsafe-assume-single-core"]
portable-atomic-critical-section = ["portable-atomic/critical-section"]
compat-1-2 = ["portable-atomic-critical-section", "compat-1-7"]
compat-1-7 = []

in internal/core/Cargo.toml

portable-atomic = { version = "1", features = ["require-cas"] }

But we also need to document that properly.

(This just makes it even more complicated to setup for MCU apps)