Copilot commented on code in PR #2018:
URL: https://github.com/apache/auron/pull/2018#discussion_r2867402740
##########
dev/reformat:
##########
@@ -48,16 +90,31 @@ else
cargo fmt --all -q --
fi
+# Run spotless with a consistent JDK/version to avoid formatting drift.
+run_spotless scala-2.12 spark-3.5
+run_spotless scala-2.13 spark-4.0
+
# Check or format all code, including third-party code, with spark-3.5
Review Comment:
After `run_spotless` runs, the script leaves `JAVA_HOME`/`PATH` pointing at
JDK 17. The subsequent third-party Spark 3.5 build
(Celeborn/Uniffle/Paimon/Flink/Iceberg) then runs under JDK 17 because there’s
no `set_java_home 8` before that section, which contradicts the intended “Spark
3.x uses JDK 8” behavior and may change/ break the build. Consider explicitly
switching back to JDK 8 before the third-party build block (or have
`run_spotless` restore the previous JAVA_HOME/PATH).
```suggestion
# Check or format all code, including third-party code, with spark-3.5
set_java_home 8
```
##########
dev/reformat:
##########
@@ -32,11 +32,53 @@ done
MODE=pre
SCALA_PROFILE=scala-2.12
+ORIGINAL_PATH="${PATH}"
+SPOTLESS_GOAL="spotless:apply"
+if [[ "$CHECK" == "true" ]]; then
+ SPOTLESS_GOAL="spotless:check"
+fi
+function set_java_home() {
+ local version="$1"
+ local mac_version="$version"
+ local candidate=""
+ if [[ "${version}" == "8" ]]; then
+ mac_version="1.8"
+ fi
+
+ if [[ "$(uname)" == "Darwin" ]] && [[ -x /usr/libexec/java_home ]]; then
+ candidate=$(/usr/libexec/java_home -v "${mac_version}" 2>/dev/null || true)
+ else
+ local java_home_version_x64_var="JAVA_HOME_${version}_X64"
+ local java_home_version_var="JAVA_HOME_${version}"
+
+ if [[ -n "${!java_home_version_x64_var:-}" ]]; then
+ candidate="${!java_home_version_x64_var}"
+ elif [[ -n "${!java_home_version_var:-}" ]]; then
+ candidate="${!java_home_version_var}"
+ fi
+ fi
+
+ if [[ -n "${candidate}" ]]; then
+ export JAVA_HOME="${candidate}"
+ export PATH="${candidate}/bin:${ORIGINAL_PATH}"
+ else
+ echo "JAVA_HOME for JDK ${version} not found."
+ exit 1
+ fi
+}
+
+function run_spotless() {
+ local scala_profile="$1"
+ local spark_profile="$2"
+ set_java_home 17
+ "${PROJECT_DIR}"/build/mvn "${SPOTLESS_GOAL}" -P"${MODE}"
-P"${scala_profile}" -P"${spark_profile}"
+}
+
function run_maven() {
if [[ "$CHECK" == "true" ]]; then
- "${PROJECT_DIR}"/build/mvn spotless:check compile test-compile
scalafix:scalafix -Dscalafix.mode=CHECK -Dscalafix.skipTest=true
-DskipBuildNative -P"${MODE}" -P"${SCALA_PROFILE}" "$@"
+ "${PROJECT_DIR}"/build/mvn compile test-compile scalafix:scalafix
-Dscalafix.mode=CHECK -Dscalafix.skipTest=true -DskipBuildNative -P"${MODE}"
-P"${SCALA_PROFILE}" "$@"
else
- "${PROJECT_DIR}"/build/mvn spotless:apply compile test-compile
scalafix:scalafix -DskipBuildNative -P"${MODE}" -P"${SCALA_PROFILE}" "$@"
+ "${PROJECT_DIR}"/build/mvn compile test-compile scalafix:scalafix
-DskipBuildNative -P"${MODE}" -P"${SCALA_PROFILE}" "$@"
fi
Review Comment:
`run_maven` no longer invokes `spotless:check/apply`, but `mvn compile` will
still run the Spotless execution bound to the `validate` phase in the root
`pom.xml` (Spotless `apply`), under whatever JDK was selected for that Spark
profile. This can reintroduce JDK-dependent formatting drift and can also
mutate files during `--check` runs. Consider skipping Spotless during the
`run_maven` invocations (e.g., via a Spotless skip flag) and relying on the
dedicated `run_spotless` calls, or otherwise ensuring Spotless only runs under
the intended JDK.
--
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]
