Skip to content

Hotfix: Test changes over #6114 #6115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
singiamtel wants to merge 5 commits into
base: master
Choose a base branch
from
+50 −37
Open

Hotfix: Test changes over #6114 #6115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ec61c39
gRPC: Disable prefer_system
singiamtel Feb 6, 2026
ea13886
gRPC: Modernize module
singiamtel Feb 10, 2026
cfde5ef
gRPC: Fix recipe picking up random packages from system
singiamtel Feb 6, 2026
72d89ba
O2,O2Physics: Properly pick libUV from system on macOS
singiamtel Feb 10, 2026
92d2bd8
O2Physics: Cleanup recipe
singiamtel Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 38 additions & 35 deletions grpc.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
source: https://github.com/grpc/grpc
incremental_recipe: |
cmake --build . -- ${JOBS:+-j$JOBS} install
mkdir -p $INSTALLROOT/etc/modulefiles && rsync -a --delete etc/modulefiles/ $INSTALLROOT/etc/modulefiles
prefer_system: .*
prefer_system_check: |
printf "#include \"grpcpp/version_info.h\"\n" | cc -I$(brew --prefix grpc)/include -xc++ -std=c++20 - -c -o /dev/null
mkdir -p "$INSTALLROOT"/etc/modulefiles && rsync -a --delete etc/modulefiles/ "$INSTALLROOT"/etc/modulefiles
# prefer_system: .*
# prefer_system_check: |
# printf "#include \"grpcpp/version_info.h\"\n" | cc -I$(brew --prefix grpc)/include -xc++ -std=c++20 - -c -o /dev/null

Check warning on line 20 in grpc.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

line too long (121 > 120 characters) [yl:line-length]

Check warning on line 20 in grpc.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

line too long (121 > 120 characters) [yl:line-length]
prepend_path:
PKG_CONFIG_PATH: "$GRPC_ROOT/lib/pkgconfig"
---
Expand All @@ -38,37 +38,41 @@
;;
esac

echo "OPENSSL_ROOT : $OPENSSL_ROOT"
echo "OPENSSL_REVISION: $OPENSSL_REVISION"
export CMAKE_FIND_USE_SYSTEM_ENVIRONMENT_PATH=OFF

# Build CMAKE_PREFIX_PATH without trailing colons from unset variables
CMAKE_PREFIX_PATH_components="$ABSEIL_ROOT/cmake:$PROTOBUF_ROOT/cmake"
Copy link
Member

@ktf ktf Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not

Suggested change
CMAKE_PREFIX_PATH_components="$ABSEIL_ROOT/cmake:$PROTOBUF_ROOT/cmake"
CMAKE_PREFIX_PATH="$ABSEIL_ROOT/cmake:$PROTOBUF_ROOT/cmake${C_ARES_ROOT:+:$C_ARES_ROOT}"

?
Why using a different name from the CMake define?

[[ $C_ARES_ROOT ]] && CMAKE_PREFIX_PATH_components="$CMAKE_PREFIX_PATH_components:$C_ARES_ROOT"

Check notice on line 45 in grpc.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Prefer explicit -n to check non-empty string (or use =/-ne to check boolean/integer). [SC2244]

Check notice on line 45 in grpc.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Prefer explicit -n to check non-empty string (or use =/-ne to check boolean/integer). [SC2244]



cmake $SOURCEDIR \
-G Ninja \
${CXXSTD:+-DCMAKE_CXX_STANDARD=$CXXSTD} \
-DCMAKE_INSTALL_PREFIX=$INSTALLROOT \
-DCMAKE_PREFIX_PATH=$ABSEIL_ROOT/cmake:$PROTOBUF_ROOT/cmake \
-DgRPC_BUILD_TESTS=OFF \
-DBUILD_SHARED_LIBS=ON \
-DgRPC_SSL_PROVIDER=package \
-DgRPC_ZLIB_PROVIDER=package \
-DgRPC_GFLAGS_PROVIDER=package \
-DgRPC_PROTOBUF_PROVIDER=package \
-DgRPC_ABSL_PROVIDER=package \
-DgRPC_BENCHMARK_PROVIDER=package \
-DgRPC_BUILD_GRPC_CSHARP_PLUGIN=OFF \
-DgRPC_BUILD_GRPC_OBJECTIVE_C_PLUGIN=OFF \
-DgRPC_BUILD_GRPC_PHP_PLUGIN=OFF \
-DgRPC_BUILD_GRPC_NODE_PLUGIN=OFF \
-DgRPC_BUILD_GRPC_CPP_PLUGIN=ON \
-DgRPC_BUILD_CSHARP_EXT=OFF \
-DgRPC_RE2_PROVIDER=package \
${OPENSSL_ROOT:+-DOPENSSL_ROOT_DIR=$OPENSSL_ROOT} \
${OPENSSL_ROOT:+-DOpenSSL_ROOT="$OPENSSL_ROOT"} \
${OPENSSL_ROOT:+-DOPENSSL_INCLUDE_DIRS=$OPENSSL_ROOT/include} \
cmake "$SOURCEDIR" \
-G Ninja \
${CXXSTD:+-DCMAKE_CXX_STANDARD=$CXXSTD} \
-DCMAKE_INSTALL_PREFIX=$INSTALLROOT \
-DCMAKE_PREFIX_PATH=$CMAKE_PREFIX_PATH_components \
Copy link
Member

