fix(cmake): Rebuild on changes in sdkconfig.defaults (IDFGH-13269)

[MathyV]

When you change sdkconfig.defaults, sdkconfig is not updated and a rebuild is not triggered. This commit fixes that by adding the files to the target.

[github-actions]

	Messages
📖	🎉 Good Job! All checks are passing!

👋 Hello MathyV, 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 3ecbb4d

[igrr]

@MathyV Do I understand it right that the situation is:

1. The project is built once, sdkconfig file is generated
2. sdkconfig.defaults is modifed
3. The project is built again

Do you expect that sdkconfig file will be re-generated at step 3, applying new values from sdkconfig.defaults?

[MathyV]

Do you expect that sdkconfig file will be re-generated at step 3, applying new values from sdkconfig.defaults?

That is correct

[igrr]

In that case, I think this doesn't match how we expect sdkconfig to work. Sdkconfig file should get values from the defaults files when the respective options are missing from sdkconfig. In other cases, it's okay for sdkconfig to contain a value different than sdkconfig.defaults has. For example, if you build the project once and then change some options in menuconfig, sdkconfig file will contain the value you have saved in menuconfig.

Otherwise, if sdkconfig.defaults would take precedence over sdkconfig (or would overwrite sdkconfig values) then it would not be possible to change the option in menuconfig to a value that doesn't match the one specified in sdkconfig.defaults.

[nebkat]

I think this is something worth exploring further - IMO there should be an sdkconfig.local or similar file for explicit/manual/temporary menuconfig changes, but otherwise the use of sdkconfig.defaults and generated sdkconfig should be strongly encouraged and more closely integrated into the build process.

If a defaults file has been changed upstream it is usually with good reason - yet we explicitly favour the local file which might not even contain any local changes.

I am also certain there are hundreds of projects out there using a git committed sdkconfig that was generated when the project was first created, with whatever the defaults were at the time in ESP-IDF, and dragging around other changes that were only intended to be temporary.

My team has successfully been using set(SDKCONFIG "${CMAKE_BINARY_DIR}/sdkconfig") and only working in terms of defaults files for the past 9 months. The only remaining pitfall has been someone forgetting to do a fullclean before building and retaining the old config, at best having a compile error, or at worst losing hours debugging a runtime error, so this change would be very welcome.

[KaeLL]

I don't think this change is necessary, although it also doesn't affect my workflow. My understanding is that IDF incentivizes working with sdkconfig.defaults rather than regular sdkconfig, both because the former is tidy and the latter is unstable (although after #13490, I don't know what to think anymore). Made changes in menuconfig? Save it as you would, which changes the sdkconfig and trigger rebuilds. Need to commit to/persist them? idf.py save-defconfig or menuconfig, press D (defconfig) and point it to your root sdkconfig.defaults and save it, which doesn't trigger rebuilds because it's not necessary at this stage.

[MathyV]

Sorry for not chiming in earlier, lost track of this one a bit.

Like @nebkat , I consider sdkconfig to be a build artifact, rather than something that should be committed into the repository. I much prefer having upstream changes for things I don't know or care about over having to keep track of these things myself.

Another reason I do it like this is because I often target multiple platforms with the same code. I don't care about all the options that need to be set for example to have wifi work on the different devices, I just enable the main wifi setting in a shared sdkconfig.defaults file, know my code will work for all and don't have to worry much more about it.

I tend to structure my configuration over multiple sdkconfig.defaults files and then just link to the ones I need for a certain platform, much in the way that MicroPython does it. You can see an example of it it here:

https://github.com/MathyV/fri3d_firmware/blob/main/boards/fox/CMakeLists.txt

@nebkat what you are suggesting with a sdkconfig.local, I'm already doing in that code too. I use it to locally trigger extra logging output, to lower the risk of accidentally committing it in a production build. The file is in .gitignore so it doesn't end up in the repo. I don't feel this needs to be explicitly added to IDF though, as I think it much depends on developer preference and it's easy to add.

[igrr]

My team has successfully been using set(SDKCONFIG "${CMAKE_BINARY_DIR}/sdkconfig") and only working in terms of defaults files for the past 9 months. The only remaining pitfall has been someone forgetting to do a fullclean before building and retaining the old config, at best having a compile error, or at worst losing hours debugging a runtime error, so this change would be very welcome.

I think this could be achieved with a CMake recipe which monitors changes to sdkconfig.defaults and deletes sdkconfig whenever sdkconfig.defaults changes, forcing sdkconfig to be re-generated from sdkconfig.defaults.

In that case, you can use sdkconfig for local changes, and when you want to save them to sdkconfig.defaults you run idf.py save-defconfig. The only downside of this approach is that if you get sdkconfig.defaults updates after pulling a new version of your project from git, your local defaults get wiped. However other than having some interactive prompt asking to resolve the conflicts between local and remote changes, I can't think of any alternative which would retain both incoming and local changes. And that seems like something that could be handled using existing tools (e.g. merging your local sdkconfig.defaults with the remote one.)

I think we would be open to adding such a CMake recipe in the form of a function which can be called from the project CMakeLists.txt file. (E.g. idf_rebuild_sdkconfig_on_defaults_changes(...)). I don't think we can change the default behavior at this point.

We are also slowly working towards the new configuration system which will resolve some of Kconfig limitations, while retaining compatibility with Kconfig files. The most significant limitation we want to fix is that we can't distinguish sdkconfig options which have been explicitly set (but equal to the default value) from default values that the project developer doesn't care about. This will help us move closer to having an sdkconfig.defaults-like file as the single source of truth that tools like menuconfig will work with. However I don't have a timeline for this, so a small improvement in the form of a CMake helper function would be welcome in the meantime.