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

cpu/esp32: fix compilation issues with GCC 12.2 [backport 2023.04] #19561

Conversation

kaspar030
Copy link
Contributor

Backport of #19450

Contribution description

This PR provides the changes in cpu/esp32 and cpu/esp_common to fix the compilation issues with GCC v12.2. It is required as the first step in the preparation of the upgrade to ESP-IDF version 5.1.

Please note: Insead of fixing the ESP-IDF 4.4 code itself by a big bunch of patches to fix the compilation problems with GCC v12.2, it temporarily disables some warnings. The reason is that the ESP-IDF 5.1 requires GCC v12.2 and should be fixed for this compiler version by the vendor.

Testing procedure

Green CI

The change were already tested with all ESP-specific modules like esp_now, esp_wifi, esp_spi and esp_ble for all supported ESP platforms.

Issues/PRs references

Prerequisite for RIOT-OS/riotdocker#227
Fixes issue #19421

@kaspar030 kaspar030 requested a review from gschorcht as a code owner May 9, 2023 08:54
@kaspar030 kaspar030 added Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels May 9, 2023
@kaspar030
Copy link
Contributor Author

@MrKevinWeiss please take a look at this.

We're having a riotdocker chicken-and-egg, RIOT-OS/riotdocker#227 needs #19450 to pass, but riotdocker needs to stay compatible with the latest release (so backport PRs still pass). @gschorcht and I discussed how to fix this. Other than waiting until the next release branch is split (which would painfully stall Gunar for up to two months), backporting this toolchain fix PR seems like the best option.

@riot-ci
Copy link

riot-ci commented May 9, 2023

Murdock results

✔️ PASSED

fcd8e8e cpu/esp32: disable warnings in ESP-IDF for gcc 12.2

Success Failures Total Runtime
6882 0 6882 11m:05s

Artifacts

@MrKevinWeiss
Copy link
Contributor

Would this also mean we need a point release? I guess it would be able to just take the release branch...

@MrKevinWeiss
Copy link
Contributor

We can merge the backport but before release I would like to run the easy automated tests and rerun the board tests on the esp.

Copy link
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, this will have to be run with the current riot docker version before release.

@MrKevinWeiss
Copy link
Contributor

bors merge

bors bot added a commit that referenced this pull request May 9, 2023
19561: cpu/esp32: fix compilation issues with GCC 12.2 [backport 2023.04] r=MrKevinWeiss a=kaspar030

# Backport of #19450

### Contribution description

This PR provides the changes in `cpu/esp32` and `cpu/esp_common` to fix the compilation issues with GCC v12.2.  It is required as the first step in the preparation of the upgrade to ESP-IDF version 5.1.

**Please note**: Insead of fixing the ESP-IDF 4.4 code itself by a big bunch of patches to fix the compilation problems with GCC v12.2, it temporarily disables some warnings. The reason is that the ESP-IDF 5.1 requires GCC v12.2 and should be fixed for this compiler version by the vendor.

### Testing procedure

Green CI

The change were already tested with all ESP-specific modules like `esp_now`, `esp_wifi`, `esp_spi`  and `esp_ble` for all supported ESP platforms.

### Issues/PRs references

Prerequisite for RIOT-OS/riotdocker#227
Fixes issue #19421

Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
@bors
Copy link
Contributor

bors bot commented May 9, 2023

Build failed:

@kaspar030
Copy link
Contributor Author

Would this also mean we need a point release?

I don't think we need a point release. Those we do if there's "critical" stuff.

@maribu
Copy link
Member

maribu commented May 9, 2023

Would this also mean we need a point release? I guess it would be able to just take the release branch...

Sorry for just adding a 👍 to a comment that states two alternatives.

I guess it would be able to just take the release branch...

This is what the 👍 was directed at ;)

@kaspar030
Copy link
Contributor Author

Hrmpf, looks like this trips over the codespell update. I guess we'll have to backport #19528, too.

2 similar comments
@kaspar030
Copy link
Contributor Author

Hrmpf, looks like this trips over the codespell update. I guess we'll have to backport #19528, too.

@kaspar030
Copy link
Contributor Author

Hrmpf, looks like this trips over the codespell update. I guess we'll have to backport #19528, too.

@MrKevinWeiss
Copy link
Contributor

I don't think we need a point release. Those we do if there's "critical" stuff.

I thought it would need it to fix the docker latest issue?

@MrKevinWeiss
Copy link
Contributor

MrKevinWeiss commented May 10, 2023

See #19563, which I guess needs to be in first...

@maribu
Copy link
Member

maribu commented May 10, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented May 10, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 89fc055 into RIOT-OS:2023.04-branch May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants