-
-
Notifications
You must be signed in to change notification settings - Fork 76
Apple: add visionOS and tvOS build support; unify build container #154
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
Conversation
|
This looks really great. I've rebased this locally and I'm doing some testing and improvements to the build scripts. I think we can go all in and also do the iOS builds using the new container, and remove the bespoke iOS one. |
98b6b98 to
38272cb
Compare
Remove `ios` image, superseded by `appleembedded`.
akien-mga
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested successfully together with the build scripts in github.com/godotengine/godot-build-scripts/pull/115 (well, my upcoming rebase there to include my changes).
| # Extract the SDKs for iOS, tvOS and XROS, including the simulator SDKs | ||
| # into separate layers, so this work is not repeated each time the image is built. | ||
| RUN mkdir -p /root/SDKs | ||
| RUN cd /root/SDKs && tar xf /root/files/iPhoneOS${IOS_SDK}.sdk.tar.xz | ||
| RUN cd /root/SDKs && tar xf /root/files/iPhoneSimulator${IOS_SDK}.sdk.tar.xz | ||
| RUN cd /root/SDKs && tar xf /root/files/AppleTVOS${TVOS_SDK}.sdk.tar.xz | ||
| RUN cd /root/SDKs && tar xf /root/files/AppleTVSimulator${TVOS_SDK}.sdk.tar.xz | ||
| RUN cd /root/SDKs && tar xf /root/files/XROS${XROS_SDK}.sdk.tar.xz | ||
| RUN cd /root/SDKs && tar xf /root/files/XRSimulator${XROS_SDK}.sdk.tar.xz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the impact on disk usage to have all this in individual layers?
I usually try to keep things that are always going to be modified together in the same RUN to avoid having a ton of intermediate layers. Here for example all SDKs are tied to an Xcode version so we would likely never run this with only some of the SDKs requiring to be extracted, it's all or nothing.
| ENV OSXCROSS_IOS=not_nothing | ||
| ENV OSXCROSS_TVOS=not_nothing | ||
| ENV OSXCROSS_VISIONOS=not_nothing | ||
| ENV OSXCROSS_APPLEEMBEDDED=not_nothing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the XROS one which was unused (SCons checks OSXCROSS_VISIONOS).
OSXCROSS_APPLEEMBEDDED is also unused but I kept it for now in case we want to unify things upstream behind OSXCROSS_APPLEEMBEDDED.
It's a bit of a weird system though where we set an environment variable to whatever just to say we want to support osxcross. Maybe we should change it upstream to instead use osxcross=yes on the command line. But it's not super important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree - as that would make it more obvious when cross-compiling with scons. A future update?
| COPY extract_xcode_sdks.sh /root/extract_xcode_sdks.sh | ||
| RUN chmod +x /root/extract_xcode_sdks.sh | ||
|
|
||
| CMD /root/extract_xcode_sdks.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not against it, but was there a specific reason you had to move this to a new script? Was the previous logic not doing a clear enough extraction of the SDKs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was easier to debug and test the individual commands as a separate script, so I figured I would just reuse it for the container.
|
Thanks! |
godot-appleembedded:<image version>for building iOS, visionOS and tvOS