Skip to content
Merged
Changes from 1 commit
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ jobs:
with:
persist-credentials: false
- name: Build and test
run: python3 Platforms/Android ci --fast-ci ${{ matrix.arch }}-linux-android
run: JAVA_HOME="${JAVA_HOME_21_X64:-$JAVA_HOME_21_arm64}" python3 Platforms/Android ci --fast-ci ${{ matrix.arch }}-linux-android

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

In my tests, Gradle only needs JAVA_HOME.

But would it be more consistent/safer to also set the PATH env var?

Suggested change
run: JAVA_HOME="${JAVA_HOME_21_X64:-$JAVA_HOME_21_arm64}" python3 Platforms/Android ci --fast-ci ${{ matrix.arch }}-linux-android
run: JAVA_HOME="${JAVA_HOME_21_X64:-$JAVA_HOME_21_arm64}" PATH="$JAVA_HOME/bin:$PATH" python3 Platforms/Android ci --fast-ci ${{ matrix.arch }}-linux-android

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My understanding is that JAVA_HOME should be all that is needed (or, more strictly: either will work; both is redundant but won't break anything; setting JAVA_HOME is a little cleaner because it only affects the Java configuration).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with only using JAVA_HOME, but according to the runner documentation, the environment variable ends with _X64 even on the ARM64 runners.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, I see it's different on the macOS runner. In that case, I'm fine with this PR as long as the README is updated as mentioned below.


build-ios:
name: iOS
Expand Down
Loading