[core] Write custom option headers to an env independent include dir - #400
Open
bdraco wants to merge 3 commits into
Open
[core] Write custom option headers to an env independent include dir#400bdraco wants to merge 3 commits into
bdraco wants to merge 3 commits into
Conversation
|
|
esphbot
suggested changes
Jul 27, 2026
esphbot
left a comment
There was a problem hiding this comment.
Warning
Important issues found.
- Shared include dir leaks generated headers between envs of the same project
|
|
esphbot
approved these changes
Jul 27, 2026
esphbot
left a comment
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
PR Review — [core] Write custom option headers to an env independent include dirBoth prior suggestions are cleanly addressed; nothing blocking remains. Approve. What's solid in this revision:
No new issues found. Residual rough edges are inert and not worth changing: a ✅ Resolved since last review (1)Previously-flagged issues verified fixed
Checklist
Automated review by Kōan (Claude) |
esphbot
approved these changes
Jul 28, 2026
esphbot
left a comment
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
bdraco
marked this pull request as ready for review
July 28, 2026 00:25
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow up to esphome/esphome#17726, which enabled ccache for ESPHome's LibreTiny builds; sharing the cache between devices is blocked by this generated header path, so the remaining fix belongs here.
The generated custom option headers (lwipopts.h, sys_config.h and friends) are currently written to ${BUILD_DIR}/include, a path that embeds the PlatformIO env name. That -I flag makes every compile command unique per env, so compiler caches like ccache can never share results between projects that differ only by env name; ESPHome creates one env per device, so nothing is shared between devices.
This writes them to ${PROJECT_BUILD_DIR}/include/ instead, where the hash is derived from the option content. The path no longer contains the env name, so with ccache base_dir the compile commands hash identically across devices with the same options; envs with different options get different directories, so they can never pick up each other's generated headers, and concurrent builds of envs with identical options write identical bytes. The header writes also moved to a with block so a failed build cannot leave a truncated header behind.
Verified with ESPHome pointing the framework source at this branch; a clean bk72xx build succeeds, the headers land in the content keyed directory, 292 dependency files reference them at the new path with none referencing the old one, and a second compile reproduces the same hash with nothing rebuilt.