esafak commented on code in PR #2357:
URL: https://github.com/apache/fory/pull/2357#discussion_r2159854203
##########
.github/workflows/release.yaml:
##########
@@ -29,7 +29,7 @@ jobs:
strategy:
matrix:
python-version: [3.8, 3.9, "3.10", 3.11, 3.12, 3.13]
- os: [ubuntu-latest, macos-13, macos-14, windows-2022] # macos-13:
x86, macos-14: arm64
+ os: [ubuntu-latest, ubuntu-24.04-arm, macos-13, macos-14,
windows-2022] # macos-13: x86, macos-14: arm64
Review Comment:
Sure! That means I'll have to remove the x86 hard-coding in `_run_cpp`
##########
python/setup.py:
##########
@@ -43,7 +44,12 @@ class BinaryDistribution(Distribution):
def __init__(self, attrs=None):
super().__init__(attrs=attrs)
if BAZEL_BUILD_EXT:
- subprocess.check_call(["bazel", "build", "-s", "//:cp_fory_so"])
+ bazel_args = ["bazel", "build", "-s"]
+ arch = platform.machine().lower()
+ if arch in ("x86_64", "amd64"):
+ bazel_args += ["--config=x86_64"]
Review Comment:
`x86_64` refers to the `build:x86_64` configs I added to .bazelrc
They used to be added by default. I changed that in order not to invoke them
in ARM, where they do not exist and fail the build.
##########
ci/run_ci.sh:
##########
@@ -46,41 +46,65 @@ install_pyfory() {
popd
}
+get_bazel_version() {
+ cat "$ROOT/.bazelversion"
+}
+
install_bazel() {
if command -v bazel >/dev/null; then
echo "existing bazel location $(which bazel)"
echo "existing bazel version $(bazel version)"
+ return
fi
- # GRPC support bazel 6.3.2
https://grpc.github.io/grpc/core/md_doc_bazel_support.html
- UNAME_OUT="$(uname -s)"
- case "${UNAME_OUT}" in
- Linux*) MACHINE=linux;;
- Darwin*) MACHINE=darwin;;
- *) echo "unknown machine: $UNAME_OUT"
+
+ ARCH="$(uname -m)"
+ OPERATING_SYSTEM="$(uname -s)"
+
+ # Normalize architecture names
+ case "${ARCH}" in
+ x86_64|amd64) ARCH="x86_64" ;;
+ aarch64|arm64) ARCH="arm64" ;;
+ *) echo "Unsupported architecture: $ARCH"; exit 1 ;;
esac
-
URL="https://github.com/bazelbuild/bazel/releases/download/6.3.2/bazel-6.3.2-installer-$MACHINE-x86_64.sh"
- wget -q -O install.sh $URL
- chmod +x install.sh
- set +x
- ./install.sh --user
- source ~/.bazel/bin/bazel-complete.bash
- set -x
- export PATH=~/bin:$PATH
- echo "$HOME/bin/bazel version: $(~/bin/bazel version)"
- rm -f install.sh
- VERSION=`bazel version`
- echo "bazel version: $VERSION"
+
+ # Handle OS-specific logic. Windows handled elsewhere
+ case "${OPERATING_SYSTEM}" in
+ Linux*) OS="linux" ;;
+ Darwin*) OS="darwin" ;;
+ *) echo "Unsupported OS: $OPERATING_SYSTEM"; exit 1 ;;
+ esac
+
+ BAZEL_VERSION=$(get_bazel_version)
+ BAZEL_DIR="/usr/local/bin"
+
+ # Construct platform-specific URL
+
BINARY_URL="https://github.com/bazelbuild/bazel/releases/download/${BAZEL_VERSION}/bazel-${BAZEL_VERSION}-${OS}-${ARCH}"
+
+ echo "Downloading bazel from: $BINARY_URL"
+ sudo wget -q -O "$BAZEL_DIR/bazel" "$BINARY_URL" || { echo "Failed to
download bazel"; exit 1; }
+
+ sudo chmod +x "$BAZEL_DIR/bazel"
+
+ # Add to current shell's PATH
+ export PATH="$BAZEL_DIR:$PATH"
+
+ # Verify installation
+ echo "Checking bazel installation..."
+ bazel version || { echo "Bazel installation verification failed"; exit 1; }
+
+ # Configure number of jobs based on memory
if [[ "$MACHINE" == linux ]]; then
- MEM=`cat /proc/meminfo | grep MemTotal | awk '{print $2}'`
- JOBS=`expr $MEM / 1024 / 1024 / 3`
- echo "build --jobs="$JOBS >> ~/.bazelrc
+ MEM=$(grep MemTotal < /proc/meminfo | awk '{print $2}')
+ JOBS=$(( MEM / 1024 / 1024 / 3 ))
+ echo "build --jobs=$JOBS" >> /etc/bazel.bazelrc
Review Comment:
I will restore it! It seemed to work locally.
##########
ci/deploy.sh:
##########
@@ -122,7 +122,15 @@ build_pyfory() {
echo "Install pyfory"
# Fix strange installed deps not found
pip install setuptools -U
- bazel build //:cp_fory_so
+
+ # Detect host architecture and only pass x86_64 config when appropriate
+ ARCH=$(uname -m)
+ if [[ "$ARCH" == "x86_64" || "$ARCH" == "amd64" ]]; then
+ bazel build --config=x86_64 //:cp_fory_so
Review Comment:
How would you like to automatically apply the right build flags from
.bazelrc? We can't leave the x86 flags enabled in ARM runs.
--
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]