You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "officialasishkumar (via GitHub)" <gi...@apache.org> on 2026/04/09 18:54:36 UTC
[PR] [GLUTEN-11885][VL] Respect custom VELOX_HOME in build info generation [gluten]
officialasishkumar opened a new pull request, #11905:
URL: https://github.com/apache/gluten/pull/11905
## What changes are proposed in this pull request?
Fixes #11885.
`dev/gluten-build-info.sh` was always reading Velox revision metadata from the default `ep/build-velox/build/velox_ep` checkout, even when the build used a different Velox source tree via `--velox_home`.
This PR updates the build-info path resolution so the Velox revision metadata is taken from the actual configured Velox checkout:
- honor an explicit backend path passed to `gluten-build-info.sh`
- fall back to the exported `VELOX_HOME` environment variable when present
- fall back to the generated `cpp/build/CMakeCache.txt` `VELOX_HOME` entry before using the default path
- quote the resolved backend path when collecting git metadata
- export `VELOX_HOME` from `dev/builddeps-veloxbe.sh` so the one-shot Velox build flow propagates the configured path into Maven's build-info generation step
- parse `--velox_repo`, `--velox_branch`, and `--velox_home` as scalars so `VELOX_HOME` can be exported to child processes correctly
## How was this patch tested?
- `bash -n dev/gluten-build-info.sh`
- `bash -n dev/builddeps-veloxbe.sh`
- created a temporary standalone git repo at `/tmp/fake-velox`
- verified `VELOX_HOME=/tmp/fake-velox bash dev/gluten-build-info.sh ... --backend velox` writes the fake repo commit into `gluten-build-info.properties`
- verified `source ./dev/builddeps-veloxbe.sh --velox_home=/tmp/fake-velox true` exports `VELOX_HOME` for child processes
## Was this patch authored or co-authored using generative AI tooling?
No
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11885][VL] Respect custom VELOX_HOME in build info generation [gluten]
Posted by "philo-he (via GitHub)" <gi...@apache.org>.
philo-he commented on code in PR #11905:
URL: https://github.com/apache/gluten/pull/11905#discussion_r3070924797
##########
dev/builddeps-veloxbe.sh:
##########
@@ -120,15 +120,15 @@ do
shift # Remove argument name from processing
;;
--velox_repo=*)
- VELOX_REPO=("${arg#*=}")
+ VELOX_REPO="${arg#*=}"
Review Comment:
Do we also need such modification for some other args?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11885][VL] Respect custom VELOX_HOME in build info generation [gluten]
Posted by "officialasishkumar (via GitHub)" <gi...@apache.org>.
officialasishkumar commented on code in PR #11905:
URL: https://github.com/apache/gluten/pull/11905#discussion_r3171001902
##########
dev/gluten-build-info.sh:
##########
@@ -49,6 +50,34 @@ function echo_clickhouse_revision_info() {
echo ch_commit=$(cat $GLUTEN_ROOT/cpp-ch/clickhouse.version | grep -oP '(?<=^CH_COMMIT=).*')
}
+function read_cmake_cache_path() {
+ CACHE_FILE="$GLUTEN_ROOT/cpp/build/CMakeCache.txt"
+ CACHE_KEY="$1"
+ if [ -f "$CACHE_FILE" ]; then
+ grep "^${CACHE_KEY}:PATH=" "$CACHE_FILE" | cut -d= -f2- | head -n 1
+ fi
+}
+
+function resolve_velox_home() {
+ if [ -n "$BACKEND_HOME" ]; then
+ echo "$BACKEND_HOME"
+ return
+ fi
+
+ if [ -n "${VELOX_HOME:-}" ]; then
+ echo "$VELOX_HOME"
+ return
+ fi
+
+ CACHED_VELOX_HOME=$(read_cmake_cache_path VELOX_HOME)
Review Comment:
Fixed in ee37515a0. The script now reads `VELOX_HOME` from `CMakeCache.txt` regardless of CMake's cache value type, so `VELOX_HOME:UNINITIALIZED=...` from `-DVELOX_HOME=...` is honored. I verified it with a local fake Velox checkout and confirmed `gluten-build-info.properties` uses the custom checkout revision.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11885][VL] Respect custom VELOX_HOME in build info generation [gluten]
Posted by "philo-he (via GitHub)" <gi...@apache.org>.
philo-he merged PR #11905:
URL: https://github.com/apache/gluten/pull/11905
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11885][VL] Respect custom VELOX_HOME in build info generation [gluten]
Posted by "officialasishkumar (via GitHub)" <gi...@apache.org>.
officialasishkumar commented on code in PR #11905:
URL: https://github.com/apache/gluten/pull/11905#discussion_r3077610658
##########
dev/builddeps-veloxbe.sh:
##########
@@ -120,15 +120,15 @@ do
shift # Remove argument name from processing
;;
--velox_repo=*)
- VELOX_REPO=("${arg#*=}")
+ VELOX_REPO="${arg#*=}"
Review Comment:
I checked the current script again, and I do not think we need the same change for the other args in this PR. The scalar handling here matters for `--velox_repo`, `--velox_branch`, and especially `--velox_home` because they are reused when building `VELOX_PARAMETER` and when propagating `VELOX_HOME` to child processes. The other flags are only consumed locally in this script today. If there is another arg that also needs to cross that boundary, I can handle it in a follow-up.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11885][VL] Respect custom VELOX_HOME in build info generation [gluten]
Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW commented on PR #11905:
URL: https://github.com/apache/gluten/pull/11905#issuecomment-4228501111
Thank you for the fix
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11885][VL] Respect custom VELOX_HOME in build info generation [gluten]
Posted by "philo-he (via GitHub)" <gi...@apache.org>.
philo-he commented on code in PR #11905:
URL: https://github.com/apache/gluten/pull/11905#discussion_r3081053106
##########
dev/builddeps-veloxbe.sh:
##########
@@ -120,15 +120,15 @@ do
shift # Remove argument name from processing
;;
--velox_repo=*)
- VELOX_REPO=("${arg#*=}")
+ VELOX_REPO="${arg#*=}"
Review Comment:
@officialasishkumar, thanks for the check. Since this functionality is not well covered by CI, could you verify it in local build with a custom Velox home used? You may simply check a local resource file generated by gluten-build-info.sh.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11885][VL] Respect custom VELOX_HOME in build info generation [gluten]
Posted by "philo-he (via GitHub)" <gi...@apache.org>.
philo-he commented on code in PR #11905:
URL: https://github.com/apache/gluten/pull/11905#discussion_r3145191275
##########
dev/gluten-build-info.sh:
##########
@@ -49,6 +50,34 @@ function echo_clickhouse_revision_info() {
echo ch_commit=$(cat $GLUTEN_ROOT/cpp-ch/clickhouse.version | grep -oP '(?<=^CH_COMMIT=).*')
}
+function read_cmake_cache_path() {
+ CACHE_FILE="$GLUTEN_ROOT/cpp/build/CMakeCache.txt"
+ CACHE_KEY="$1"
+ if [ -f "$CACHE_FILE" ]; then
+ grep "^${CACHE_KEY}:PATH=" "$CACHE_FILE" | cut -d= -f2- | head -n 1
+ fi
+}
+
+function resolve_velox_home() {
+ if [ -n "$BACKEND_HOME" ]; then
+ echo "$BACKEND_HOME"
+ return
+ fi
+
+ if [ -n "${VELOX_HOME:-}" ]; then
+ echo "$VELOX_HOME"
+ return
+ fi
+
+ CACHED_VELOX_HOME=$(read_cmake_cache_path VELOX_HOME)
Review Comment:
Is this detection always reliable? Can we just use this for getting velox home?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org