Copilot commented on code in PR #2455:
URL: https://github.com/apache/orc/pull/2455#discussion_r2658924566
##########
.github/workflows/build_and_test.yml:
##########
@@ -73,6 +73,30 @@ jobs:
distribution: zulu
java-version: ${{ matrix.java }}
cache: 'maven'
+ - name: Install CMake
+ if: matrix.os == 'macos-14'
+ run: |
+ # Pin cmake to 3.31.6 due to a backward compatibility issue
+ cmake_commit="b4e46db74e74a8c1650b38b1da222284ce1ec5ce"
+ tap_name="local/pinned"
+
+ echo "Creating local tap (no git)..."
+ brew tap-new --no-git "$tap_name" >/dev/null
+
+ cmake_formula_dir="$(brew --repo "$tap_name")/Formula"
+ mkdir -p "$cmake_formula_dir"
+
+
cmake_rb_link="https://raw.githubusercontent.com/Homebrew/homebrew-core/$cmake_commit/Formula/c/cmake.rb";
+ cmake_rb_path="$cmake_formula_dir/cmake.rb"
+
+ echo "Downloading cmake.rb from $cmake_rb_link"
+ curl -fsSL "$cmake_rb_link" -o "$cmake_rb_path"
+
+ echo "uninstalling existing cmake..."
+ brew uninstall cmake
Review Comment:
The 'brew uninstall cmake' command will fail if cmake is not already
installed or has already been uninstalled. Consider adding error handling to
continue even if the uninstall fails, such as using '|| true' or checking if
cmake is installed first.
```suggestion
brew uninstall cmake || true
```
##########
.github/workflows/build_and_test.yml:
##########
@@ -73,6 +73,30 @@ jobs:
distribution: zulu
java-version: ${{ matrix.java }}
cache: 'maven'
+ - name: Install CMake
+ if: matrix.os == 'macos-14'
+ run: |
+ # Pin cmake to 3.31.6 due to a backward compatibility issue
+ cmake_commit="b4e46db74e74a8c1650b38b1da222284ce1ec5ce"
+ tap_name="local/pinned"
+
+ echo "Creating local tap (no git)..."
+ brew tap-new --no-git "$tap_name" >/dev/null
+
+ cmake_formula_dir="$(brew --repo "$tap_name")/Formula"
+ mkdir -p "$cmake_formula_dir"
+
+
cmake_rb_link="https://raw.githubusercontent.com/Homebrew/homebrew-core/$cmake_commit/Formula/c/cmake.rb";
+ cmake_rb_path="$cmake_formula_dir/cmake.rb"
+
+ echo "Downloading cmake.rb from $cmake_rb_link"
+ curl -fsSL "$cmake_rb_link" -o "$cmake_rb_path"
+
+ echo "uninstalling existing cmake..."
Review Comment:
Inconsistent capitalization in echo messages. The message on line 95 starts
with lowercase 'uninstalling' while other messages on lines 83, 92, and 98
start with uppercase. Consider using consistent capitalization across all echo
statements for better code consistency.
```suggestion
echo "Uninstalling existing cmake..."
```
##########
.github/workflows/build_and_test.yml:
##########
@@ -73,6 +73,30 @@ jobs:
distribution: zulu
java-version: ${{ matrix.java }}
cache: 'maven'
+ - name: Install CMake
+ if: matrix.os == 'macos-14'
+ run: |
+ # Pin cmake to 3.31.6 due to a backward compatibility issue
+ cmake_commit="b4e46db74e74a8c1650b38b1da222284ce1ec5ce"
+ tap_name="local/pinned"
+
+ echo "Creating local tap (no git)..."
+ brew tap-new --no-git "$tap_name" >/dev/null
+
+ cmake_formula_dir="$(brew --repo "$tap_name")/Formula"
+ mkdir -p "$cmake_formula_dir"
+
+
cmake_rb_link="https://raw.githubusercontent.com/Homebrew/homebrew-core/$cmake_commit/Formula/c/cmake.rb";
+ cmake_rb_path="$cmake_formula_dir/cmake.rb"
+
+ echo "Downloading cmake.rb from $cmake_rb_link"
+ curl -fsSL "$cmake_rb_link" -o "$cmake_rb_path"
+
+ echo "uninstalling existing cmake..."
+ brew uninstall cmake
+
+ echo "Installing cmake 3.31.6 from custom tap..."
+ brew install "$tap_name/cmake"
Review Comment:
The script does not verify that the correct CMake version (3.31.6) was
actually installed. Consider adding a verification step after installation,
such as running 'cmake --version' and checking the output to ensure the
installation was successful and the correct version is available.
```suggestion
brew install "$tap_name/cmake"
echo "Verifying installed CMake version..."
cmake_version="$(cmake --version | head -n 1 | awk '{print $3}')"
echo "CMake version reported by cmake --version: ${cmake_version}"
if [ "$cmake_version" != "3.31.6" ]; then
echo "Error: Expected CMake version 3.31.6 but found
${cmake_version}."
exit 1
fi
```
--
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]
