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

[RN][iOS] Add matrix for Debug/Release, New/Legacy Architecture, (No)Hermes #34469

Closed
wants to merge 1 commit into from
Closed

[RN][iOS] Add matrix for Debug/Release, New/Legacy Architecture, (No)Hermes #34469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 47 additions & 8 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,16 @@ jobs:
# -------------------------
test_ios_template:
executor: reactnativeios
parameters:
flavor:
type: string
default: "Debug"
architecture:
type: string
default: "OldArch"
jsengine:
type: string
default: "Hermes"
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to add a flipper or not (since I think flipper in release mode had a problem recently, and release mode issues are typically not discovered until later in the RC cycle after the first few build checks)

And of course my favorite we add a use_frameworks check (with possibly both linkages - so it's actually a tri-state, no frameworks, frameworks+dynamic, frameworks+static)

These are the variations I'm currently aware of

I expect failure in hermes+frameworks+dynamic, and flipper+frameworks+any and newarch+frameworks+any, but on the good side, the other permutations are all working I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed offline (but leaving the reply here for future doc), yes.

I plan to add the Flipper Yes/No, excluding all the WithFlipper+Release because flipper is not added by default in Release.

For use framework, yes... it is possible, but requires some more changes also in the podfiles...

I'll probably add them in a separate PR, though. Just not to add too many things at once.

Copy link
Contributor

Choose a reason for hiding this comment

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

Totally agree with incremental improvement in all things, and specifically here. But as a brainstorm for a future PR: I also just remembered macCatalyst builds. That was an odyssey getting them working again after they broke in 0.63, would be nice to have a canary here that would tell us if they broke again.

environment:
- PROJECT_NAME: "iOSTemplateProject"
- HERMES_WS_DIR: *hermes_workspace_root
Expand All @@ -689,25 +699,49 @@ jobs:
- attach_workspace:
at: .
- *attach_hermes_workspace
- run:
name: Set USE_HERMES=1
command: echo "export USE_HERMES=1" >> $BASH_ENV
- run:
name: Set HERMES_ENGINE_TARBALL_PATH
command: |
echo "export HERMES_ENGINE_TARBALL_PATH=$(ls -AU $HERMES_WS_DIR/hermes-runtime-darwin/hermes-runtime-darwin-*.tar.gz | head -1)" >> $BASH_ENV
- when:
condition:
equal: ["Hermes", << parameters.jsengine >>]
steps:
- run:
name: Set HERMES_ENGINE_TARBALL_PATH
command: |
echo "export HERMES_ENGINE_TARBALL_PATH=$(ls -AU $HERMES_WS_DIR/hermes-runtime-darwin/hermes-runtime-darwin-*.tar.gz | head -1)" >> $BASH_ENV
- run:
name: Create iOS template project
command: |
REPO_ROOT=$(pwd)
PACKAGE=$(cat build/react-native-package-version)
PATH_TO_PACKAGE="$REPO_ROOT/build/$PACKAGE"
node ./scripts/set-rn-template-version.js "file:$PATH_TO_PACKAGE"
node cli.js init $PROJECT_NAME --directory "/tmp/$PROJECT_NAME" --template $REPO_ROOT --verbose
node cli.js init $PROJECT_NAME --directory "/tmp/$PROJECT_NAME" --template $REPO_ROOT --verbose --skip-install
- run:
name: Install iOS dependencies - Configuration << parameters.flavor >>; New Architecture << parameters.architecture >>; JS Engine << parameters.jsengine>>
command: |
cd /tmp/$PROJECT_NAME
yarn install
cd ios

bundle install

if [[ << parameters.flavor >> == "Release" ]]; then
export PRODUCTION=1
fi

if [[ << parameters.architecture >> == "NewArch" ]]; then
export RCT_NEW_ARCH_ENABLED=1
fi

if [[ << parameters.jsengine >> == "JSC" ]]; then
export USE_HERMES=0
fi

bundle exec pod install
- run:
name: Build template project
command: |
xcodebuild build \
-configuration << parameters.flavor >> \
-workspace /tmp/$PROJECT_NAME/ios/$PROJECT_NAME.xcworkspace \
-scheme $PROJECT_NAME \
-sdk iphonesimulator
Expand Down Expand Up @@ -1308,6 +1342,11 @@ workflows:
- test_ios_template:
requires:
- build_npm_package
matrix:
parameters:
architecture: ["NewArch", "OldArch"]
flavor: ["Debug", "Release"]
jsengine: ["Hermes", "JSC"]
- test_ios_rntester:
requires:
- build_hermes_macos
Expand Down
13 changes: 12 additions & 1 deletion scripts/react-native-xcode.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,24 @@ fi
# shellcheck source=/dev/null
source "$REACT_NATIVE_DIR/scripts/node-binary.sh"

# If hermes-engine is in the podfile.lock, it means that Hermes is a dependency of the project
# and it is enabled. If not, it means that hermes is disabled.
HERMES_ENABLED=$(grep hermes-engine podfile.lock)

# If hermes-engine is not in the podfile.lock, it means that the app is not using Hermes.
# Setting USE_HERMES is no the only way to set whether the app can use hermes or not: users
# can also modify manually the Podfile.
if [[ -z "$HERMES_ENABLED" ]]; then
USE_HERMES=false
fi

HERMES_ENGINE_PATH="$PODS_ROOT/hermes-engine"
[ -z "$HERMES_CLI_PATH" ] && HERMES_CLI_PATH="$HERMES_ENGINE_PATH/destroot/bin/hermesc"

# Hermes is enabled in new projects by default, so we cannot assume that USE_HERMES=1 is set as an envvar.
# If hermes-engine is found in Pods, we can assume Hermes has not been disabled.
# If hermesc is not available and USE_HERMES is either unset or true, show error.
if [[ -f "$HERMES_ENGINE_PATH" && ! -f "$HERMES_CLI_PATH" ]]; then
if [[ ! -z "$HERMES_ENABLED" && -f "$HERMES_ENGINE_PATH" && ! -f "$HERMES_CLI_PATH" ]]; then
echo "error: Hermes is enabled but the hermesc binary could not be found at ${HERMES_CLI_PATH}." \
"Perhaps you need to run 'bundle exec pod install' or otherwise " \
"point the HERMES_CLI_PATH variable to your custom location." >&2
Expand Down
2 changes: 1 addition & 1 deletion scripts/react_native_pods.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def use_react_native! (
fabric_enabled: false,
new_arch_enabled: ENV['RCT_NEW_ARCH_ENABLED'] == '1',
production: ENV['PRODUCTION'] == '1',
hermes_enabled: true,
hermes_enabled: ENV['USE_HERMES'] && ENV['USE_HERMES'] == '0' ? false : true,
flipper_configuration: FlipperConfiguration.disabled,
app_path: '..',
config_file_dir: '')
Expand Down
2 changes: 1 addition & 1 deletion template/ios/Podfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ target 'HelloWorld' do
# Hermes is now enabled by default. Disable by setting this flag to false.
# Upcoming versions of React Native may rely on get_default_flags(), but
# we make it explicit here to aid in the React Native upgrade process.
:hermes_enabled => true,
:hermes_enabled => flags[:hermes_enabled],
:fabric_enabled => flags[:fabric_enabled],
# Enables Flipper.
#
Expand Down