Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial Support for MacOS IL2CPP Builds #326

Merged
merged 52 commits into from
Feb 2, 2022
Merged

Initial Support for MacOS IL2CPP Builds #326

merged 52 commits into from
Feb 2, 2022

Conversation

AndrewKahr
Copy link
Member

@AndrewKahr AndrewKahr commented Jan 26, 2022

Changes

  • Ported Linux scripts for use on macOS runner
  • Added bash script to install unity hub/editor on the macOS runner in lieu of docker support
  • Updated action to support macOS and call appropriate bootstrap script instead of docker container
  • Adds Unity-Changeset to yarn dependencies for getting the Unity changeset given the version
  • Fixes building library during activation. Changes backported to Windows as well.

Checklist

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

@github-actions
Copy link

Cat Gif

@codecov-commenter
Copy link

codecov-commenter commented Jan 26, 2022

Codecov Report

Merging #326 (bc513b8) into main (9f13e1d) will decrease coverage by 0.06%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #326      +/-   ##
==========================================
- Coverage   45.12%   45.06%   -0.07%     
==========================================
  Files          51       51              
  Lines        1476     1478       +2     
  Branches      239      240       +1     
==========================================
  Hits          666      666              
- Misses        806      808       +2     
  Partials        4        4              
Impacted Files Coverage Δ
src/model/docker.ts 15.38% <ø> (ø)
src/model/action.ts 75.75% <33.33%> (-4.89%) ⬇️

@webbertakken
Copy link
Member

What is the status of this? Will you let us know when it's ready for review?

Did you get it to work?

@AndrewKahr
Copy link
Member Author

@webbertakken Apologies, I forgot to re-request a review. It is ready for a new review. On my fork it successfully built the test-project for all platforms.

I'm trying to now merge upstream into my branch and am having some issues. When committing, jest fails to find tests, and the commit is canceled. When I run yarn test several tests fail for system.test.ts where it is unable to run commands. I also get a lot of these errors: Command failed: gh auth status -t 'gh' is not recognized as an internal or external command, operable program or batch file.
I reran yarn and yarn build and this continues to happen. Is there an issue with the tests right now or am I doing something wrong?

@webbertakken
Copy link
Member

Oh wow it's already working! Nice job.

What you could try is run yarn then git commit -am "chore: update dist files" --no-verify to get the commit in first, and push it.

After that you can run yarn test and yarn build again I think.

Copy link
Member

@webbertakken webbertakken left a comment

Choose a reason for hiding this comment

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

LGTM

One thing I was wondering about the entrypoint. Ideally the entrypoint (which runs inside the container) would not know about the "action folder" (which is a concept that ideally only exists outside the of the container) but rather just copy in the right files in the dockerfile. But I'm not sure if that would work here?

.github/workflows/mac-build-tests.yml Outdated Show resolved Hide resolved
.github/workflows/mac-build-tests.yml Outdated Show resolved Hide resolved
dist/BlankProject/Packages/packages-lock.json Outdated Show resolved Hide resolved
Comment on lines +1 to +4
{
"dependencies": {
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Pretty cool that this works :)

src/model/platform-setup/setup-mac.ts Outdated Show resolved Hide resolved
src/model/platform-setup/setup-mac.ts Outdated Show resolved Hide resolved
AndrewKahr and others added 6 commits February 1, 2022 15:40
Co-authored-by: Webber Takken <webber.nl@gmail.com>
Co-authored-by: Webber Takken <webber.nl@gmail.com>
Co-authored-by: Webber Takken <webber.nl@gmail.com>
Co-authored-by: Webber Takken <webber.nl@gmail.com>
Co-authored-by: Webber Takken <webber.nl@gmail.com>
@AndrewKahr
Copy link
Member Author

AndrewKahr commented Feb 1, 2022

One thing I was wondering about the entrypoint. Ideally the entrypoint (which runs inside the container) would not know about the "action folder" (which is a concept that ideally only exists outside the of the container) but rather just copy in the right files in the dockerfile.

Yea so if this were in the container it would be copied to root and be ready to go for the script. I had considered having it copied to the home directory to mimic root but that would require us to write a recursive folder copy function in ts unless we want to have an additional bash script. We also still have to modify the scripts so they start with ~/ so I found simply adding an env variable with the base path to be simpler.

@AndrewKahr
Copy link
Member Author

After that you can run yarn test and yarn build again I think.

Gave it another go, yarn build is fine, but yarn test still blows up. Can you try running it on your own machine? I'm wondering if it's just my setup is broken or if the tests are broken now.

@webbertakken
Copy link
Member

After that you can run yarn test and yarn build again I think.

Gave it another go, yarn build is fine, but yarn test still blows up. Can you try running it on your own machine? I'm wondering if it's just my setup is broken or if the tests are broken now.

I'm getting 10 failures.

image

I believe they may be related to #310 as I'm getting the same errors on main.

@frostebite I saw you made some changes to tests and also used gh auth. Perhaps you know why tests are failing?

@webbertakken
Copy link
Member

webbertakken commented Feb 2, 2022

@AndrewKahr note that the workflow is failing because of formatting issues.

I'll merge it after that is fixed.

@AndrewKahr
Copy link
Member Author

@webbertakken Fixed 😃

@webbertakken webbertakken merged commit c7907c7 into game-ci:main Feb 2, 2022
@vinaysshenoy
Copy link

Question: Does this mean we will be able to run the Unity build action on a self-hosted M1 Mac Mini machine soon?

@webbertakken
Copy link
Member

Question: Does this mean we will be able to run the Unity build action on a self-hosted M1 Mac Mini machine soon?

Kindly open a new issue with a question so that everyone can find it. I think this is useful because it might not involve a one-liner answer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants