Skip to content
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

0.1.11 release results in build errors when using the esp toolchain (for Xtensa) #11

Closed
jessebraham opened this issue Jul 9, 2024 · 5 comments
Labels
bug Something isn't working O-xtensa Target: Xtensa processors

Comments

@jessebraham
Copy link

While updating our Xtensa toolchain version to 1.79.0 in CI for esp-hal, we recently encountered some new build errors which seem to originate from the 0.1.11 release:

The esp toolchain lags behind nightly so it's understandable why this occurs. Adding a patch to our hil-test package which pins the semihosting package to 0.1.10 has resolved these issues for the time being, however this is obviously not a long-term solution.

I see that #10 already exists, would be nice to close this to help avoid similar issues in the future. If you would like some help with that issue please let me know and I can find some time to take care of it, otherwise we will continue to patch the dependency until an appropriate fix has been released.

@taiki-e
Copy link
Owner

taiki-e commented Jul 9, 2024

Thanks for the report. I yanked 0.1.11 for now.

I see that #10 already exists

#10 is about "test", not "build", and "build" is already done in our CI and already catches this issue: https://github.com/taiki-e/semihosting/actions/runs/9849899467/job/27194302779
(I was busy at work so it was late to look into that issue.)

Also this is a problem that first happened today: https://github.com/taiki-e/semihosting/actions/workflows/ci.yml

And the odd thing is that we already check that the toolchain is after the stabilization patches have been merged to avoid the "problem when built with the old nightly toolchain" and yet we have this problem. (In other words, this is a completely different situation than what happened with proc-macro2 and others, where the maintainers were not interested in supporting old nightly.)

semihosting/build.rs

Lines 40 to 42 in cc18471

// error_in_core stabilized in Rust 1.81 (nightly-2024-06-09): https://github.com/rust-lang/rust/pull/125951
if !version.probe(81, 2024, 6, 8) {
println!("cargo:rustc-cfg=semihosting_no_error_in_core");

semihosting/build.rs

Lines 163 to 168 in cc18471

pub(crate) fn probe(&self, minor: u32, year: u16, month: u8, day: u8) -> bool {
if self.nightly {
self.minor > minor || self.commit_date >= Date::new(year, month, day)
} else {
self.minor >= minor
}

I don't know what is happening in their toolchain, but are commit dates not a reliable indicator? (At least, this works approach perfectly with the toolchain installed by rustup...)

cc @SergioGasquez @MabezDev

@taiki-e
Copy link
Owner

taiki-e commented Jul 9, 2024

(Perhaps rustversion would also be affected by such custom toolchain's commit date issue. https://github.com/dtolnay/rustversion#selectors

#[rustversion::nightly(2019-01-01)]

@taiki-e
Copy link
Owner

taiki-e commented Jul 9, 2024

self.minor > minor || self.commit_date >= Date::new(year, month, day) 

In the short term we can handle this issue by adding a version check (self.minor == minor &&) on the right side of this code as well as stable/beta case1, but I'm not convinced it will work in the future (it still has normal "old nightly" issue).

That said, if the esp toolchain is building the stable branch of rust as nightly, that is also enough to prevent future problems.

Footnotes

  1. Btw, the reason this check was not done in the first place is that sometimes the stable/beta stabilized version did not match the version that actually stabilized the feature in the nightly toolchain. For example, Atomic{I,U}{8,16,32,64} has been stable since stable/beta 1.34 and nightly-2019-01-27 (1.33.0-nightly).

@taiki-e taiki-e closed this as completed in ba827e2 Jul 9, 2024
@taiki-e taiki-e added bug Something isn't working O-xtensa Target: Xtensa processors labels Jul 9, 2024
taiki-e added a commit that referenced this issue Jul 9, 2024
@taiki-e
Copy link
Owner

taiki-e commented Jul 9, 2024

Fixed in v0.1.12 which implemented a way mentioned in #11 (comment).

@jessebraham
Copy link
Author

That was fast, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working O-xtensa Target: Xtensa processors
Projects
None yet
Development

No branches or pull requests

2 participants