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

Add rustflags for building with ESP IDF 5 #170

Closed
wants to merge 1 commit into from

Conversation

sirhcel
Copy link
Contributor

@sirhcel sirhcel commented Nov 21, 2022

  • Without them, building this crate failed with the error message
    $ export ESP_IDF_VERSION=release/v5.0
    $ rm -rf .embuild
    $ cargo clean
    $ export ESP_IDF_SDKCONFIG_DEFAULTS=$(pwd)/.github/configs/sdkconfig.defaults; cargo +nightly build --features experimental --target riscv32imc-esp-espidf -Zbuild-std=std,panic_abort -Zbuild-std-features=panic_immediate_abort
    [...]
       Compiling embedded-svc v0.23.1
    error[E0308]: mismatched types
      --> ~/.cargo/registry/src/github.com-1ecc6299db9ec823/esp-idf-sys-0.31.11/src/lib.rs:36:62
       |
    36 | const ESP_IDF_TIME64_CHECK: ::std::os::espidf::raw::time_t = 0 as crate::time_t;
       |                                                              ^^^^^^^^^^^^^^^^^^ expected `i32`, found `i64`
    
    error[E0308]: mismatched types
      --> ~/.cargo/registry/src/github.com-1ecc6299db9ec823/esp-idf-sys-0.31.11/src/lib.rs:38:51
       |
    38 | const ESP_IDF_TIME64_CHECK_LIBC: ::libc::time_t = 0 as crate::time_t;
       |                                                   ^^^^^^^^^^^^^^^^^^ expected `i32`, found `i64`
    
  • It was not obvious to me how to get this sorted out at a first glance
  • Thank you very much @Vollbrecht for your help!
  • What about using these flags by default (for development builds) for this repository?

@sirhcel
Copy link
Contributor Author

sirhcel commented Nov 21, 2022

@ivmarkov
Copy link
Collaborator

Is this working? If yes, that would be brilliant, BUT:

  • It needs to be defined in your binary crate, not in esp-idf-svc.

@sirhcel
Copy link
Contributor Author

sirhcel commented Nov 21, 2022

Is this working? If yes, that would be brilliant,

Yes, it does its job for IDF 5 and leaves IDF 4 alone.

  • It needs to be defined in your binary crate, not in esp-idf-svc.

I meant this as a hint for how to build just esp-idf-svc - for checking its code or for using this configuration in another (binary) crate. There is actually no other documentation available and i felt too lazy for starting to write developer documentation. But with this PR both, .github/worflows/ci.yml and .cargo/config.toml will give all the required information to a technical audience.

To me it was not clear from the beginning which knob would make this crate compile with IDF 5.

I thought about adding a comment to .cargo/config.toml but did not come up with a short and crispy one. Maybe the scope of the findings in .cargo/config.toml should be halfway clear to people looking there. The attempt of explaining the scope and how cargo handles these flags fells like opening a can of worms as of now. So I went for commenting better nothing than a wall of text.

So what about adding .cargo/config.toml as a hint for the ones interested and halfway knowing what they are looking at?

@ivmarkov
Copy link
Collaborator

I don't think the [build.'cfg(esp_idf_version_major = "5")'] is a valid syntax, because I get a cargo warning when trying to put it in ./cargo/config.toml of rust-esp32-std-demo:

warning: unused config key `build.cfg(not(esp_idf_version_major = "4"))` in ...

I was only able to find a similar [target.'cfg(...)'] syntax.
But even if this syntax did somehow work, I don't see how that can be used for the espidf_time64 flag. The point is, this flag is necessary for the Rust libc crate. And that crate is built before esp-idf-sys, and it is esp-idf-sys which produces the esp_idf_version* cfgs; these are not built-in in Rust of course.

Closing. Feel free to re-open if you can really put it in rust-esp32-astd-demo/.cargo/config.toml and prove that it works.

@ivmarkov ivmarkov closed this Nov 22, 2022
@sirhcel
Copy link
Contributor Author

sirhcel commented Nov 22, 2022

I don't think the [build.'cfg(esp_idf_version_major = "5")'] is a valid syntax, because I get a cargo warning when trying to put it in ./cargo/config.toml of rust-esp32-std-demo:

warning: unused config key `build.cfg(not(esp_idf_version_major = "4"))` in ...

You are right. I can't reproduce it today and I see this warning too. Don't know what I had messed up. Please excuse the confusion.

Closing. Feel free to re-open if you can really put it in rust-esp32-astd-demo/.cargo/config.toml and prove that it works.

Sure. This looks like a dead end now for the above reason and rust-lang/cargo#5376.

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

Successfully merging this pull request may close these issues.

2 participants