github-actions[bot] commented on code in PR #64131:
URL: https://github.com/apache/doris/pull/64131#discussion_r3364121071
##########
be/CMakeLists.txt:
##########
@@ -894,7 +894,13 @@ if (ENABLE_PCH)
endif()
endif()
- add_library(pch STATIC ${SRC_DIR}pch/pch.cc)
+ # Add gensrc headers as PCH dependencies so ninja rebuilds PCH when they
change.
+ file(GLOB GENSRC_PCH_DEPENDS
+ ${GENSRC_DIR}/gen_cpp/*.h
+ )
+
+ add_library(pch STATIC ${SRC_DIR}pch/pch.cc ${GENSRC_PCH_DEPENDS})
+ set_source_files_properties(${GENSRC_PCH_DEPENDS} PROPERTIES
HEADER_FILE_ONLY ON)
Review Comment:
This does not create the dependency edge the comment relies on. In
Ninja/CMake, files added to a target with `HEADER_FILE_ONLY` are not inputs of
the PCH build rule; they are ignored for build purposes/IDE visibility only.
The PCH output still only depends on headers discovered by the compiler depfile
from the previous successful PCH compile, so if
`gensrc/build/gen_cpp/AgentService_types.h` is rewritten after the PCH starts,
Ninja can still run the stale PCH and Clang will report the same "file has been
modified since the precompiled header was built" error. I verified this with a
minimal CMake/Ninja PCH target: a `HEADER_FILE_ONLY` source not included by
`pch.h` is absent from `ninja -t deps CMakeFiles/pch.dir/cmake_pch.hxx.gch`.
Please model the generated headers as real generated outputs/order-only
dependencies before `pch` is built (or otherwise attach an actual dependency to
the PCH build rule), rather than adding them as header-only sources.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
