Allow to install multiple modules in the editor image #116

shr-project · 2021-05-26T13:59:52Z

Changes

Instead of having separate image for windows, android, mac, ... you can now create just one image which will have all modules installed.
Existing module build-arg now allows to use space separated list of modules.

Checklist

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

github-actions · 2021-05-26T14:00:09Z

webbertakken

Thanks for your proposal!

I think this is worth merging, because specific builds require more than one module. I have left a few comments that would need to be addressed before doing so.

Please also note that we generally discourage our users from installing all modules in one single image as it goes against parallelisation in many cases. In many cases pulling the (compressed) image directly is just as fast as caching, however when trying to cache an image that holds all modules, it will actually slow down each build (even when cached, depending on the mechanism).

editor/Dockerfile

shr-project · 2021-05-27T20:42:03Z

Please also note that we generally discourage our users from installing all modules in one single image as it goes against parallelisation in many cases. In many cases pulling the (compressed) image directly is just as fast as caching, however when trying to cache an image that holds all modules, it will actually slow down each build (even when cached, depending on the mechanism).

Understood, I was going to use this for building SVL Simulator (https://github.com/lgsvl/simulator/blob/master/Jenkins/Jenkinsfile) where the initial checkout stages takes quite long, so instead of triggering 3 jobs for 3 different platforms in parallel it builds all 3 platforms in 1 job and the unity version isn't changed very often, so the bigger editor docker image can stay cached on the builders for relatively long.

Currently it was using UnitySetup instead of UnityHub for installation (with linux, windows, mac support all included):
https://github.com/lgsvl/simulator/blob/master/Jenkins/Dockerfile#L26
but I haven't found a way how to let UnitySetup install childModules as well (which seems to be mostly useful for android).

DEVELOPMENT.md

webbertakken · 2021-05-28T21:15:45Z

editor/Dockerfile

-  && chmod +x /usr/bin/unity-editor
+ADD editor/unity-editor /usr/bin/unity-editor
+RUN chmod 755 /usr/bin/unity-editor \
+  && mkdir /usr/bin/unity-editor.d


Adding this external file and running different .sh files inside a folder is adding a lot of unnecessary complexity.

In my opinion the best way to solve the file overwrites would have been to simply move the last line of that file after the android clauses, and make sure that each addition is only added once.

I'm sorry about having to request changes again. It is however quite important that this Dockerfile stays very clean. As we publish thousands of images with it.

But it also makes it useful for extending this script if someone needs to export few more variables in unity-editor script in another docker image which will be based on this editor.

Every /etc/profile does this with /etc/profile.d/*sh and many other scripts/configs use the same mechanism to combine various pieces together.

I would argue that this is out of scope for this proposal.

There are much easier ways of adding variables. For example:

When using Unity Builder, you can add it specifically for that workflow in its extended Dockerfile.

When not using Builder, it's still very easy to achieve by using

FROM <this image:some-version-with-specific-compatibility-fixes-maintained-by-game-ci> ENV my_var="my var"

I would argue that this is out of scope for this proposal.

Well, you wanted an elegant way in:

What we need to make this PR merge-able is to find an elegant way account for multiple modules when creating that unity-editor alias script (and the .bashrc file).

and I believe these .d directories is quite well-established elegant way.

Being able to extend it from user images is just a small bonus (yes you can export variables with ENV, but you cannot do much more, like put something in ANDROID_SDK_ROOT or use already exported variable (like ANDROID_SDK_ROOT, ANDROID_HOME, ANDROID_NDK_HOME) which will be available at runtime, but won't be in the extended Dockerfile.

If the main issue is the file being external to Dockerfile, I can create it with here-doc inside Dockerfile, it's just a bit more error prone with Dockerfile syntax getting in a way of simple here-doc.

I don't necessarily think it's a bad way of doing things, but it does increase complexity.

In my opinion my proposed solution of "simply moving the last line of the unity-editor alias file after the android clauses, and make sure that each addition is only added once" is much simpler and more appropriate for the scope of this proposal, which is about supporting multiple modules - not finding the best way to make images extensible (which may or may not be needed).

That said, lets get some additional opinions in here and decide based on what they say.

cc: @davidmfinol @frostebite @GabLeRoux

I tend to agree with @webbertakken and his explanation here: #116 (comment)

/etc/profile does this with /etc/profile.d/*sh

Indeed, but as part of the docker image here, I'm not sure I would have had the reflex to look in that folder.

Using a here-doc would be a bit more verbose indeed, but if the goal is to allow user to extend the image and if moving the last line of that file after the android clauses solves that, I would go with @webbertakken's suggestion.

editor/Dockerfile

GabLeRoux · 2021-05-30T01:51:35Z

I see that all android checks are failing above, this will most likely need to be fixed before it can be merged 👀 I haven't looked at why it's currently failing tho.

All in all, I think this contribution will be helpful in cases where a single image must be used for multiple platforms. Thanks so much for that contribution! :D

shr-project · 2021-06-09T15:59:29Z

I see that all android checks are failing above, this will most likely need to be fixed before it can be merged I haven't looked at why it's currently failing tho.

Is there some successful check since #97 was merged? The "Validate Android Utils" step fails for me even without any changes from this PR, because it depends on JAVA_HOME from ~/.bashrc while the step passes also -e HOME which will break it unless your HOME is /root (it's '/home/runner' in your CI pipeline).

2021-05-29T12:08:56.0092844Z ##[command]/usr/bin/docker run --name a33c1a032c225baa74958bdb83e761f1e8692_15bc0a --label 8a33c1 --workdir /github/workspace --rm -e UNITY_LICENSE -e CHANGESET -e MODULE -e INPUT_IMAGE -e INPUT_RUN -e INPUT_OPTIONS -e INPUT_SHELL -e INPUT_REGISTRY -e INPUT_USERNAME -e INPUT_PASSWORD -e INPUT_DOCKER_NETWORK -e HOME -e GITHUB_JOB -e GITHUB_REF -e GITHUB_SHA -e GITHUB_REPOSITORY -e GITHUB_REPOSITORY_OWNER -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RETENTION_DAYS -e GITHUB_ACTOR -e GITHUB_WORKFLOW -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -e GITHUB_EVENT_NAME -e GITHUB_SERVER_URL -e GITHUB_API_URL -e GITHUB_GRAPHQL_URL -e GITHUB_WORKSPACE -e GITHUB_ACTION -e GITHUB_EVENT_PATH -e GITHUB_ACTION_REPOSITORY -e GITHUB_ACTION_REF -e GITHUB_PATH -e GITHUB_ENV -e RUNNER_OS -e RUNNER_TOOL_CACHE -e RUNNER_TEMP -e RUNNER_WORKSPACE -e ACTIONS_RUNTIME_URL -e ACTIONS_RUNTIME_TOKEN -e ACTIONS_CACHE_URL -e GITHUB_ACTIONS=true -e CI=true -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/runner/work/_temp/_github_home":"/github/home" -v "/home/runner/work/_temp/_github_workflow":"/github/workflow" -v "/home/runner/work/_temp/_runner_file_commands":"/github/file_commands" -v "/home/runner/work/docker/docker":"/github/workspace" 8a33c1:a032c225baa74958bdb83e761f1e8692
2021-05-29T12:08:57.2402601Z mesg: ttyname failed: Inappropriate ioctl for device
2021-05-29T12:08:57.2470282Z bash: java: command not found
2021-05-29T12:08:57.2601392Z Post job cleanup.

It fails the same with docker image built from current main branch:

$ docker run -e HOME --rm -it unityci/jansa:editor-android-main /bin/bash -l -c 'java -version'
/bin/bash: java: command not found

$ docker run --rm -it unityci/jansa:editor-android-main /bin/bash -l -c 'java -version'
openjdk version "1.8.0-adoptopenjdk"
OpenJDK Runtime Environment (build 1.8.0-adoptopenjdk-jenkins_2018_05_19_01_00-b00)
OpenJDK 64-Bit Server VM (build 25.71-b00, mixed mode)

$ HOME=/root docker run -e HOME --rm -it unityci/jansa:editor-android-main /bin/bash -l -c 'java -version'
WARNING: Error loading config file: /root/.docker/config.json: open /root/.docker/config.json: permission denied
openjdk version "1.8.0-adoptopenjdk"
OpenJDK Runtime Environment (build 1.8.0-adoptopenjdk-jenkins_2018_05_19_01_00-b00)
OpenJDK 64-Bit Server VM (build 25.71-b00, mixed mode)

$ docker run -e HOME=/root --rm -it unityci/jansa:editor-android-main /bin/bash -c 'java -version'
openjdk version "1.8.0-adoptopenjdk"
OpenJDK Runtime Environment (build 1.8.0-adoptopenjdk-jenkins_2018_05_19_01_00-b00)
OpenJDK 64-Bit Server VM (build 25.71-b00, mixed mode)

$ docker run -e HOME=/foo --rm -it unityci/jansa:editor-android-main /bin/bash -l -c 'java -version'
/bin/bash: java: command not found

webbertakken · 2021-06-09T16:26:00Z

Sure, the latest workflows on main were not failing (except for 1 windows run).

The workflows are public and can be found here.

I'm running a test to verify the integrity of main right now.

webbertakken · 2021-06-09T18:38:54Z

The integrity of main looks to be fine. Only 1 iOS build failed because of an activation failure.

* fetching Unity takes very long, better show the progress during the fetch

* remove unnecessary [ call and ``

* useful to reuse the same docker image for e.g. linux+windows+android platform builds

* it has 744 after unzip: -rwxr--r-- 1 root root 6.0K Apr 24 23:13 /opt/unity/Editor/Data/PlaybackEngines/AndroidPlayer/SDK/tools/bin/sdkmanager * fixes Unity failing to use it under different UID 18:59:44 Win32Exception: ApplicationName='/opt/unity/Editor/Data/PlaybackEngines/AndroidPlayer/SDK/tools/bin/sdkmanager', CommandLine='--list', CurrentDirectory='/mnt', Native error= mono-io-layer-error (5) 18:59:44 at System.Diagnostics.Process.StartWithCreateProcess (System.Diagnostics.ProcessStartInfo startInfo) [0x002dc] in <aa976c2104104b7ca9e1785715722c9d>:0 18:59:44 at System.Diagnostics.Process.Start () [0x0003a] in <aa976c2104104b7ca9e1785715722c9d>:0 18:59:44 at (wrapper remoting-invoke-with-check) System.Diagnostics.Process.Start() 18:59:44 at UnityEditor.Utils.Program.Start (System.EventHandler exitCallback) [0x0006d] in /home/bokken/buildslave/unity/build/Editor/Mono/Utils/Program.cs:48 18:59:44 at UnityEditor.Utils.Program.Start () [0x00001] in /home/bokken/buildslave/unity/build/Editor/Mono/Utils/Program.cs:32 18:59:44 at UnityEditor.Android.Command.Run (System.Diagnostics.ProcessStartInfo psi, UnityEditor.Android.Command+WaitingForProcessToExit waitingForProcessToExit, System.String errorMsg) [0x00026] in <2c6e9a95f1dd4e06ad71afcd2684dcb7>:0 18:59:44 at UnityEditor.Android.AndroidSDKTools.RunAndroidSdkTool (System.String toolName, System.String arguments, System.Boolean updateCommand, System.String errorMsg, System.String toolsDir, System.String[] warningsToIgnore) [0x0007b] in <2c6e9a95f1dd4e06ad71afcd2684dcb7>:0 18:59:44 at UnityEditor.Android.AndroidSDKTools.ListComponentsVersions () [0x00062] in <2c6e9a95f1dd4e06ad71afcd2684dcb7>:0 18:59:44 at UnityEditor.Android.SDKManager.UpdatePackagesList () [0x00079] in <2c6e9a95f1dd4e06ad71afcd2684dcb7>:0 18:59:44 at UnityEditor.Android.SDKManager.HighestVersionInstalled (UnityEditor.Android.SDKManager+Component tool) [0x00001] in <2c6e9a95f1dd4e06ad71afcd2684dcb7>:0 18:59:44 at UnityEditor.Android.PostProcessor.Tasks.CheckAndroidSDK+SDKToolsDetector.GetVersion () [0x00006] in <2c6e9a95f1dd4e06ad71afcd2684dcb7>:0 18:59:44 at UnityEditor.Android.PostProcessor.Tasks.CheckAndroidSDK+SDKComponentDetector.Detect (System.Version minVersion, UnityEditor.Android.PostProcessor.ProgressHandler onProgress) [0x0002b] in <2c6e9a95f1dd4e06ad71afcd2684dcb7>:0 18:59:44 at UnityEditor.Android.PostProcessor.Tasks.CheckAndroidSDK.EnsureSDKComponentVersion (System.Version minVersion, UnityEditor.Android.PostProcessor.Tasks.CheckAndroidSDK+SDKComponentDetector detector) [0x00007] in <2c6e9a95f1dd4e06ad71afcd2684dcb7>:0 18:59:44 at UnityEditor.Android.PostProcessor.Tasks.CheckAndroidSDK.Execute (UnityEditor.Android.PostProcessor.PostProcessorContext context) [0x00177] in <2c6e9a95f1dd4e06ad71afcd2684dcb7>:0 18:59:44 at UnityEditor.Android.PostProcessor.PostProcessRunner.RunAllTasks (UnityEditor.Android.PostProcessor.PostProcessorContext context) [0x00078] in <2c6e9a95f1dd4e06ad71afcd2684dcb7>:0 18:59:44 at UnityEditor.Android.PostProcessAndroidPlayer.PrepareForBuild (UnityEditor.BuildOptions options, UnityEditor.BuildTarget target) [0x00084] in <2c6e9a95f1dd4e06ad71afcd2684dcb7>:0 18:59:44 at UnityEditor.Android.AndroidBuildPostprocessor.PrepareForBuild (UnityEditor.BuildOptions options, UnityEditor.BuildTarget target) [0x00001] in <2c6e9a95f1dd4e06ad71afcd2684dcb7>:0 18:59:44 at UnityEditor.PostprocessBuildPlayer.PrepareForBuild (UnityEditor.BuildOptions options, UnityEditor.BuildTargetGroup targetGroup, UnityEditor.BuildTarget target) [0x00015] in /home/bokken/buildslave/unity/build/Editor/Mono/BuildPipeline/PostprocessBuildPlayer.cs:146 18:59:44 UnityEditor.BuildPipeline:BuildPlayerInternalNoCheck(String[], String, String, BuildTargetGroup, BuildTarget, BuildOptions, String[], Boolean) 18:59:44 UnityEditor.BuildPipeline:BuildPlayerInternal(String[], String, String, BuildTargetGroup, BuildTarget, BuildOptions, String[]) (at /home/bokken/buildslave/unity/build/Editor/Mono/BuildPipeline.bindings.cs:422) 18:59:44 UnityEditor.BuildPipeline:BuildPlayer(String[], String, String, BuildTargetGroup, BuildTarget, BuildOptions, String[]) (at /home/bokken/buildslave/unity/build/Editor/Mono/BuildPipeline.bindings.cs:321) 18:59:44 UnityEditor.BuildPipeline:BuildPlayer(BuildPlayerOptions) (at /home/bokken/buildslave/unity/build/Editor/Mono/BuildPipeline.bindings.cs:295) 18:59:44 18:59:44 Error building Player: Win32Exception: ApplicationName='/opt/unity/Editor/Data/PlaybackEngines/AndroidPlayer/SDK/tools/bin/sdkmanager', CommandLine='--list', CurrentDirectory='/mnt', Native error= mono-io-layer-error (5)

* otherwise building base won't find asound.conf: Step 7/9 : ADD config/asound.conf /etc/ ADD failed: file not found in build context or excluded by .dockerignore: stat config/asound.conf: file does not exist

…ditor.d/*.sh

….bashrc * instead of duplicating the same exports as what unity-editor.d is using * also append to it instead of overwritting it in case some other module will need to source something else in future (currently it's only used by 2 different versions of android module - only one of these sections will be executed

Lets see if it fixes unityci CI step "Validate Android Utils".

editor/unity-editor

…arate file

shr-project · 2021-08-02T16:22:57Z

I have no idea why many CI jobs failed now, I see logs in the successful tests, but just "This check failed" for failed like in https://github.com/game-ci/docker/pull/116/checks?check_run_id=3185307822

webbertakken · 2021-08-02T16:58:36Z

I have no idea why many CI jobs failed now, I see logs in the successful tests, but just "This check failed" for failed like in https://github.com/game-ci/docker/pull/116/checks?check_run_id=3185307822

Likely a GitHub error. Lets rerun them.

webbertakken · 2021-08-02T19:06:14Z

Reported failure here https://github.community/t/many-runners-failing-on-different-stages-seemingly-at-random/193479

webbertakken · 2021-08-04T09:32:46Z

DEVELOPMENT.md


 ```bash
-docker build base -t base
+docker build -t base -f base/Dockerfile .


The readme can be simplified to what it was before. no need to specify the file specifically if you're not passing context. I can revert it in a followup.

* built in: https://auto-jenkins.lgsvl.net/view/Jenkins%20test/job/unityci-docker-image/12/console but https://hub.docker.com/layers/unityci/editor/ubuntu-2020.3.3f1-windows-mono-0.13.0/images/sha256-3eaa8da58fda00f5f198527339b5839a46f3af6f3f0eca8e8f1f4a0510665f64?context=explore https://hub.docker.com/layers/unityci/editor/ubuntu-2020.3.3f1-mac-mono-0.13.0/images/sha256-9ff06baedd8ff428d6151c36ea91494d70058fa2282d1fd458dcff6b1021f1cd?context=explore https://hub.docker.com/layers/unityci/editor/ubuntu-2020.3.3f1-android-0.13.0/images/sha256-c8f65a43f7436ccd082e658a9b683f0db5c81ea3c71e764e3ce886e6e9743e5e?context=explore should work as base image as well (just different base image for each platform until game-ci/docker#116 is merged upstream and some image with combination of platforms we use is on dockerhub.

webbertakken requested changes May 27, 2021

View reviewed changes

editor/Dockerfile Outdated Show resolved Hide resolved

editor/Dockerfile Show resolved Hide resolved

editor/Dockerfile Outdated Show resolved Hide resolved

editor/Dockerfile Outdated Show resolved Hide resolved

shr-project force-pushed the jansa/multiple-modules branch from 6136482 to a12a583 Compare May 27, 2021 20:28

shr-project force-pushed the jansa/multiple-modules branch 2 times, most recently from a171b6c to 0bf7a54 Compare May 28, 2021 19:12

webbertakken reviewed May 28, 2021

View reviewed changes

DEVELOPMENT.md Show resolved Hide resolved

webbertakken reviewed May 28, 2021

View reviewed changes

editor/Dockerfile Show resolved Hide resolved

shr-project force-pushed the jansa/multiple-modules branch from 5e62e96 to 0bebded Compare May 29, 2021 10:03

webbertakken mentioned this pull request Jun 4, 2021

Using SSH_AUTH_SOCK (ssh agent forwarding) to pull upm private repos game-ci/unity-builder#256

Merged

3 tasks

shr-project added 7 commits June 9, 2021 11:47

editor: show output from unity-hub when installing unity and its modules

9d425e8

* fetching Unity takes very long, better show the progress during the fetch

editor: simplify Extra steps a bit

98d64f3

* remove unnecessary [ call and ``

editor: allow to install multiple modules

263beaa

* useful to reuse the same docker image for e.g. linux+windows+android platform builds

DEVELOPMENT.md: use the repository root as context

a1757fc

* otherwise building base won't find asound.conf: Step 7/9 : ADD config/asound.conf /etc/ ADD failed: file not found in build context or excluded by .dockerignore: stat config/asound.conf: file does not exist

editor: allow to prepend environment changes through /usr/bin/unity-e…

7ffbf75

…ditor.d/*.sh

shr-project force-pushed the jansa/multiple-modules branch from 0bebded to c7aa26b Compare June 9, 2021 18:52

editor: empty the default ~/.bashrc file

200102f

Lets see if it fixes unityci CI step "Validate Android Utils".

shr-project force-pushed the jansa/multiple-modules branch from c7aa26b to 200102f Compare June 9, 2021 21:57

webbertakken reviewed Jul 19, 2021

View reviewed changes

editor/unity-editor Outdated Show resolved Hide resolved

shr-project added 2 commits July 28, 2021 12:23

editor: create /usr/bin/unity-editor inside Dockerfile instead of sep…

6024410

…arate file

base: create /etc/asound.conf inside Dockerfile instead of separate file

4c4effd

webbertakken reviewed Aug 4, 2021

View reviewed changes

webbertakken merged commit 487dc88 into game-ci:main Aug 4, 2021

Uh oh!

Allow to install multiple modules in the editor image #116

Allow to install multiple modules in the editor image #116

Uh oh!

Conversation

shr-project commented May 26, 2021

Changes

Checklist

Uh oh!

github-actions bot commented May 26, 2021

Uh oh!

webbertakken left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shr-project commented May 27, 2021

Uh oh!

Uh oh!

webbertakken May 28, 2021

Choose a reason for hiding this comment

Uh oh!

shr-project May 28, 2021

Choose a reason for hiding this comment

Uh oh!

webbertakken May 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shr-project May 29, 2021

Choose a reason for hiding this comment

Uh oh!

webbertakken May 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GabLeRoux May 30, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GabLeRoux commented May 30, 2021

Uh oh!

shr-project commented Jun 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webbertakken commented Jun 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webbertakken commented Jun 9, 2021

Uh oh!

Uh oh!

shr-project commented Aug 2, 2021

Uh oh!

webbertakken commented Aug 2, 2021

Uh oh!

webbertakken commented Aug 2, 2021

Uh oh!

webbertakken Aug 4, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

webbertakken May 28, 2021 •

edited

Loading

webbertakken May 29, 2021 •

edited

Loading

shr-project commented Jun 9, 2021 •

edited

Loading

webbertakken commented Jun 9, 2021 •

edited

Loading