add support for macos aarch64 #12319
Conversation
| @@ -46,6 +46,11 @@ else | |||
| -DCMAKE_CXX_STANDARD=14 -Dprotobuf_BUILD_TESTS=OFF -DBUILD_SHARED_LIBS=OFF \ | |||
| -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DABSL_INTERNAL_AT_LEAST_CXX17=0 \ | |||
| -B. || exit 1 | |||
| elif [[ "$ARCH" == "aarch_64" && "$(uname -s)" == "Darwin" ]]; then | |||
| cmake .. \ | |||
| -DCMAKE_CXX_STANDARD=14 -Dprotobuf_BUILD_TESTS=OFF -DBUILD_SHARED_LIBS=OFF \ | |||
There was a problem hiding this comment.
Eric's comment said we need to pass both arm64 and x86_64 to get the universal binary. Are you getting the x86 binary?
There was a problem hiding this comment.
I'm avoiding the cross-compilation here that Eric mentioned. Each binary is built natively on its target hardware, which is simpler. This is the alternate way to support in arm64.
The existing Kokoro job (macos.cfg) will continue to run on an x86_64 machine and produce a pure osx-x86_64 binary. The new job I've added (macos_aarch64.cfg) will run in parallel on a native aarch64 (Apple Silicon) machine and produce a pure osx-aarch_64 binary.
There was a problem hiding this comment.
Also, by keeping the builds separate, we de-risk the change and ensure the existing x86_64 build process remains completely untouched and stable.
There was a problem hiding this comment.
The problem you have to consider with this approach is if we build on Sequoia, then the binary is only guaranteed to work on Sequoia and newer OS X versions. So we need to build on the oldest version of OS X that is supported.
Right now we're using Ventura and our CI doesn't have an ARM Ventura. However, we can wait some weeks, and Apple tends to drop OS X support in September/October/November, at which point we'd support Sonoma. We do have Sonoma available for ARM and x86. But after that with Sequoia we're going to have the opposite problem, of still wanting to support x86 but only having an ARM CI. That leaves us in a bind with Sequoia and Tahoe, which both still support x86 hardware (Tahoe will be the last to support x86).
There was a problem hiding this comment.
Alright, to mitigate these problems I tried with the original idea that Eric proposed—having universal binaries. Take a look!
buildscripts/kokoro/macos_aarch64.sh
Outdated
| export -n JAVA_HOME | ||
| export PATH="$(/usr/libexec/java_home -v"1.8.0")/bin:${PATH}" | ||
|
|
||
| export ARCH=aarch_64 |
There was a problem hiding this comment.
This makes me uneasy. We're having two builds but assuming the machine they are running on, and it isn't obvious from the pool names which machine type is which. We can do that, but it is probably clearer/better to detect the machine type and then choose the ARCH to match?
| @@ -46,6 +46,11 @@ else | |||
| -DCMAKE_CXX_STANDARD=14 -Dprotobuf_BUILD_TESTS=OFF -DBUILD_SHARED_LIBS=OFF \ | |||
| -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DABSL_INTERNAL_AT_LEAST_CXX17=0 \ | |||
| -B. || exit 1 | |||
| elif [[ "$ARCH" == "aarch_64" && "$(uname -s)" == "Darwin" ]]; then | |||
| cmake .. \ | |||
| -DCMAKE_CXX_STANDARD=14 -Dprotobuf_BUILD_TESTS=OFF -DBUILD_SHARED_LIBS=OFF \ | |||
There was a problem hiding this comment.
The problem you have to consider with this approach is if we build on Sequoia, then the binary is only guaranteed to work on Sequoia and newer OS X versions. So we need to build on the oldest version of OS X that is supported.
Right now we're using Ventura and our CI doesn't have an ARM Ventura. However, we can wait some weeks, and Apple tends to drop OS X support in September/October/November, at which point we'd support Sonoma. We do have Sonoma available for ARM and x86. But after that with Sequoia we're going to have the opposite problem, of still wanting to support x86 but only having an ARM CI. That leaves us in a bind with Sequoia and Tahoe, which both still support x86 hardware (Tahoe will be the last to support x86).
buildscripts/make_dependencies.sh
Outdated
| @@ -41,7 +41,12 @@ else | |||
| mkdir "$DOWNLOAD_DIR/protobuf-${PROTOBUF_VERSION}/build" | |||
| pushd "$DOWNLOAD_DIR/protobuf-${PROTOBUF_VERSION}/build" | |||
| # install here so we don't need sudo | |||
| if [[ "$ARCH" == x86* ]]; then | |||
| if [[ "$(uname -s)" == "Darwin" ]]; then | |||
| cmake .. \ | |||
There was a problem hiding this comment.
Unindent this line two spaces. It shouldn't be at the same level as the next line as that's a line continuation.
| @@ -42,8 +42,6 @@ LOCAL_OTHER_ARTIFACTS="$KOKORO_GFILE_DIR"/github/grpc-java/artifacts/ | |||
|
|
|||
| # from macos job: | |||
| [[ "$(find "$LOCAL_MVN_ARTIFACTS" -type f -iname 'protoc-gen-grpc-java-*-osx-x86_64.exe' | wc -l)" != '0' ]] | |||
| # copy all x86 artifacts to aarch until native artifacts are built | |||
| find "$LOCAL_MVN_ARTIFACTS" -type f -iname 'protoc-gen-grpc-java-*-osx-x86_64.exe*' -exec bash -c 'cp "${0}" "${0/x86/aarch}"' {} \; | |||
There was a problem hiding this comment.
Now that we're building a single universal binary for macOS, isn't this copy obsolete?
buildscripts/kokoro/unix.sh
Outdated
| GRADLE_FLAGS+=" -PbuildUniversal=true" | ||
| else | ||
| # For Linux, build for a single, specific architecture. | ||
| GRADLE_FLAGS+=" -PtargetArch=$ARCH" |
There was a problem hiding this comment.
Note that targetArch has a default in the build.gradle when it isn't specified. So avoiding this line for Mac doesn't do much.
compiler/build.gradle
Outdated
| @@ -103,6 +103,10 @@ model { | |||
| cppCompiler.args "--std=c++14" | |||
| addEnvArgs("CXXFLAGS", cppCompiler.args) | |||
| addEnvArgs("CPPFLAGS", cppCompiler.args) | |||
| if (project.hasProperty('buildUniversal') && osdetector.os == "osx") { | |||
There was a problem hiding this comment.
-PbuildUniversal=false will be true here.
Fixes #7690
VERIFIED: