Copilot commented on code in PR #12625:
URL: https://github.com/apache/gluten/pull/12625#discussion_r3650690078
##########
dev/builddeps-veloxbe.sh:
##########
@@ -196,12 +210,43 @@ function concat_velox_param {
VELOX_PARAMETER+="--run_setup_script=$RUN_SETUP_SCRIPT "
}
+function print_vcpkg_diagnostics {
+ local phase="$1"
+ echo "::group::vcpkg diagnostics: ${phase}"
+ printf '%s\n' \
+ "PATH=${PATH}" \
+ "CC=${CC:-}" \
+ "CXX=${CXX:-}" \
+ "LD_LIBRARY_PATH=${LD_LIBRARY_PATH:-}" \
+ "LIBRARY_PATH=${LIBRARY_PATH:-}" \
+ "COMPILER_PATH=${COMPILER_PATH:-}" \
+ "GCC_EXEC_PREFIX=${GCC_EXEC_PREFIX:-}" \
+ "PKG_CONFIG_PATH=${PKG_CONFIG_PATH:-}" \
+ "PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-}" \
+ "CMAKE_PREFIX_PATH=${CMAKE_PREFIX_PATH:-}" \
+ "CMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE:-}"
+ for tool in gcc g++ cc c++ ld flex; do
+ echo "--- ${tool}"
+ type -a "${tool}" || true
+ command -v "${tool}" | xargs -r readlink -f || true
+ done
+ for compiler in gcc g++ cc c++; do
+ echo "--- ${compiler} details"
+ "${compiler}" --version 2>/dev/null | head -n 1 || true
+ "${compiler}" -print-file-name=libstdc++.a 2>/dev/null || true
+ "${compiler}" -print-search-dirs 2>/dev/null || true
+ done
+ find /usr/include /usr/local/include /opt/rh -name FlexLexer.h -print
2>/dev/null || true
+ echo "::endgroup::"
+}
if [ "$ENABLE_VCPKG" = "ON" ]; then
# vcpkg will install static depends and init build environment
BUILD_OPTIONS="--build_tests=$BUILD_TESTS --enable_s3=$ENABLE_S3
--enable_gcs=$ENABLE_GCS \
--enable_hdfs=$ENABLE_HDFS --enable_abfs=$ENABLE_ABFS"
+ print_vcpkg_diagnostics "before env.sh"
source ./dev/vcpkg/env.sh ${BUILD_OPTIONS}
+ print_vcpkg_diagnostics "after env.sh"
fi
Review Comment:
The vcpkg diagnostics are always printed when `--enable_vcpkg=ON`, including
a filesystem-wide `find` over `/usr/include`, `/usr/local/include`, and
`/opt/rh`. This can add noticeable overhead and log noise to normal builds;
it’s better as an opt-in debug mode.
##########
.github/workflows/velox_weekly.yml:
##########
@@ -41,7 +41,7 @@ jobs:
fail-fast: false
matrix:
os: [ "centos:8", "quay.io/centos/centos:stream9" ]
- if: ${{ startsWith(github.repository, 'apache/') }}
+ if: ${{ false }}
runs-on: ubuntu-22.04
Review Comment:
`if: ${{ false }}` disables this scheduled/PR weekly build job entirely, so
regressions in Velox backend won’t be caught. If the intent is to keep the
workflow active for apache/gluten only, restore the previous repository guard
instead of hard-disabling it.
This issue also appears in the following locations of the same file:
- line 72
- line 115
- line 144
- line 172
##########
.github/workflows/velox_backend_arm.yml:
##########
@@ -144,6 +144,7 @@ jobs:
--local --preset=velox --benchmark-type=ds --error-on-memleak
--off-heap-size=10g -s=1.0 --threads=16 --iterations=1
cpp-test-udf-test:
+ if: ${{ false }}
runs-on: ubuntu-24.04-arm
Review Comment:
`if: ${{ false }}` disables this job entirely, removing UDF test coverage on
ARM CI. If this workflow is expected to keep that coverage, drop the
hard-disable.
##########
.github/workflows/velox_backend_x86.yml:
##########
@@ -1118,6 +1120,7 @@ jobs:
"
build-fast-build-test:
+ if: ${{ false }}
runs-on: ubuntu-22.04
Review Comment:
This job is hard-disabled via `if: ${{ false }}`, which eliminates the
fast-build profile coverage from CI. Drop the hard-disable if this workflow is
still expected to validate that profile.
##########
.github/workflows/velox_backend_x86.yml:
##########
@@ -1068,6 +1069,7 @@ jobs:
path: "**/target/*.log"
build-cudf-centos-9:
+ if: ${{ false }}
runs-on: ubuntu-22.04
Review Comment:
This job is hard-disabled via `if: ${{ false }}`, so the cuDF build path
will never run and regressions won’t be detected. Remove the hard-disable if
the workflow is still intended to cover cuDF builds.
##########
.github/workflows/velox_backend_x86.yml:
##########
@@ -985,6 +985,7 @@ jobs:
**/gluten-ut/**/core.*
cpp-test-udf-test:
+ if: ${{ false }}
runs-on: ubuntu-22.04
Review Comment:
Setting `if: ${{ false }}` disables this job entirely, removing UDF test
coverage from the CI workflow. If the job is meant to remain part of the Velox
CI, drop the hard-disable.
##########
dev/vcpkg/toolchain.cmake:
##########
@@ -33,11 +33,52 @@ set(VCPKG_HOST_TRIPLET $ENV{VCPKG_TRIPLET})
set(VCPKG_INSTALLED_DIR $ENV{VCPKG_MANIFEST_DIR}/vcpkg_installed)
set(VCPKG_INSTALL_OPTIONS --no-print-usage)
-# Force read CMAKE_PREFIX_PATH from env
-set(CMAKE_PREFIX_PATH $ENV{CMAKE_PREFIX_PATH})
-
+set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY)
include($ENV{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake)
+if(DEFINED ENV{GLUTEN_VCPKG_PREFER_CONFIG}
+ AND NOT "$ENV{GLUTEN_VCPKG_PREFER_CONFIG}" STREQUAL "")
+ set(_GLUTEN_VCPKG_PREFER_CONFIG "$ENV{GLUTEN_VCPKG_PREFER_CONFIG}")
+elseif(DEFINED CMAKE_FIND_PACKAGE_PREFER_CONFIG)
+ set(_GLUTEN_VCPKG_PREFER_CONFIG "${CMAKE_FIND_PACKAGE_PREFER_CONFIG}")
+else()
+ set(_GLUTEN_VCPKG_PREFER_CONFIG ON)
+endif()
+set(CMAKE_FIND_PACKAGE_PREFER_CONFIG "${_GLUTEN_VCPKG_PREFER_CONFIG}" CACHE
BOOL
+ "Prefer package configuration files." FORCE)
+unset(_GLUTEN_VCPKG_PREFER_CONFIG)
+set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY CACHE STRING "Search for packages
only in root paths." FORCE)
+set(CMAKE_FIND_USE_PACKAGE_REGISTRY OFF CACHE BOOL "Disable the user package
registry." FORCE)
+set(CMAKE_FIND_USE_SYSTEM_PACKAGE_REGISTRY OFF CACHE BOOL "Disable the system
package registry." FORCE)
+
+set(_GLUTEN_VCPKG_IGNORED_PREFIXES /usr/local)
+if(APPLE)
+ list(APPEND _GLUTEN_VCPKG_IGNORED_PREFIXES /opt/homebrew /opt/local)
+endif()
+if(NOT "$ENV{CONDA_PREFIX}" STREQUAL "")
+ list(APPEND _GLUTEN_VCPKG_IGNORED_PREFIXES "$ENV{CONDA_PREFIX}")
+endif()
+list(APPEND CMAKE_IGNORE_PREFIX_PATH ${_GLUTEN_VCPKG_IGNORED_PREFIXES})
+list(REMOVE_DUPLICATES CMAKE_IGNORE_PREFIX_PATH)
+set(CMAKE_IGNORE_PREFIX_PATH "${CMAKE_IGNORE_PREFIX_PATH}" CACHE STRING
+ "Prefixes ignored by Gluten's vcpkg toolchain." FORCE)
+unset(_GLUTEN_VCPKG_IGNORED_PREFIXES)
+
+find_program(_GLUTEN_DIAGNOSTIC_CXX NAMES c++ g++ NO_CACHE)
+find_program(_GLUTEN_DIAGNOSTIC_FLEX NAMES flex NO_CACHE)
+find_path(_GLUTEN_DIAGNOSTIC_FLEX_INCLUDE_DIR NAMES FlexLexer.h NO_CACHE)
+message(STATUS "[Gluten vcpkg diagnostics] ENV{CC}=$ENV{CC}")
+message(STATUS "[Gluten vcpkg diagnostics] ENV{CXX}=$ENV{CXX}")
+message(STATUS "[Gluten vcpkg diagnostics] ENV{PATH}=$ENV{PATH}")
+message(STATUS "[Gluten vcpkg diagnostics]
CMAKE_C_COMPILER=${CMAKE_C_COMPILER}")
+message(STATUS "[Gluten vcpkg diagnostics]
CMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}")
+message(STATUS "[Gluten vcpkg diagnostics] c++=${_GLUTEN_DIAGNOSTIC_CXX}")
+message(STATUS "[Gluten vcpkg diagnostics] flex=${_GLUTEN_DIAGNOSTIC_FLEX}")
+message(STATUS "[Gluten vcpkg diagnostics]
FlexLexer.h=${_GLUTEN_DIAGNOSTIC_FLEX_INCLUDE_DIR}")
+unset(_GLUTEN_DIAGNOSTIC_CXX)
+unset(_GLUTEN_DIAGNOSTIC_FLEX)
+unset(_GLUTEN_DIAGNOSTIC_FLEX_INCLUDE_DIR)
Review Comment:
`find_program(... NO_CACHE)` / `find_path(... NO_CACHE)` is not supported by
this repo’s CMake minimum version (3.16) and can cause the vcpkg toolchain to
fail to configure. Also, the diagnostic `message(STATUS ...)` block currently
runs on every configure and can spam CI logs; consider gating it behind an
opt-in env var.
--
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]
