Feature naming is likely to cause confusion

[jamesmunns]

We had someone in embedded trip over the fact that:

• portable-atomic is an optional dep
• the feature is portable_atomic
• optional deps are allowed to have - or _ and are unified
• If you select portable-atomic as the feature, you don't get a warning because the feature does something, but not what you expect (it still tries to use core::sync::atomic).

An example of the cfg is here: https://github.com/mvdnes/spin-rs/blob/master/src/lib.rs#L66-L67

The user had this in their Cargo.toml:

spin = { version = "0.9.8", default-features = false, features = ["portable-atomic"] }

You might want to add a "trap" for this in spin that will save someone needing to debug this in the future:

#[cfg(feature = "portable-atomic")]
compiler_error!("Oops it's a trap");

[jamesmunns]

Another option would be to switch to using the dep:portable_atomic syntax, which would give a better error like this:

error: failed to select a version for `demo-lib`.
    ... required by package `demo-bin v0.1.0 (/private/tmp/feature-trap/demo-bin)`
versions that meet the requirements `*` (locked to 0.1.0) are: 0.1.0

the package `demo-bin` depends on `demo-lib`, with features: `lazy_static` but `demo-lib` does not have these features.
 It has an optional dependency with that name, but that dependency uses the "dep:" syntax in the features table, so it does not have an implicit feature with that name.


failed to select a version for `demo-lib` which could resolve this conflict

[zesterer]

Thanks, I think switching to the new syntax would be most appropriate. Any chance you'd be able to open a PR?

[jamesmunns]

Ah, I just realized you have a MSRV of 1.38: https://github.com/mvdnes/spin-rs/blob/master/Cargo.toml#L13

the dep: syntax wasn't added until v1.60. Is that a blocker for you? And if so, would you like me to also add a minor version bump?

[zesterer]

Ah, that's a shame.

Our MSRV is quite important to us: spin sits quite deep in the dependency tree of a lot of crates, and supporting old compilers is particularly important for quirky embedded environments that might be tied to a legacy Rust version, a common use-case for spin.

That said, we don't yet have a particular MSRV policy and 1.60 was almost 3 years ago now, so perhaps a bump is fine, with a minor version change. I'd appreciate a PR!

[jamesmunns]

PR opened!

By the way:

important for quirky embedded environments that might be tied to a legacy Rust version

When you say "quirky embedded environments", do you mean embedded linux or other similar "with an OS" sort of targets, or bare metal targets? Because we've actually generally been advising AGAINST using spin-rs on bare metal systems, because spinlocks and interrupts don't work well together, if you have a lock held in non-interrupt context, and attempt to lock in interrupt context, then the interrupt can never make forward progress and will deadlock.

If you mean systems like armv5te which have linux but no atomics, then feel free to disregard! Embedded is a very nebulous term :)

[zesterer]

PR opened!

Thanks!

If you mean systems like armv5te which have linux but no atomics, then feel free to disregard! Embedded is a very nebulous term :)

I'm talking about single-core systems, yes.