esp-idf-sys icon indicating copy to clipboard operation
esp-idf-sys copied to clipboard

Time for `espidf_time32`?

Open ivmarkov opened this issue 6 months ago • 4 comments

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

ivmarkov avatar Feb 01 '24 19:02 ivmarkov

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

bjoernQ avatar Feb 02 '24 07:02 bjoernQ

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.

MabezDev avatar Feb 02 '24 11:02 MabezDev

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?

ivmarkov avatar Feb 06 '24 18:02 ivmarkov

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.

MabezDev avatar Feb 20 '24 13:02 MabezDev