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

Update Mac CI build job to handle the new targets #5

Merged

Conversation

DeBlister
Copy link
Owner

@DeBlister DeBlister commented May 29, 2022

I'm doing this in a PR so I can trigger actions quietly, and, hopefully, re-run them myself if I need to since this is all in my own fork.

DeBlister added 2 commits May 28, 2022 21:27
Add more artifacts to account for there now being multiple build targets.
Update brew if any of the artifacts can't be downloaded and cached.
Build any artifacts that could not be downloaded and cached.
Upload any artifacts that could not be downloaded and cached.
@DeBlister DeBlister changed the title Update Mac CD build job to handle the new targets Update Mac CI build job to handle the new targets May 29, 2022
@DeBlister DeBlister force-pushed the Xcode_CompileUnitTests_UpdateBuildCI branch from c114d0f to f5aa3d2 Compare May 29, 2022 01:36
DeBlister added 3 commits May 28, 2022 21:48
This reverts commit 06c9507.
There should be no need to fallback from caching to fetching the framework in these workflows; the build targets should fetch it themselves.
@DeBlister DeBlister deleted the branch Xcode_CompileUnitTests May 29, 2022 02:15
@DeBlister DeBlister closed this May 29, 2022
@DeBlister DeBlister deleted the Xcode_CompileUnitTests_UpdateBuildCI branch May 29, 2022 02:15
@DeBlister DeBlister restored the Xcode_CompileUnitTests_UpdateBuildCI branch May 29, 2022 02:17
@DeBlister DeBlister reopened this May 29, 2022
@DeBlister DeBlister force-pushed the Xcode_CompileUnitTests_UpdateBuildCI branch from ff43418 to 0319f0b Compare May 29, 2022 03:43
DeBlister added 5 commits May 29, 2022 14:09
This isn't 100% necessary but it simplifies other steps (parse and unit).  Also, it does mirror the steps which building actually follows.
Use more specific names for env artifacts.
Use different names for the different uploaded artifacts.
Typo fix on 348. (game => unit tests)
Uploading the entire framework was simple but it took more space for 2 reasons:
It packaged its dependencies with it.  This is the obvious one.
Both the A and Current versions for the engine itself and for its dependencies were copied, so it was something like 4x the size it really needed to be, even if the entire thing was bundled.

There will have to be some more sym-linking and re-downloading the engine's dependencies but that and the storage space will now be in line with the other OS tests.
@DeBlister
Copy link
Owner Author

See this issue on the upload artifact repository describing the problem addressed in the previous commit. Due to the quadrupling effect (((versions + 1) ^ (recursive iterations)) = 2 ^ 2 = 4 in our case) brought on by the lack of sym-link support, I'm going to have to piece the engine framework back together in each of the tests for mac (parse & unit).

DeBlister added 3 commits May 29, 2022 17:06
Attempt to rebuild app file structure from all its pieces.  This is necessary because of the previous commit which only saves the engine binary as an artifact.
@DeBlister DeBlister force-pushed the Xcode_CompileUnitTests_UpdateBuildCI branch from a6b8489 to ded5c89 Compare May 30, 2022 21:07
DeBlister added 15 commits May 30, 2022 17:29
There shouldn't even be a B version so Current would only ever point to A; this way when reconstructing the .app file structure the Current symlink doesn't need to be created.
Test if this is necessary.
Also remove the related tree line because now nothing changes between it and the one prior.
Testing is over, remove clutter.
Hopefully the options will be accepted now.
Now that test_macos-parse is done, restore its if-guard.
Lower the if-guard on test_macos-unit so it can be tested.
The -v option is coded in main.cpp, which the unit tests executable obviously doesn't use.
@DeBlister
Copy link
Owner Author

Everything seems to be working now. I fixed the rpath issue by adding to the flags so that Endless Sky and ES_UnitTests will look inside ES_Engine.framework if they can't find what they need in their own Frameworks folder.

Ideally someone would review for general style and syntax before I merge this back into the main PR branch and get out of this container.

DeBlister added 2 commits May 31, 2022 18:59
If a variable doesn't directly reference a binary or framework (as it usually doesn't in the macos jobs) its name shouldn't imply that it does.

SDL2_FRAMEWORK needs more testing to see if it can be changed like these were.
@DeBlister
Copy link
Owner Author

I'm not going to change the way SDL2_FRAMEWORK is declared because it doubles as a cache destination and a reference to the place in the repository where SDL2 is located normally.

DeBlister added 2 commits May 31, 2022 20:15
Everything needs to go back where it came from before merging.
Match test_macos-unit if-guard to the windows and ubuntu versions.
Update the macos/Xcode path filters used by the changed job.  The workspace probably shouldn't change but if it does tests should probably still run.  Shared data needs to be checked, at least on the project level (this checks both project and workspace) in order to verify schemes.
@DeBlister DeBlister force-pushed the Xcode_CompileUnitTests_UpdateBuildCI branch from cd068c9 to b53dd72 Compare June 1, 2022 00:34
DeBlister added 6 commits May 31, 2022 20:52
This reverts commit 010b71f.

This caused issues with the game target because it has spaces in its name.  The quotes could be used selectively but that would be worse than the quotes themselves.
This reverts commit 36c5706.

This caused issues with the game target because it has spaces in its name. The quotes could be used selectively but that would be worse than the quotes themselves.
@DeBlister DeBlister merged commit cd2c994 into Xcode_CompileUnitTests Jun 1, 2022
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.

1 participant