Track #embed file dependencies in CMake and enable ccache depend mode#100411
Track #embed file dependencies in CMake and enable ccache depend mode#100411alexey-milovidov merged 10 commits intomasterfrom
Conversation
The C++ `#embed` directive includes file contents at compile time, but the build system does not automatically track these as dependencies. When an embedded file (e.g., `play.html`) changes, the .cpp file that embeds it is not recompiled. Fix this by adding `OBJECT_DEPENDS` properties to all source files that use `#embed`, covering: HTML pages, JavaScript files, XML configs, and NLP data files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ccache's direct mode uses its own source parser to find included files, but it does not recognize C23 `#embed` directives. This means changes to files included via `#embed` (HTML pages, JS libraries, XML configs) do not invalidate the ccache, leading to stale build artifacts. Enable depend mode (available since ccache 4.0), which uses the compiler's dependency file (.d) instead. The compiler correctly lists `#embed` files in its dependency output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Workflow [PR], commit [0b63b69] Summary: ✅ AI ReviewSummaryThis PR fixes stale rebuild behavior for C23 Missing context
ClickHouse Rules
Final Verdict
|
clickhouse-gh
Bot
added
the
pr-build
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
| set_source_files_properties(Server/KeeperDashboardRequestHandler.cpp PROPERTIES OBJECT_DEPENDS | ||
| "${CMAKE_SOURCE_DIR}/programs/keeper/dashboard.html") | ||
|
|
||
| # Note: Common/FrequencyHolder.cpp embeds files from contrib/nlp-data/ via #embed, |
There was a problem hiding this comment.
#embed dependency tracking is still incomplete for src/Common/FrequencyHolder.cpp.
Right now this PR adds OBJECT_DEPENDS for most #embed users, but explicitly skips NLP blobs. That means when contrib/nlp-data/*.zst changes in a checkout where those files exist, FrequencyHolder.cpp can still be left stale and not recompiled.
Please add conditional dependencies instead of skipping entirely, e.g. guard with if (EXISTS ...) and then call set_source_files_properties(Common/FrequencyHolder.cpp PROPERTIES OBJECT_DEPENDS "...charset.zst;...tonality_ru.zst").
| "${CMAKE_SOURCE_DIR}/programs/keeper/dashboard.html") | ||
|
|
||
| # Note: Common/FrequencyHolder.cpp embeds files from contrib/nlp-data/ via #embed, | ||
| # but that submodule is not always checked out, so we cannot add OBJECT_DEPENDS for it. |
There was a problem hiding this comment.
maybe we can do this only of this file exists? (i.e. submodules has been initialized)
|
While at it, would you please remove the comments which I introduced with #99075? |
| set_source_files_properties(Server/IndexRequestHandler.cpp PROPERTIES OBJECT_DEPENDS | ||
| "${CMAKE_SOURCE_DIR}/programs/server/index.html") | ||
|
|
||
| set_source_files_properties(Server/KeeperDashboardRequestHandler.cpp PROPERTIES OBJECT_DEPENDS |
There was a problem hiding this comment.
This still leaves one #embed consumer untracked: src/Storages/System/attachInformationSchemaTables.cpp embeds multiple files under src/Storages/System/information_schema/*.sql, but there is no matching OBJECT_DEPENDS entry.
As a result, edits to these SQL templates can still reuse stale objects and skip recompilation. Please add set_source_files_properties(Storages/System/attachInformationSchemaTables.cpp PROPERTIES OBJECT_DEPENDS "...") with the embedded SQL files.
…ng deps - Remove "CMake doesn't recognize changes in #embed-ed files" comments from all files, since this PR now tracks those dependencies. - Add conditional `OBJECT_DEPENDS` for `FrequencyHolder.cpp` NLP blobs (guarded by `if (EXISTS ...)` for when the submodule is not checked out). - Add `OBJECT_DEPENDS` for `attachInformationSchemaTables.cpp` which was also missing dependency tracking for its embedded SQL files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nto cmake-embed-deps
LLVM Coverage Report
Changed lines: PR changed-lines coverage: N/A (no coverable changed lines found in .info). · Uncovered code |
robot-ch-test-poll
added
the
pr-synced-to-cloud
4 participants
The C++
#embeddirective includes file contents at compile time,but the build system does not automatically track these as dependencies.
When an embedded file (e.g.,
play.html) changes, the .cpp file thatembeds it is not recompiled.
Fix by adding
OBJECT_DEPENDSproperties to source files that use#embed,and enabling ccache depend mode so that ccache also correctly invalidates
when
#embed-ed files change (ccache's direct mode parser does not recognize#embed).Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Track
#embedfile dependencies in CMake and enable ccache depend mode for correct rebuilds.Documentation entry for user-facing changes