From 6919c5152eebd08ab5ca7c92b16a261e16ead77b Mon Sep 17 00:00:00 2001 From: Niels Pardon Date: Mon, 10 Aug 2026 19:31:33 +0200 Subject: [PATCH] [GLUTEN-12742][CORE] Fix protoc DEPENDS so incremental builds regenerate proto sources Both protoc `add_custom_command`s in cpp/core/CMakeLists.txt declared their dependency on a directory rather than on the `.proto` files, and the trailing slash passed to `get_filename_component(... DIRECTORY)` stripped a component, so the declared path ended up one to two levels above the inputs: SUBSTRAIT_PROTO_SRC_DIR .../resources/substrait/proto SUBSTRAIT_PROTO_DIR .../resources/substrait <-- declared dep files protoc reads .../resources/substrait/proto/substrait/*.proto Editing a `.proto` in place therefore changed no mtime in the build graph, and an incremental build silently skipped protoc and kept linking stale `*.pb.cc` / `*.pb.h`. This has been benign so far only because every `algebra.proto` change to date was additive or comment-only; it turns wrong as soon as a field is renumbered or removed, because the JVM then writes one tag while the native library probes another and protobuf shunts the bytes into unknown fields without raising anything. Depend on the globbed file lists instead, matching what the ClickHouse backend already does in cpp-ch/local-engine/proto/CMakeLists.txt. The `get_filename_component` calls become dead and are dropped. Also add `CONFIGURE_DEPENDS` to both globs so that adding or deleting a `.proto` re-runs CMake instead of requiring a manual re-configure. `dev/builddeps-veloxbe.sh` does `rm -rf build`, so CI and the official build script were unaffected; this only bit local incremental builds, IDE builds, and any workflow reusing cpp/build. Fixes #12742 --- cpp/core/CMakeLists.txt | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/cpp/core/CMakeLists.txt b/cpp/core/CMakeLists.txt index 240567b8ff1..e0d82d42041 100644 --- a/cpp/core/CMakeLists.txt +++ b/cpp/core/CMakeLists.txt @@ -87,7 +87,8 @@ set(PROTO_OUTPUT_DIR "${CMAKE_CURRENT_BINARY_DIR}/proto") file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/proto) # List Substrait Proto compiled files -file(GLOB SUBSTRAIT_PROTO_FILES ${SUBSTRAIT_PROTO_SRC_DIR}/substrait/*.proto +file(GLOB SUBSTRAIT_PROTO_FILES CONFIGURE_DEPENDS + ${SUBSTRAIT_PROTO_SRC_DIR}/substrait/*.proto ${SUBSTRAIT_PROTO_SRC_DIR}/substrait/extensions/*.proto) foreach(PROTO ${SUBSTRAIT_PROTO_FILES}) file(RELATIVE_PATH REL_PROTO ${SUBSTRAIT_PROTO_SRC_DIR} ${PROTO}) @@ -99,11 +100,9 @@ set(SUBSTRAIT_PROTO_OUTPUT_FILES ${SUBSTRAIT_PROTO_HDRS} ${SUBSTRAIT_PROTO_SRCS}) set_source_files_properties(${SUBSTRAIT_PROTO_OUTPUT_FILES} PROPERTIES GENERATED TRUE) -get_filename_component(SUBSTRAIT_PROTO_DIR ${SUBSTRAIT_PROTO_SRC_DIR}/ - DIRECTORY) # List Gluten Proto compiled files -file(GLOB GLUTEN_PROTO_FILES ${GLUTEN_PROTO_SRC_DIR}/*.proto) +file(GLOB GLUTEN_PROTO_FILES CONFIGURE_DEPENDS ${GLUTEN_PROTO_SRC_DIR}/*.proto) foreach(PROTO ${GLUTEN_PROTO_FILES}) file(RELATIVE_PATH REL_PROTO ${GLUTEN_PROTO_SRC_DIR} ${PROTO}) string(REGEX REPLACE "\\.proto" "" PROTO_NAME ${REL_PROTO}) @@ -113,7 +112,6 @@ endforeach() set(GLUTEN_PROTO_OUTPUT_FILES ${GLUTEN_PROTO_HDRS} ${GLUTEN_PROTO_SRCS}) set_source_files_properties(${GLUTEN_PROTO_OUTPUT_FILES} PROPERTIES GENERATED TRUE) -get_filename_component(GLUTEN_PROTO_DIR ${GLUTEN_PROTO_SRC_DIR}/ DIRECTORY) set(SPARK_COLUMNAR_PLUGIN_SRCS ${SUBSTRAIT_PROTO_SRCS} @@ -221,7 +219,7 @@ add_custom_command( OUTPUT ${SUBSTRAIT_PROTO_OUTPUT_FILES} COMMAND ${PROTOC_BIN} --proto_path ${SUBSTRAIT_PROTO_SRC_DIR}/ --cpp_out ${PROTO_OUTPUT_DIR} ${SUBSTRAIT_PROTO_FILES} - DEPENDS ${SUBSTRAIT_PROTO_DIR} + DEPENDS ${SUBSTRAIT_PROTO_FILES} COMMENT "Running Substrait PROTO compiler" VERBATIM) @@ -229,7 +227,7 @@ add_custom_command( OUTPUT ${GLUTEN_PROTO_OUTPUT_FILES} COMMAND ${PROTOC_BIN} --proto_path ${GLUTEN_PROTO_SRC_DIR}/ --cpp_out ${PROTO_OUTPUT_DIR} ${GLUTEN_PROTO_FILES} - DEPENDS ${GLUTEN_PROTO_DIR} + DEPENDS ${GLUTEN_PROTO_FILES} COMMENT "Running Gluten PROTO compiler" VERBATIM)