Add androidTargetSdkVersion as an option to update the Android SDK

[davidmfinol]

Changes

• Add androidTargetSdkVersion as an option to update the Android SDK

Checklist

• Read the contribution guide and accept the code of conduct
• Readme (updated or not needed)
• Tests (added, updated or not needed)

[github-actions]

[codecov-commenter]

Codecov Report

Merging #298 (3cb9821) into main (11a0d09) will increase coverage by 0.33%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #298      +/-   ##
==========================================
+ Coverage   56.38%   56.72%   +0.33%     
==========================================
  Files          23       23              
  Lines         775      781       +6     
  Branches      156      158       +2     
==========================================
+ Hits          437      443       +6     
  Misses        337      337              
  Partials        1        1              

Impacted Files	Coverage Δ
src/model/docker.ts	17.64% <ø> (ø)
src/model/android-versioning.ts	100.00% <100.00%> (ø)
src/model/build-parameters.ts	100.00% <100.00%> (ø)
src/model/input.ts	100.00% <100.00%> (ø)

[davidmfinol]

A run where I don't set the androidTargetSdkVersion: https://github.com/finol-digital/Card-Game-Simulator/actions/runs/1474689325
And one where I do set the androidTargetSdkVersion to level 30: https://github.com/finol-digital/Card-Game-Simulator/actions/runs/1474802849

EDIT: Looks like the second run didn't correctly update the SDK due to missing java home; I'll check again.

[davidmfinol]

I'm getting this error:

ERROR: JAVA_HOME is set to an invalid directory: /opt/unity/Editor/Data/PlaybackEngines/AndroidPlayer/Tools/OpenJDK/Linux

Please set the JAVA_HOME variable in your environment to match the
location of your Java installation.

I tried to set the JAVA_HOME the way that it's done here: https://github.com/game-ci/docker/blob/main/editor/Dockerfile#L100
Then again, I shouldn't even need to set JAVA_HOME since it was already done there, right?
Maybe you know what I am doing wrong, @GabLeRoux ?

[webbertakken]

Amazing work, looks very clean and I like that you're only exposing one parameter and determine the rest in BuildParameters.create.

[webbertakken]

re @davidmfinol:

I'm getting this error:

ERROR: JAVA_HOME is set to an invalid directory: /opt/unity/Editor/Data/PlaybackEngines/AndroidPlayer/Tools/OpenJDK/Linux

Have you tried using $UNITY_PATH?

[davidmfinol]

Same issue when I use UNITY_PATH: https://github.com/finol-digital/Card-Game-Simulator/runs/4258191146?check_suite_focus=true#step:6:333

[davidmfinol]

I found that the JAVA_HOME for 2020.3 should be "$UNITY_PATH/Editor/Data/PlaybackEngines/AndroidPlayer/OpenJDK" (note the lack of "/Tools" and "/Linux").
With that, I've got a working build that successfully uses API Level 31 with Unity 2020.3: https://github.com/finol-digital/Card-Game-Simulator/actions/runs/1478884782

My only concern now is that this JAVA_HOME path doesn't match the value I saw in our Dockerfile (I'm assuming maybe because of differing Unity versions?), and I'm not sure how to confirm what the correct path should be for each and every version of Unity.

Thoughts on how to proceed?
We've seen that Unity will (silently?) continue with the default SDK version if JAVA_HOME isn't correctly set.

[jsteinich]

That referenced line in the editor docker file is actually only used by older versions of Unity. Lines 155-158 are what is used on newer versions (certainly a bit more convoluted).

One thing I've ran into is depending on how the underlying container is accessed impacts whether or not the underlying environment variables have been exported (set in a unity wrapper script or ~/.bashrc only).

Ideally it would be worth figuring out why neither the wrapper script or ~/.bashrc is being invoked to currently set the variables.

We've used an ugly workaround by invoking the sdkmanager as follows:
"$(awk -F'=' '/ANDROID_HOME=/{print $2}' /root/.bashrc)/tools/bin/sdkmanager".
Our usage has a different version of java set for JAVA_HOME, but the same idea could be applied to JAVA_HOME.
Note: We're using that workaround because we are actually building our own editor images on top of the base ones with extra tools added.

[davidmfinol]

"$(awk -F'=' '/ANDROID_HOME=/{print $2}' /root/.bashrc)/tools/bin/sdkmanager".
Our usage has a different version of java set for JAVA_HOME, but the same idea could be applied to JAVA_HOME.

I'm not skilled with bash, so please help me understand.
It looks like awk -F'=' '/ANDROID_HOME=/{print $2}' /root/.bashrc will read the value of ANDROID_HOME from /root/.bashrc, right?
Is the JAVA_HOME value also inside /root/.bashrc?
Could I do something like export JAVA_HOME="$(awk -F'=' '/JAVA_HOME=/{print $2}' /root/.bashrc)"?

[jsteinich]

It looks like awk -F'=' '/ANDROID_HOME=/{print $2}' /root/.bashrc will read the value of ANDROID_HOME from /root/.bashrc, right?

That is correct.

Is the JAVA_HOME value also inside /root/.bashrc?
Could I do something like export JAVA_HOME="$(awk -F'=' '/JAVA_HOME=/{print $2}' /root/.bashrc)"?

Yep.

Ideally it would be worth figuring out why neither the wrapper script or ~/.bashrc is being invoked to currently set the variables.

Since the android sdk step is done prior to invoking the unity-editor wrapper script, only /root/.bashrc would have a chance to run; however, the docker container is ran with --volume /home/runner/work/_temp/_github_home:/root, so that file is effectively clobbered.

[davidmfinol]

I get this error: awk: cannot open /root/.bashrc (No such file or directory)

[jsteinich]

I get this error: awk: cannot open /root/.bashrc (No such file or directory)

Ah, yes. It's failing for the same reason that /root/.bashrc is failing in the 1st place.
Looks like the updated docker file also writes to /usr/bin/unity-editor.d/, though the exact file is named differently depending on Unity version.

I just did a quick test, and replacing export JAVA_HOME="$(awk -F'=' '/JAVA_HOME=/{print $2}' /root/.bashrc)" with export JAVA_HOME="$(awk -F'=' '/JAVA_HOME=/{print $2}' /usr/bin/unity-editor.d/*)" is working for me in the unity 2020.3.23f1-android-0.15.0 image. (Need to do a similar update for android home)

[davidmfinol]

Great, that works!
Here's my passing test run: https://github.com/finol-digital/Card-Game-Simulator/actions/runs/1497846444

If no one has any concerns, I'll merge this tomorrow.

[trumpets]

Don't wanna review an old PR, but I just tried using the new param and it seems the actions haven't been updated since this PR was merged.

Any hints on what the process could be here? Is is something an outsider can contribute or a specific process needs to be triggered?

[webbertakken]

@trumpets thanks for your question.

Ah I see that it's been a while since we've put out a new release. I'll put one out soon.

In the meantime you can use @main or @<specific commit sha> to use the very latest version. Please don't hesitate to let us know your results!

[trumpets]

Thank you @webbertakken ! I didn't know I could configure the action to use another version. Very cool!