Skip to content

Fixes compilation with -save-temps flag (IDFGH-12615)#13613

Open
denizzzka wants to merge 3 commits into
espressif:masterfrom
denizzzka:save-temps_compilation_fix
Open

Fixes compilation with -save-temps flag (IDFGH-12615)#13613
denizzzka wants to merge 3 commits into
espressif:masterfrom
denizzzka:save-temps_compilation_fix

Conversation

@denizzzka

@denizzzka denizzzka commented Apr 15, 2024

Copy link
Copy Markdown
Contributor

Project's root CMakeLists.txt:

cmake_minimum_required(VERSION 3.16)

# Preserve *.i files
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -save-temps")

include($ENV{IDF_PATH}/tools/cmake/project.cmake)
project(mcu_software)

If try to compile with -save-temps and with:

CONFIG_COMPILER_DISABLE_GCC12_WARNINGS=y
CONFIG_COMPILER_DISABLE_GCC13_WARNINGS=y

it causes error:
$ idf.py build lib
[...]
components/spiffs/spiffs/src/spiffs_nucleus.c: In function 'spiffs_populate_ix_map_v':
components/spiffs/spiffs/src/spiffs_nucleus.c:682:348: error: self-comparison always evaluates to false [-Werror=tautological-compare]

This PR fixes compilation for such case

Depends from #13612


Note

Low Risk
CMake-only compile-flag wiring; no SPIFFS runtime or API behavior changes.

Overview
Fixes SPIFFS failing to compile when the project adds -save-temps to CMAKE_C_FLAGS and GCC 12/13 warning disables are not enabled globally (e.g. spiffs_populate_ix_map_v tripping -Werror=tautological-compare).

The component CMakeLists.txt now prepends ${CMAKE_CURRENT_LIST_DIR}/ to upstream SPIFFS sources before set_source_files_properties, so per-file flags like -Wno-format apply reliably with temp-file builds. It drops the old GNU-only -Wno-stringop-truncation block on spiffs_nucleus.c and instead appends -Wno-tautological-compare to that file’s existing compile flags (keeping -Wno-format).

Reviewed by Cursor Bugbot for commit 50f947e. Bugbot is set up for automated code reviews on this repo. Configure here.

@CLAassistant

CLAassistant commented Apr 15, 2024

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions

github-actions Bot commented Apr 15, 2024

Copy link
Copy Markdown
Messages
📖 🎉 Good Job! All checks are passing!

👋 Hello denizzzka, we appreciate your contribution to this project!


📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project.

Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.

This GitHub project is public mirror of our internal git repository

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved, we synchronize it into our internal git repository.
4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
5. If the change is approved and passes the tests it is merged into the default branch.
5. On next sync from the internal git repository merged change will appear in this public GitHub repository.

Generated by 🚫 dangerJS against 4183043

@espressif-bot espressif-bot added the Status: Opened Issue is new label Apr 15, 2024
@github-actions github-actions Bot changed the title Fixes compilation with -save-temps flag Fixes compilation with -save-temps flag (IDFGH-12615) Apr 15, 2024
@igrr

igrr commented Apr 15, 2024

Copy link
Copy Markdown
Member

@denizzzka Thanks for the PR, I think we can accept both changes in one branch (i.e. this PR). Could you please update the commit messages as suggested above by the checker, and click through the CLA?

@denizzzka denizzzka force-pushed the save-temps_compilation_fix branch from fa5a517 to 209f2cb Compare April 15, 2024 15:57
@denizzzka denizzzka force-pushed the save-temps_compilation_fix branch from 209f2cb to 1a38852 Compare April 15, 2024 16:05
@denizzzka denizzzka marked this pull request as ready for review April 15, 2024 16:05
@denizzzka denizzzka force-pushed the save-temps_compilation_fix branch 3 times, most recently from e6dabd7 to 4183043 Compare April 19, 2024 13:19
@denizzzka

Copy link
Copy Markdown
Contributor Author

Please pay attention to this PR

@denizzzka

Copy link
Copy Markdown
Contributor Author

(still relevant)

@denizzzka

Copy link
Copy Markdown
Contributor Author

Bump

@igrr

@igrr

igrr commented May 26, 2025

Copy link
Copy Markdown
Member

sha=41830433f4dc338b29ba1f97c5907bd805a271d8

@igrr igrr added the PR-Sync-Merge Pull request sync as merge commit label May 26, 2025
@denizzzka

Copy link
Copy Markdown
Contributor Author

Merging is blocked
You're not authorized to push to this branch. Visit https://docs.github.com/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches for more information.

I don't understand what's going with this PR :-(

@denizzzka denizzzka force-pushed the save-temps_compilation_fix branch from 4183043 to 50f947e Compare May 20, 2026 08:25
@denizzzka

denizzzka commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

I don't understand. Why does the CI care about my local commit hooks?!

This tiny PR is already two years old, lol

@denizzzka denizzzka force-pushed the save-temps_compilation_fix branch 4 times, most recently from c87067e to 50f947e Compare June 24, 2026 02:50
@denizzzka

Copy link
Copy Markdown
Contributor Author
Notice: It looks like the commits in this PR have been made without having pre-commit hooks installed.

This damn machine is lying!

Okay, just tell me if you need this or not. Maybe I'm wasting my time.

I hate your CI system.

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

Labels

PR-Sync-Merge Pull request sync as merge commit Status: Opened Issue is new

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants