nielspardon opened a new issue, #12742:
URL: https://github.com/apache/gluten/issues/12742
## Problem description
Both protoc `add_custom_command`s in `cpp/core/CMakeLists.txt` declare their
dependency on a **directory**, and on the wrong one — one to two levels above
the directory that actually holds the `.proto` files. Editing a `.proto` in
place therefore changes no mtime the build graph is watching, so an incremental
native build silently skips protoc and keeps linking stale `*.pb.cc` / `*.pb.h`.
### Root cause
```cmake
# cpp/core/CMakeLists.txt:42-43
set(SUBSTRAIT_PROTO_SRC_DIR
${GLUTEN_HOME}/gluten-substrait/src/main/resources/substrait/proto)
# cpp/core/CMakeLists.txt:102-103
get_filename_component(SUBSTRAIT_PROTO_DIR ${SUBSTRAIT_PROTO_SRC_DIR}/
DIRECTORY)
# cpp/core/CMakeLists.txt:220-226
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} # <-- a directory, and the wrong one
COMMENT "Running Substrait PROTO compiler"
VERBATIM)
```
The trailing slash on `${SUBSTRAIT_PROTO_SRC_DIR}/` makes
`get_filename_component(... DIRECTORY)` strip the last component, so the paths
end up as:
| | path |
|---|---|
| `SUBSTRAIT_PROTO_SRC_DIR` | `.../resources/substrait/proto` |
| `SUBSTRAIT_PROTO_DIR` (declared dependency) | `.../resources/substrait` |
| files protoc actually reads |
`.../resources/substrait/proto/substrait/*.proto` |
The declared dependency is two directory levels above the inputs.
`SUBSTRAIT_PROTO_FILES` is already globbed at lines 90-91 and is what should
have been depended on.
The Gluten protos have the identical bug at lines 116 and 230-232:
`GLUTEN_PROTO_DIR` resolves to `.../org/apache/gluten` while the protos live in
`.../org/apache/gluten/proto/`.
Visible in the generated build graph — the single declared input is the
wrong directory:
```
# cpp/build/build.ninja:681
build core/proto/substrait/algebra.pb.h ... : CUSTOM_COMMAND \
/path/to/gluten/gluten-substrait/src/main/resources/substrait
```
### Reproduction
With an existing `cpp/build` tree:
```console
$ touch
gluten-substrait/src/main/resources/substrait/proto/substrait/algebra.proto
$ ninja -n core/proto/substrait/algebra.pb.h
ninja: no work to do. # <-- protoc will NOT re-run
$ touch gluten-core/src/main/resources/org/apache/gluten/proto/config.proto
$ ninja -n core/proto/config.pb.h
ninja: no work to do. # <-- same for the Gluten protos
$ touch gluten-substrait/src/main/resources/substrait # the *declared* dep
$ ninja -n core/proto/substrait/algebra.pb.h
[1/1] Running Substrait PROTO compiler # <-- only this triggers it
```
The mtimes show the edge is already stale in a normal working tree — the
declared dependency is 19 days older than the file protoc reads:
```
2026-07-22 09:50:08 .../resources/substrait
<-- declared dep
2026-08-10 19:14:06 .../resources/substrait/proto/substrait/algebra.proto
<-- real input
```
### Impact
Every change to `algebra.proto` so far has been additive or comment-only,
which makes a stale descriptor benign — so this has gone unnoticed. It stops
being benign as soon as a field is renumbered or removed.
I hit it on a branch that moves `WriteRel.bucket_spec` off field 7 (upstream
Substrait 0.98 assigns 7 to `common`). With a stale `algebra.pb.cc`, the JVM
emits the bucket spec at the new tag while the native library probes the old
one: `writeRel.has_bucket_spec()`
(`cpp/velox/substrait/SubstraitToVeloxPlan.cc:827`) returns false,
`bucketProperty` stays `nullptr`, and Velox writes **unbucketed** files for a
table the metastore records as `CLUSTERED BY`. Both tags are length-delimited,
so protobuf raises nothing — the bytes are shunted into unknown fields. No
exception, no fallback to vanilla Spark; just a wrong on-disk layout that later
bucket-pruned reads and bucketed joins answer incorrectly.
`dev/builddeps-veloxbe.sh` does `rm -rf build`, so CI and the official build
script always do a clean generation. That is precisely why this survives: it
only bites local incremental builds, IDE builds, and any workflow that reuses
`cpp/build`.
### Suggested fix
Depend on the files rather than a directory. The ClickHouse backend already
gets this right — `cpp-ch/local-engine/proto/CMakeLists.txt:33` uses `DEPENDS
${protobuf_files}`:
```cmake
DEPENDS ${SUBSTRAIT_PROTO_FILES}
...
DEPENDS ${GLUTEN_PROTO_FILES}
```
The `get_filename_component` calls at lines 102-103 and 116 then become dead
and can be dropped.
Worth noting separately: the `file(GLOB ...)` at lines 90-91 and 106 still
requires a CMake re-run to notice a newly added or deleted `.proto`. Adding
`CONFIGURE_DEPENDS` would close that gap, but the incorrect `DEPENDS` above is
the part that causes silently stale generated code.
---
This issue was written with the assistance of AI.
## System information
This is a static defect in the CMake build graph, so it is independent of
platform and toolchain — it reproduces wherever an existing `cpp/build` tree is
reused. Verified on:
```
os: Darwin 25.5.0 arm64
cmake: cmake version 4.4.0
ninja: 1.13.2
gluten: 6273a48a8016c13a33e1c254a140874e8de2717c
```
## CMake log
No CMake error is produced — that is the nature of the bug. The evidence is
the generated graph and the dry runs quoted above:
```bash
$ grep -n "algebra.pb.h" cpp/build/build.ninja
681:build core/proto/substrait/algebra.pb.h ... : CUSTOM_COMMAND \
/path/to/gluten/gluten-substrait/src/main/resources/substrait
$ touch
gluten-substrait/src/main/resources/substrait/proto/substrait/algebra.proto
$ ninja -n core/proto/substrait/algebra.pb.h
ninja: no work to do.
```
--
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]
