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

Time for espidf_time32? #280

Open
ivmarkov opened this issue Feb 1, 2024 · 4 comments
Open

Time for espidf_time32? #280

ivmarkov opened this issue Feb 1, 2024 · 4 comments

Comments

@ivmarkov
Copy link
Collaborator

ivmarkov commented Feb 1, 2024

Background

Currently, when compiling for ESP IDF 5+, user needs RUSTFLAGS="--cfg espidf_time64 in their environment.
This is because ESP IDF 5+ migrated to a new GCC toolchain that defines the time(val?) type as 64 bit type, whereas in ESP IDF < 5 it is 32 bit. ... and since Rust's libc (and by extension - Rust STD) is just a bunch of hard-coded type definitions, completely unaware of esp-idf-sys's bindgen superpowers, it needs to be explicitly instructed whether it should treat timeval as 64 bit or 32 bit.

Proposal

With ESP IDF 4 getting out of maintenance by middle of this year, I suggest to switch - in a backward incompatible way - how libc is compiled. Namely:

  • If there is no flag defined, it should treat timeval as 64 bit, i.e. it should compile itself compatibly with ESP IDF 5. I hope everyone is aligned here
  • The question is, how to compile libc for ESP IDF 4?
    • Option A: Don't. it gets out of maintenance in 6 months. So introduce this change in July (or earlier?) and that's it.
    • Option B: Introduce espidf_time32 which should be present when compiling libc for ESP IDF < 5
    • Other options?

Timing to introduce this change? Might depend on which option is chosen of course.

@MabezDev @jessebraham @bjoernQ @Vollbrecht @igrr

@bjoernQ
Copy link
Contributor

bjoernQ commented Feb 2, 2024

t.b.h. I almost never use Rust-std on ESP-IDF 🙈 so my opinion probably shouldn't weight much but I think option A but probably not before July

@MabezDev
Copy link
Member

MabezDev commented Feb 2, 2024

I think option A is probably the right call, if users are using IDF versions that are EOL, they need to upgrade. In hindsight, we probably should have had an espidf_time32 in the first place, but we can keep that in mind for future breaking changes in esp-idf versions.

Regarding the timing, it might be a bit hard to time precisely as we don't have control on a) libc merging b) libc upgrades in rustc. Maybe we get the ball rolling fast, after announcing (in esp-rs? anywhere else?) the plan to only support 5.0 in the future. Users of 4.4 can still use an old compiler (I guess 1.76 might be last supported) to build their 4.4 projects until they upgrade.

@ivmarkov
Copy link
Collaborator Author

ivmarkov commented Feb 6, 2024

I think option A is probably the right call, if users are using IDF versions that are EOL, they need to upgrade. In hindsight, we probably should have had an espidf_time32 in the first place, but we can keep that in mind for future breaking changes in esp-idf versions.

I can agree to that. Though removing the espidf_time64 flag from upstream libc is just as much effort as removing this flag and adding espidf_time32 for 4.4.
But you are probably right - why bother? Or maybe...? You know, just to be on the safe side if anybody is using this in production?

@MabezDev
Copy link
Member

You're right, if we're already touching that code, we may as well add the time32 flag. It's better to have the work around there than not at all!

You know, just to be on the safe side if anybody is using this in production?

If they are, I would hope that they are taking care of upgrading there esp-idf version before it's EoL or contacting Espressif for an extended support period :D, but again adding the time32 flag is probably a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

3 participants