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

Disable CMake workflow #153

Closed
SergioGasquez opened this issue Sep 18, 2023 · 7 comments · Fixed by #155
Closed

Disable CMake workflow #153

SergioGasquez opened this issue Sep 18, 2023 · 7 comments · Fixed by #155
Assignees

Comments

@SergioGasquez
Copy link
Member

@ivmarkov shall we disable the CMake CI? We know it will be failing so it does not give too much information

@ivmarkov
Copy link
Collaborator

I would rather not. CMake workflow will continue to be supported, but only for ESP IDF 5.1+, which happens to use a recent-enough GCC toolchain that can link with the libs generated by recent rustc.

In other words, the right fix is to run the CI only for esp idf 5.1 and greater, and remove the options that allow you to generate a project targeting earlier esp idf releases.

@ivmarkov
Copy link
Collaborator

I mean, older esp idf versions can still be supported, but it is a lot of work, as you you need to e.g. work with the esp idf 4.x gcc toolchain to compile the esp idf itself, but then use the gcc toolchain from an esp idf 5.1+ sdk to link together the esp idf with the Rust code. We have this setup for the cargo-first approach in esp-idf-sys but for the cmake approach it would be too much effort to support it. For one, it cannot be centralized in esp-idf-sys, as this crate does not control neither the esp idf sdk, nor the toolchains for cmake based builds.

@SergioGasquez
Copy link
Member Author

I would rather not. CMake workflow will continue to be supported, but only for ESP IDF 5.1+, which happens to use a recent-enough GCC toolchain that can link with the libs generated by recent rustc.

In other words, the right fix is to run the CI only for esp idf 5.1 and greater, and remove the options that allow you to generate a project targeting earlier esp idf releases.

Thanks for the clarification! I am ok with only allowing versions greater than 5.1, the template is to generate new projects and I think is good to enforce to use the latest esp-idf version (as long as it's stable).

The workflow currently does the opposite, fails for v5.1 but succeeds for v4.4: https://github.com/esp-rs/esp-idf-template/actions/runs/6219698181/job/16878262796

@ivmarkov
Copy link
Collaborator

ivmarkov commented Sep 18, 2023

I would rather not. CMake workflow will continue to be supported, but only for ESP IDF 5.1+, which happens to use a recent-enough GCC toolchain that can link with the libs generated by recent rustc.
In other words, the right fix is to run the CI only for esp idf 5.1 and greater, and remove the options that allow you to generate a project targeting earlier esp idf releases.

Thanks for the clarification! I am ok with only allowing versions greater than 5.1, the template is to generate new projects and I think is good to enforce to use the latest esp-idf version (as long as it's stable).

The workflow currently does the opposite, fails for v5.1 but succeeds for v4.4: https://github.com/esp-rs/esp-idf-template/actions/runs/6219698181/job/16878262796

Ah, yeah. 4.4 build works, but not for long ;) Notice that the 4.4 build works with the ESP toolchain, but not with the upstream nightly Rust compiler:

  • The moment we have a new release of the ESP toolchain, based on the current Rust nightly (it is beta in the meantime, I think) it will start failing
  • 5.1 fails for a completely unrelated reason, I must fix in esp-idf-sys and then release it (the new ADC driver apparently is a separate component, and needs to be guarded with an #ifdef in our bindings.h)

@ivmarkov
Copy link
Collaborator

@SergioGasquez Let's do the following:

  • Remove ESP IDF < 5.1 (and maybe include 5.2 if it is released in the meantime) from the build matrix. Would you contribute a PR for that?
  • I'll address the adc_continuous.h issue in esp-idf-sys and will then release a new version of the crate; need to do this anyway, just waiting for "one more enhancement". The new esp-idf-* releases will be a big deal, btw

@SergioGasquez
Copy link
Member Author

Just created the PR. There is not yet a released v5.2, will include it once it's out!

@ivmarkov
Copy link
Collaborator

I know I still have to fix esp-idf-sys, but that's probably a separate issue. Closing.

@github-project-automation github-project-automation bot moved this from Todo to Done in esp-rs Sep 18, 2023
@SergioGasquez SergioGasquez linked a pull request Sep 18, 2023 that will close this issue
@SergioGasquez SergioGasquez self-assigned this Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants