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.

Sign up for GitHub

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

Jump to bottom

arm64 simulators & catalyst support #475

Closed
wants to merge 4 commits into from
Closed

arm64 simulators & catalyst support #475

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
88e6566
arm64 simulators & catalyst support
Arkkeeper Mar 20, 2021
b63d240
.podspec macOS version downgrade
Arkkeeper Mar 22, 2021
cee5fcf
create_universal_framework improved
Arkkeeper May 2, 2021
bdd75a4
build-apple-framework.sh reversion
Arkkeeper Jun 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ endif()
# https://cmake.org/cmake/help/latest/variable/CMAKE_OSX_SYSROOT.html
set(CMAKE_OSX_SYSROOT ${HERMES_APPLE_TARGET_PLATFORM})

if(HERMES_APPLE_CATALYST)
set(CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS} "-target x86_64-arm64-apple-ios14.0-macabi -isystem ${CMAKE_OSX_SYSROOT}/System/iOSSupport/usr/include")
set(CMAKE_C_FLAGS ${CMAKE_C_FLAGS} "-target x86_64-arm64-apple-ios14.0-macabi -isystem ${CMAKE_OSX_SYSROOT}/System/iOSSupport/usr/include")
set(CMAKE_THREAD_LIBS_INIT "-lpthread")
set(CMAKE_HAVE_THREADS_LIBRARY 1)
set(CMAKE_USE_WIN32_THREADS_INIT 0)
set(CMAKE_USE_PTHREADS_INIT 1)
set(THREADS_PREFER_PTHREAD_FLAG ON)
endif()

# This must be consistent with the release_version in:
# - android/build.gradle
# - npm/package.json
Expand Down
2 changes: 1 addition & 1 deletion hermes-engine.podspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Pod::Spec.new do |spec|
spec.source_files = "destroot/include/**/*.h"
spec.header_mappings_dir = "destroot/include"

spec.ios.vendored_frameworks = "destroot/Library/Frameworks/iphoneos/hermes.framework"
spec.ios.vendored_frameworks = "destroot/Library/Frameworks/iphoneos/hermes.xcframework"
spec.osx.vendored_frameworks = "destroot/Library/Frameworks/macosx/hermes.framework"

spec.xcconfig = { "CLANG_CXX_LANGUAGE_STANDARD" => "c++14", "CLANG_CXX_LIBRARY" => "compiler-default", "GCC_PREPROCESSOR_DEFINITIONS" => "HERMES_ENABLE_DEBUGGER=1" }
Expand Down
36 changes: 17 additions & 19 deletions utils/build-apple-framework.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ function build_host_hermesc {
# Utility function to configure an Apple framework
function configure_apple_framework {
local build_cli_tools enable_bitcode
if [[ $1 == iphoneos ]]; then
local catalyst="false"
local platform=$1

if [[ $1 == iphoneos || $1 == catalyst ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps also replace further use of $1 with the $platform alias here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would actually not use $platform here. Further explanation on line 62.

enable_bitcode="true"
else
enable_bitcode="false"
Expand All @@ -56,11 +59,16 @@ function configure_apple_framework {
else
build_cli_tools="false"
fi
if [[ $1 == catalyst ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

This comment was marked as resolved.

Sign in to view

catalyst="true"
platform="macosx"
fi

local cmake_flags=" \
-DHERMES_APPLE_TARGET_PLATFORM:STRING=$1 \
-DHERMES_APPLE_TARGET_PLATFORM:STRING=$platform \
-DCMAKE_OSX_ARCHITECTURES:STRING=$2 \
-DCMAKE_OSX_DEPLOYMENT_TARGET:STRING=$3 \
-DHERMES_APPLE_CATALYST:BOOLEAN=$catalyst \
-DHERMES_ENABLE_DEBUGGER:BOOLEAN=true \
-DHERMES_ENABLE_LIBFUZZER:BOOLEAN=false \
-DHERMES_ENABLE_FUZZILLI:BOOLEAN=false \
Expand Down Expand Up @@ -92,28 +100,18 @@ function build_apple_framework {
fi
}

# Accepts an array of frameworks and will place all of
# the architectures into the first one in the list
# Merges compiled frameworks and combines all of
# the architectures into the single xcframework
function create_universal_framework {
cd ./destroot/Library/Frameworks || exit 1

local platforms=("$@")

echo "Creating universal framework for platforms: ${platforms[*]}"

for i in "${!platforms[@]}"; do
platforms[$i]="${platforms[$i]}/hermes.framework/hermes"
done

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view


lipo -create -output "${platforms[0]}" "${platforms[@]}"
echo "Creating universal framework for Apple Platforms"

# Once all was linked into a single framework, clean destroot
# from unused frameworks
for platform in "${@:2}"; do
rm -r "$platform"
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason why this has been removed? This should be reverted. We need to clean up destroot to only contain the artifacts that are used in production.

This is because CircleCI will package the entire destroot folder into a tarball that is distributed along the npm package. If we don't clean it up, we are going to ship one xcframework and every platform separately as well.

Copy link
Author

Choose a reason for hiding this comment

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

OK, I've reverted that removal

done
xcodebuild -create-xcframework -framework iphoneos/hermes.framework -framework iphonesimulator/hermes.framework -framework macosx/hermes.framework -output iphoneos/hermes.xcframework
Copy link
Contributor

Choose a reason for hiding this comment

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

The output should not assume the framework is just for iOS here, it should theoretically also be able to contain the macOS slices, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Also, it should also get the platforms from the platforms, like the lipo did, without hardcoding them. I just think it's nicer and doesn't require manual rm of every folder, like it's currently done on line 112-114.


lipo -info "${platforms[0]}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason why this has been removed? I think this is a good add-on for debugging purposes on the CI.

Copy link
Author

Choose a reason for hiding this comment

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

Because lipo -info fails on xcframework with a fatal error:
can't map input file: iphoneos (Invalid argument)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

rm -r iphonesimulator
rm -r macosx
rm -r iphoneos/hermes.framework

cd - || exit 1
}
5 changes: 3 additions & 2 deletions utils/build-ios-framework.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ if [ ! -d destroot/Library/Frameworks/iphoneos/hermes.framework ]; then
ios_deployment_target=$(get_ios_deployment_target)

build_apple_framework "iphoneos" "armv7;armv7s;arm64" "$ios_deployment_target"
build_apple_framework "iphonesimulator" "x86_64;i386" "$ios_deployment_target"
build_apple_framework "iphonesimulator" "x86_64;arm64" "$ios_deployment_target"
build_apple_framework "catalyst" "x86_64;arm64" "$ios_deployment_target"

create_universal_framework "iphoneos" "iphonesimulator"
create_universal_framework
else
echo "Skipping; Clean \"destroot\" to rebuild".
fi