@ktf ktf Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above...

Suggested change
-DCMAKE_PREFIX_PATH=$CMAKE_PREFIX_PATH_components \
-DCMAKE_PREFIX_PATH=$CMAKE_PREFIX_PATH \

-DgRPC_BUILD_TESTS=OFF \
-DBUILD_SHARED_LIBS=ON \
-DgRPC_SSL_PROVIDER=package \
-DgRPC_ZLIB_PROVIDER=package \
-DgRPC_GFLAGS_PROVIDER=package \
${PROTOBUF_ROOT:+-DProtobuf_DIR=${PROTOBUF_ROOT}} \
${ABSEIL_ROOT:+-Dabsl_DIR=$ABSEIL_ROOT} \
${C_ARES_ROOT:+-Dc-ares_DIR=$C_ARES_ROOT} \
-Dre2_DIR=${RE2_ROOT:-$(brew --prefix re2)/lib/cmake/re2} \
Copy link
Member

@ktf ktf Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail when not on macOS, no?

-DgRPC_PROTOBUF_PROVIDER=package \
-DgRPC_ABSL_PROVIDER=package \
-DgRPC_BENCHMARK_PROVIDER=package \
-DgRPC_BUILD_GRPC_CSHARP_PLUGIN=OFF \
-DgRPC_BUILD_GRPC_OBJECTIVE_C_PLUGIN=OFF \
-DgRPC_BUILD_GRPC_PHP_PLUGIN=OFF \
-DgRPC_BUILD_GRPC_NODE_PLUGIN=OFF \
-DgRPC_BUILD_GRPC_CPP_PLUGIN=ON \
-DgRPC_BUILD_CSHARP_EXT=OFF \
-DgRPC_RE2_PROVIDER=package \
${OPENSSL_ROOT:+-DOPENSSL_ROOT_DIR=$OPENSSL_ROOT} \
${OPENSSL_ROOT:+-DOpenSSL_ROOT="$OPENSSL_ROOT"} \
${OPENSSL_ROOT:+-DOPENSSL_INCLUDE_DIRS=$OPENSSL_ROOT/include} \
${OPENSSL_ROOT:+-DOPENSSL_LIBRARIES=$OPENSSL_ROOT/lib/libssl.$SONAME;$OPENSSL_ROOT/lib/libcrypto.$SONAME} \
-DgRPC_CARES_PROVIDER=package \
-DgRPC_CARES_PROVIDER=package \
$extra_cmake_variables

cmake --build . -- ${JOBS:+-j$JOBS} install
Expand All @@ -85,6 +89,5 @@


#ModuleFile
mkdir -p etc/modulefiles
alibuild-generate-module --bin --lib > etc/modulefiles/$PKGNAME
mkdir -p $INSTALLROOT/etc/modulefiles && rsync -a --delete etc/modulefiles/ $INSTALLROOT/etc/modulefiles
mkdir -p "$INSTALLROOT/etc/modulefiles"
alibuild-generate-module --bin --lib >"$INSTALLROOT/etc/modulefiles/$PKGNAME"
Copy link
Member

@ktf ktf Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break the incremental recipe. Why changing it?

