Copilot commented on code in PR #2018:
URL: https://github.com/apache/auron/pull/2018#discussion_r2867295015
##########
dev/reformat:
##########
@@ -32,6 +32,29 @@ done
MODE=pre
SCALA_PROFILE=scala-2.12
+function set_java_home() {
+ local version="$1"
+ local mac_version="$version"
+ if [[ "${version}" == "8" ]]; then
+ mac_version="1.8"
+ fi
+
+ if [[ "$(uname)" == "Darwin" ]] && [[ -x /usr/libexec/java_home ]]; then
+ JAVA_HOME=$(/usr/libexec/java_home -v "${mac_version}" 2>/dev/null || true)
+ elif [[ -n "${JAVA_HOME_${version}_X64:-}" ]]; then
+ JAVA_HOME="${JAVA_HOME_${version}_X64}"
+ elif [[ -n "${JAVA_HOME_${version}:-}" ]]; then
+ JAVA_HOME="${JAVA_HOME_${version}}"
Review Comment:
The bash parameter expansions `${JAVA_HOME_${version}_X64:-}` /
`${JAVA_HOME_${version}:-}` are invalid in bash and will trigger `bad
substitution` at runtime. To read `JAVA_HOME_8_X64` / `JAVA_HOME_17_X64`, use
indirect expansion (e.g., build the var name in a local and access it via
`${!var}`) or another safe lookup approach.
```suggestion
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
JAVA_HOME="${!java_home_version_x64_var}"
elif [[ -n "${!java_home_version_var:-}" ]]; then
JAVA_HOME="${!java_home_version_var}"
fi
```
##########
dev/reformat:
##########
@@ -32,6 +32,29 @@ done
MODE=pre
SCALA_PROFILE=scala-2.12
+function set_java_home() {
+ local version="$1"
+ local mac_version="$version"
+ if [[ "${version}" == "8" ]]; then
+ mac_version="1.8"
+ fi
+
+ if [[ "$(uname)" == "Darwin" ]] && [[ -x /usr/libexec/java_home ]]; then
+ JAVA_HOME=$(/usr/libexec/java_home -v "${mac_version}" 2>/dev/null || true)
+ elif [[ -n "${JAVA_HOME_${version}_X64:-}" ]]; then
+ JAVA_HOME="${JAVA_HOME_${version}_X64}"
+ elif [[ -n "${JAVA_HOME_${version}:-}" ]]; then
+ JAVA_HOME="${JAVA_HOME_${version}}"
+ fi
+
+ if [[ -n "${JAVA_HOME:-}" ]]; then
+ export JAVA_HOME
+ export PATH="${JAVA_HOME}/bin:${PATH}"
+ else
+ echo "JAVA_HOME for JDK ${version} not found; using existing
JAVA_HOME=${JAVA_HOME:-<unset>}"
+ fi
Review Comment:
`set_java_home` mutates global `JAVA_HOME`/`PATH` in a way that can be hard
to reason about across loop iterations: it overwrites `JAVA_HOME` even when the
lookup fails, and it prepends `${JAVA_HOME}/bin` to `PATH` on every call (so
`PATH` grows and can contain multiple JDK bins). Consider computing the
candidate JAVA_HOME in a local variable and only exporting/updating PATH when
it is found; also consider resetting PATH to an original baseline before
prepending.
--
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]
