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.
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
Migrate to embedded-hal 1.0.0-alpha.9 #38
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for the PR!
I'm a bit hesitant about this as it is technically a semver-breaking change. I am not sure how to best handle this... The only solution I see would be to declare that the eh-alpha
feature is not following semver so users must pin shared-bus
with a =0.2.x
dependency. But we didn't do this from the get-go so there will be projects out there who'd get broken by introducing this now...
What do you think about this?
Thank you for checking. It is best to be able to choose alpha.8 or alpha.9.
Even if I specified el-alpha feature, it was recognized as version “1.0.0-alpha.9”. |
Indeed, your approach might be the best solution for the time being. Hm, does it work if you use [dependencies]
embedded-hal-alpha = { package = "embedded-hal", version = "=1.0.0-alpha.8", optional = true }
embedded-hal-alpha-9 = { package = "embedded-hal", version = "=1.0.0-alpha.9", optional = true } |
Unfortunately, the settings you provided will result in the following error.
|
Uff, that's weird. To all my knowledge, they shouldn't interfere when specified this way. I'll try to investigate this as well, I will let you know if I find something... |
I created a test branch using #38 (comment) settings. (https://github.com/taks/shared-bus/tree/embedded-hal-1.0.0-alpha.9-test) The result is usable from the projects that used shared-bus library. (https://github.com/taks/shared-bus/tree/embedded-hal-1.0.0-alpha.9-test/test_projects) |
Hm, it feels like this is a bug in cargo. With just the following lines in an empty project, it is failing: [dependencies]
embedded-hal = "=0.2.3"
ver2 = { package = "embedded-hal", version = "=0.2.4" } while it works with [dependencies]
embedded-hal = "=0.2.3"
ver2 = { package = "embedded-hal", version = "=0.1.0" } It looks like this fails when the versions are semver-compatible to each other but Do you want to report it upstream or should I take care of it? |
I searched cargo issues and found a similar issue here. |
Just for the record, this was discussed in the embedded rust working group meeting today: rust-embedded/wg#640 The consensus was mostly that the alpha versions shouldn't be considered semver-compatible by cargo in the first place which would have avoided this problem. We'll have to talk to cargo folks to see what they think about this... Still, I am surprised that there is no way to override this behavior when absolutely needed. |
Thank you for sharing. Regarding this change, since v1.0.0-alpha.8 is still used by other libraries, |
Hi @taks @Rahix, thanks for the work on the crate! Just wanted to chime in wrt release timing of this PR. We're currently using this branch with Also, if you look at the charts at https://crates.io/crates/embedded-hal/1.0.0-alpha.8 and https://crates.io/crates/embedded-hal/1.0.0-alpha.9, you can see that the alpha.9 weekly downloads have already surpassed alpha.8. |
Hm, okay, I think I've made up my mind: As this is the embedded-hal alpha, we can accept breaking downstream code in a semver-compatible release. People should know what they are doing when they are using an alpha version. So I'll merge this change and release a new patch release. If you have users on the alpha release, please encourage them to pin (use Of course this is still quite unfortunate but I don't see any other option that doesn't end in maintenance hell or depends on changing the cargo resolver as discussed previously... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@taks, thanks for providing this changeset!
See https://github.com/rust-embedded/embedded-hal/releases/tag/v1.0.0-alpha.9