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

Add missing --osx-sdk and --osx-version-min options #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

neikeq
Copy link
Contributor

@neikeq neikeq commented Jul 12, 2021

  • Add missing --osx-version-min option to the iOS AOT compiler script.
  • Add missing --osx-sdk and --osx-version-min options to the macOS desktop script.
  • Use xcrun to get macosx sdk path in the macOS desktop script. Previously the path was hard-coded. This matches the iOS script.

@neikeq
Copy link
Contributor Author

neikeq commented Jul 12, 2021

WARNING: This is not tested as I don't have a osxcross build environment set up.

- Add missing `--osx-version-min` option to the iOS AOT compiler script.
- Add missing `--osx-sdk` and `--osx-version-min` options to the macOS desktop script.
- Use xcrun to get macosx sdk path in the macOS desktop script. Previously the path was hard-coded. This matches the iOS script.
@@ -253,9 +258,13 @@ def run_main(raw_args, target_platform):

default_help = 'default: %(default)s'

default_osx_version_min = '10.9'
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if we should handle it here or leave it to the user, but for macOS arm64, the min supported version is 10.15 (which is equivalent to 11.0 IIRC), i.e. Big Sur.

For x86_64 we indeed still support down to 10.9 in 3.x, and 10.12 in master (but we can sync that later on in these scripts, for now it's better to track 3.x).

Copy link
Member

Choose a reason for hiding this comment

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

Also I guess to use it I'd have to split my ./osx.py configure --target=x86_64 --target=arm64 and matching make into two, so I can pass different --osx-version-min for x86_64 and arm64?

Copy link
Member

Choose a reason for hiding this comment

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

I'm going to test this PR with this diff to my build scripts:

diff --git a/Dockerfile.ios b/Dockerfile.ios
index 74344e9..2379965 100644
--- a/Dockerfile.ios
+++ b/Dockerfile.ios
@@ -42,13 +42,16 @@ RUN if [ -z "${mono_version}" ]; then echo -e "\n\nargument mono-version is mand
     cd /root/${mono_version} && \
     cd godot-mono-builds && \
     export MONO_SOURCE_ROOT=/root/${mono_version} && \
-    python3 ios.py configure -j --target=arm64 --ios-version-min 10.0 --osx-toolchain ${OSXCROSS_ROOT} \
+    python3 ios.py configure -j --target=arm64 --osx-version-min 10.9 --ios-version-min 10.0 \
+      --osx-toolchain ${OSXCROSS_ROOT} --osx-sdk ${OSXCROSS_ROOT}/target/SDK/MacOSX11.1.sdk \
       --ios-toolchain ${IOSCROSS_ROOT}/arm64 --ios-sdk ${IOSCROSS_ROOT}/arm64/SDK/iPhoneOS${IOS_SDK}.sdk && \
     python3 ios.py make -j --target=arm64 && \
-    #python3 ios.py configure -j --target=arm64-sim --ios-version-min 10.0 --osx-toolchain ${OSXCROSS_ROOT} \
+    #python3 ios.py configure -j --target=arm64-sim --osx-version-min 10.9 --ios-version-min 10.0 \
+    #  --osx-toolchain ${OSXCROSS_ROOT} --osx-sdk ${OSXCROSS_ROOT}/target/SDK/MacOSX11.1.sdk \
     #  --ios-toolchain ${IOSCROSS_ROOT}/arm64_sim --ios-sdk ${IOSCROSS_ROOT}/arm64_sim/SDK/iPhoneOS${IOS_SDK}.sdk && \
     #python3 ios.py make -j --target=arm64-sim && \
-    python3 ios.py configure -j --target=x86_64 --ios-version-min 10.0 --osx-toolchain ${OSXCROSS_ROOT} \
+    python3 ios.py configure -j --target=x86_64 --osx-version-min 10.9 --ios-version-min 10.0 \
+      --osx-toolchain ${OSXCROSS_ROOT} --osx-sdk ${OSXCROSS_ROOT}/target/SDK/MacOSX11.1.sdk \
       --ios-toolchain ${IOSCROSS_ROOT}/x86_64_sim --ios-sdk ${IOSCROSS_ROOT}/x86_64_sim/SDK/iPhoneOS${IOS_SDK}.sdk && \
     python3 ios.py make -j --target=x86_64 && \
     python3 bcl.py make -j --product=ios && \
diff --git a/Dockerfile.osx b/Dockerfile.osx
index 2fb935b..c585bcc 100644
--- a/Dockerfile.osx
+++ b/Dockerfile.osx
@@ -23,8 +23,12 @@ RUN cp -a /root/files/${mono_version} /root && \
     export MONO_SOURCE_ROOT=/root/${mono_version} && \
     export OSXCROSS_SDK=20.2 && \
     cd /root/${mono_version}/godot-mono-builds && \
-    python3 osx.py configure -j --target=x86_64 --target=arm64 && \
-    python3 osx.py make -j --target=x86_64 --target=arm64 && \
+    python3 osx.py configure -j --target=x86_64 \
+      --osx-sdk ${OSXCROSS_ROOT}/target/SDK/MacOSX11.1.sdk --osx-version-min 10.9 && \
+    python3 osx.py make -j --target=x86_64 && \
+    python3 osx.py configure -j --target=arm64 \
+      --osx-sdk ${OSXCROSS_ROOT}/target/SDK/MacOSX11.1.sdk --osx-version-min 10.15 && \
+    python3 osx.py make -j --target=arm64 && \
     python3 bcl.py make --product=desktop && \
     python3 osx.py copy-bcl --target=x86_64 --target=arm64 && \
     cd /root && \

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if we should handle it here or leave it to the user, but for macOS arm64, the min supported version is 10.15 (which is equivalent to 11.0 IIRC), i.e. Big Sur.

For x86_64 we indeed still support down to 10.9 in 3.x, and 10.12 in master (but we can sync that later on in these scripts, for now it's better to track 3.x).

I think the reason I didn't have issues with this is because previously it wasn't specifying -mmacosx-version-min at all. Maybe that's what we should do if the option is not specified? To not pass -mmacosx-version-min instead of passing a default?

P.S.: This shouldn't affect official scripts as those would pass a custom version, right?

Also I guess to use it I'd have to split my ./osx.py configure --target=x86_64 --target=arm64 and matching make into two, so I can pass different --osx-version-min for x86_64 and arm64?

Yes. If the min versions are different for the two targets then these should go into separate commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The diff looks good, but shouldn't the min versions be the same as the ones Godot specifies?

Copy link
Member

Choose a reason for hiding this comment

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

I think the reason I didn't have issues with this is because previously it wasn't specifying -mmacosx-version-min at all. Maybe that's what we should do if the option is not specified? To not pass -mmacosx-version-min instead of passing a default?

Yeah I'm not sure yet to what extent we actually need to specify this. Back when investigating godotengine/build-containers#83 I ended up trying this out, but the actual problem there was solved by building compiler_rt so I don't currently have an actual issue that needs to be handled - just this inconsistency which made me wonder if we need to specify it.

The Mono buildsystem passes it here: https://github.com/mono/mono/blob/bdd772531d379b4e78593587d15113c37edd4a64/sdks/builds/mac.mk#L131-L134
With default value 10.9: https://github.com/mono/mono/blob/bdd772531d379b4e78593587d15113c37edd4a64/sdks/versions.mk#L27

They don't seem to care to set 11.0 for their macarm64 define.

The diff looks good, but shouldn't the min versions be the same as the ones Godot specifies?

They should be normally:

  • macOS: 10.9 for x86_64 and 10.15 for arm64 in 3.x.
  • iOS: 10.0 in 3.x.

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.

2 participants