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

Compatibility with ESP-IDF V5 #2913

Merged
merged 1 commit into from Sep 19, 2022
Merged

Compatibility with ESP-IDF V5 #2913

merged 1 commit into from Sep 19, 2022

Conversation

ivmarkov
Copy link
Contributor

@ivmarkov ivmarkov commented Sep 17, 2022

The new major release of the ESP-IDF (V5) has extended the time_t type from 32 to 64 bits. Unfortunately, this brings the challenge how to simultaneously support older ESP-IDF releases (current stable is V4.4.2) and the V5 one with the libc (and Rust STD) crates.

After dismissing the option to introduce 5 (or more) additional ESP-IDF targets, we settled on the introduction of a rustc cfg flag, as the most lightweight option: espidf_time64:

  • This flag needs to be enabled by the user for ESP-IDF V5 (for example, by setting it in the [build] section of .config/cargo.toml or using one of the other methods to pass flags to the Rust compiler);
  • This flag should not be set by the user for earlier ESP-IDF versions (we might reverse the logic once ESP-IDF V5 becomes more main-stream in future).

The good news in all that is that it is impossible for the user to set the flag to the wrong value as his/her build will simply fail. This is because compiling for the ESP-IDF framework does require a hard dependency on the esp-idf-sys crate, which - besides exposing autogenerated (with bindgen) APIs which are a super-set of the libc APIs - also does the heavy-lifting of compiling the ESP-IDF framework itself and emitting the relevant Rust linker flags and a lot of other things. So when compiling esp-idf-sys, we are checking the compatibility of the libc's type_t with the bindgen-ed one inside esp-idf-sys where the latter is the source of truth, so to say.

@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@JohnTitor
Copy link
Member

Makes sense, thanks for the detailed explanation!
@bors r+

@bors
Copy link
Contributor

bors commented Sep 19, 2022

📌 Commit ba7ff45 has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Sep 19, 2022

⌛ Testing commit ba7ff45 with merge 8531d01...

@bors
Copy link
Contributor

bors commented Sep 19, 2022

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing 8531d01 to master...

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

Successfully merging this pull request may close these issues.

None yet

5 participants