2 changes: 2 additions & 0 deletions o2.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@
[[ ! $PROTOBUF_ROOT ]] && PROTOBUF_ROOT=`brew --prefix protobuf`
[[ ! $GLFW_ROOT ]] && GLFW_ROOT=`brew --prefix glfw`
[[ ! $FMT_ROOT ]] && FMT_ROOT=`brew --prefix fmt`
[[ ! $LIBUV_ROOT ]] && LIBUV_ROOT=`brew --prefix libuv`

Check notice on line 194 in o2.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Use $(...) notation instead of legacy backticks `...`. [SC2006]

Check notice on line 194 in o2.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Prefer explicit -n to check non-empty string (or use =/-ne to check boolean/integer). [SC2244]

Check notice on line 194 in o2.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Use $(...) notation instead of legacy backticks `...`. [SC2006]

Check notice on line 194 in o2.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Prefer explicit -n to check non-empty string (or use =/-ne to check boolean/integer). [SC2244]
;;
esac

Expand Down Expand Up @@ -243,6 +244,7 @@
-DENABLE_CUDA="${O2_GPU_CUDA_AVAILABLE:-AUTO}" \
-DENABLE_HIP="${O2_GPU_ROCM_AVAILABLE:-AUTO}" \
-DENABLE_OPENCL="${O2_GPU_OPENCL_AVAILABLE:-AUTO}" \
-DCMAKE_IGNORE_PATH="/opt/homebrew/include" \
${O2_GPU_ROCM_AVAILABLE_ARCH:+-DHIP_AMDGPUTARGET="${O2_GPU_ROCM_AVAILABLE_ARCH}"} \
${O2_GPU_CUDA_AVAILABLE_ARCH:+-DCUDA_COMPUTETARGET="${O2_GPU_CUDA_AVAILABLE_ARCH}"} \
${CURL_ROOT:+-DCURL_ROOT=$CURL_ROOT} \
Expand Down
12 changes: 10 additions & 2 deletions o2physics.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@
---
#!/bin/sh

case $ARCHITECTURE in
osx*)
# If we preferred system tools, we need to make sure we can pick them up.
[[ ! $LIBUV_ROOT ]] && LIBUV_ROOT="$(brew --prefix libuv)"

Check notice on line 27 in o2physics.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Prefer explicit -n to check non-empty string (or use =/-ne to check boolean/integer). [SC2244]

Check notice on line 27 in o2physics.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Prefer explicit -n to check non-empty string (or use =/-ne to check boolean/integer). [SC2244]
;;
esac

# When O2 is built against Gandiva (from Arrow), then we need to use
# -DLLVM_ROOT=$CLANG_ROOT, since O2's CMake calls into Gandiva's
# -CMake, which requires it.
Expand All @@ -29,6 +36,7 @@
${CMAKE_BUILD_TYPE:+"-DCMAKE_BUILD_TYPE=$CMAKE_BUILD_TYPE"} \
${CXXSTD:+"-DCMAKE_CXX_STANDARD=$CXXSTD"} \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
-DCMAKE_IGNORE_PATH="/opt/homebrew/include" \
${CLANG_ROOT:+-DLLVM_ROOT="$CLANG_ROOT"} \
${ONNXRUNTIME_ROOT:+-DONNXRuntime_DIR=$ONNXRUNTIME_ROOT} \
${FASTJET_ROOT:+-Dfjcontrib_ROOT="$FASTJET_ROOT"} \
Expand All @@ -41,10 +49,10 @@
cmake --build . -- ${JOBS+-j $JOBS} ${NINJA_ALICE_REVISION:+--keep-free-memory 4G} ${O2PHYSICS_COMPONENTS:-install}

# export compile_commands.json in (taken from o2.sh)
DEVEL_SOURCES="$(readlink $SOURCEDIR || echo $SOURCEDIR)"
DEVEL_SOURCES="$(readlink "$SOURCEDIR" || echo "$SOURCEDIR")"
Copy link
Member

@ktf ktf Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated. Please move them to a separate PR.

if [ "$DEVEL_SOURCES" != "$SOURCEDIR" ]; then
perl -p -i -e "s|$SOURCEDIR|$DEVEL_SOURCES|" compile_commands.json
ln -sf $BUILDDIR/compile_commands.json $DEVEL_SOURCES/compile_commands.json
ln -sf "$BUILDDIR"/compile_commands.json "$DEVEL_SOURCES"/compile_commands.json
Copy link
Member

@ktf ktf Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated. Please move them to a separate PR.

fi

# Modulefile
Expand Down