Skip to content

Commit 12fccde

Browse files
janicduplessisfacebook-github-bot
janicduplessis
authored and
facebook-github-bot
committed
Make codegen more reliable on iOS (#30792)
Summary: This addesses a few issues I noticed while migrating my app to the new build-time codegen on iOS. 1. I noticed random failures because of codegen on iOS. This is mostly due to the fact the codegen output files are not specified in the xcode script. The only reason it works relatively fine currently is because the codegen output is inside the input files directory. This has the side effect of causing files to be regenerated every build, then causes all core modules to be recompiled which adds up a significant amount of time to rebuilds. To fix this I added the generated files to the script phase output and moved the FBReactNativeSpec dir outside of the codegen source (Libraries). I moved it to the React directory as this seemed to make sense and is where a lot of iOS files are as well as the core modules. Note this might require internal changes. This removes the circular dependency between our build phase input and output so consecutive builds can be cached properly. 2. Add `set -o pipefail` to the xcode script, this helped propagate errors properly to xcode because of the `| tee` pipe so it fails at the script phase and not later with a header not found error. Also add `2>&1` to pipe stderr to stdout so errors are also captured in the log file. 3. Add the `-l` flag to the bash invocation to help finding the yarn binary. With my setup yarn is added to the system PATH in my user .profile. Adding this file will cause bash to source the user environment which xcode scripts does not by default. I think this will help with most setups. 4. If yarn is not found the `command -v yarn` would make the script exit without any output because of the -e flag. I made a change to ignore the return code and check later if YARN_BINARY is set and have an explicit error message if not. ## Changelog [iOS] [Fixed] - Make codegen more reliable on iOS Pull Request resolved: #30792 Test Plan: Tested various project states to make sure the build always succeeds in RN tester: - Simulate fresh clone, remove all ignored files, install pods, build - Build, delete FBReactNativeSpec generated files, build again - Build, build again, make sure FBReactNativeSpec is cached and not rebuilt - Make the script fail and check that xcode shows the script error logs properly ![image](https://user-images.githubusercontent.com/2677334/105891571-c8badd00-5fde-11eb-839c-259d8e448523.png) Note: Did not test fabric Reviewed By: fkgozali Differential Revision: D26104213 Pulled By: hramos fbshipit-source-id: e18d9a0b9ada7c0c2e608d29ffe88087f04605b4
1 parent 6eea597 commit 12fccde

File tree

6 files changed

+47
-38
lines changed

6 files changed

+47
-38
lines changed

‎.gitignore

+2-2
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,9 @@ package-lock.json
8585

8686
# Libs that shouldn't have Xcode project
8787
/Libraries/FBLazyVector/**/*.xcodeproj
88-
/Libraries/FBReactNativeSpec/**/*.xcodeproj
8988
/Libraries/RCTRequired/**/*.xcodeproj
9089
/React/CoreModules/**/*.xcodeproj
90+
/React/FBReactNativeSpec/**/*.xcodeproj
9191
/packages/react-native-codegen/**/*.xcodeproj
9292

9393
# CocoaPods
@@ -100,7 +100,7 @@ package-lock.json
100100
!/packages/rn-tester/Pods/__offline_mirrors__
101101

102102
# react-native-codegen
103-
/Libraries/FBReactNativeSpec/FBReactNativeSpec
103+
/React/FBReactNativeSpec/FBReactNativeSpec
104104
/packages/react-native-codegen/lib
105105
/ReactCommon/react/renderer/components/rncore/
106106

‎React-Core.podspec

+1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ Pod::Spec.new do |s|
5757
ss.exclude_files = "React/CoreModules/**/*",
5858
"React/DevSupport/**/*",
5959
"React/Fabric/**/*",
60+
"React/FBReactNativeSpec/**/*",
6061
"React/Tests/**/*",
6162
"React/Inspector/**/*"
6263
ss.private_header_files = "React/Cxx*/*.h"

‎Libraries/FBReactNativeSpec/FBReactNativeSpec.podspec ‎React/FBReactNativeSpec/FBReactNativeSpec.podspec

+1-2
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,12 @@ Pod::Spec.new do |s|
3131
s.compiler_flags = folly_compiler_flags + ' -Wno-nullability-completeness'
3232
s.source = source
3333
s.source_files = "**/*.{c,h,m,mm,cpp}"
34-
s.exclude_files = "jni"
3534
s.header_dir = "FBReactNativeSpec"
3635

3736
s.pod_target_xcconfig = {
3837
"USE_HEADERMAP" => "YES",
3938
"CLANG_CXX_LANGUAGE_STANDARD" => "c++14",
40-
"HEADER_SEARCH_PATHS" => "\"$(PODS_TARGET_SRCROOT)/Libraries/FBReactNativeSpec\" \"$(PODS_ROOT)/RCT-Folly\""
39+
"HEADER_SEARCH_PATHS" => "\"$(PODS_TARGET_SRCROOT)/React/FBReactNativeSpec\" \"$(PODS_ROOT)/RCT-Folly\""
4140
}
4241

4342
s.dependency "RCT-Folly", folly_version

‎packages/rn-tester/Podfile.lock

+7-7
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
PODS:
22
- boost-for-react-native (1.63.0)
3-
- CocoaAsyncSocket (7.6.4)
3+
- CocoaAsyncSocket (7.6.5)
44
- CocoaLibEvent (1.0.0)
55
- DoubleConversion (1.1.6)
66
- FBLazyVector (1000.0.0)
@@ -653,7 +653,7 @@ PODS:
653653
DEPENDENCIES:
654654
- DoubleConversion (from `../../third-party-podspecs/DoubleConversion.podspec`)
655655
- FBLazyVector (from `../../Libraries/FBLazyVector`)
656-
- FBReactNativeSpec (from `../../Libraries/FBReactNativeSpec`)
656+
- FBReactNativeSpec (from `../../React/FBReactNativeSpec`)
657657
- Flipper (~> 0.54.0)
658658
- Flipper-DoubleConversion (= 1.1.7)
659659
- Flipper-Folly (~> 2.2)
@@ -730,7 +730,7 @@ EXTERNAL SOURCES:
730730
FBLazyVector:
731731
:path: "../../Libraries/FBLazyVector"
732732
FBReactNativeSpec:
733-
:path: "../../Libraries/FBReactNativeSpec"
733+
:path: "../../React/FBReactNativeSpec"
734734
glog:
735735
:podspec: "../../third-party-podspecs/glog.podspec"
736736
RCT-Folly:
@@ -794,11 +794,11 @@ EXTERNAL SOURCES:
794794

795795
SPEC CHECKSUMS:
796796
boost-for-react-native: 39c7adb57c4e60d6c5479dd8623128eb5b3f0f2c
797-
CocoaAsyncSocket: 694058e7c0ed05a9e217d1b3c7ded962f4180845
797+
CocoaAsyncSocket: 065fd1e645c7abab64f7a6a2007a48038fdc6a99
798798
CocoaLibEvent: 2fab71b8bd46dd33ddb959f7928ec5909f838e3f
799799
DoubleConversion: cde416483dac037923206447da6e1454df403714
800800
FBLazyVector: 91e874a8823933a268c38765a88cbd5dba1fa024
801-
FBReactNativeSpec: 17a863c5e24969051850a3acab3a06069bb06e7f
801+
FBReactNativeSpec: 58a907f57c40ca74a954abe86862baa5eb423c63
802802
Flipper: be611d4b742d8c87fbae2ca5f44603a02539e365
803803
Flipper-DoubleConversion: 38631e41ef4f9b12861c67d17cb5518d06badc41
804804
Flipper-Folly: e4493b013c02d9347d5e0cb4d128680239f6c78a
@@ -808,12 +808,12 @@ SPEC CHECKSUMS:
808808
FlipperKit: ab353d41aea8aae2ea6daaf813e67496642f3d7d
809809
glog: 40a13f7840415b9a77023fbcae0f1e6f43192af3
810810
OpenSSL-Universal: ff34003318d5e1163e9529b08470708e389ffcdd
811-
RCT-Folly: b39288cedafe50da43317ec7d91bcc8cc0abbf33
811+
RCT-Folly: ec7a233ccc97cc556cf7237f0db1ff65b986f27c
812812
RCTRequired: d3d4ce60e1e2282864d7560340690a3c8c646de1
813813
RCTTypeSafety: 4da4f9f218727257c50fd3bf2683a06cdb4fede3
814814
React: 63b1f2a4e0e908c95416fd54e9dcca5d409e2a45
815815
React-callinvoker: e9524d75cf0b7ae108868f8d34c0b8d7dc08ec03
816-
React-Core: 2b2a8ac8bfb65779965456570a871f4cf5e5e03a
816+
React-Core: c63389ffebc383f834a2cae4d1c120968ffb3cdb
817817
React-CoreModules: 87f011fa87190ffe979e443ce578ec93ec6ff4d4
818818
React-cxxreact: 14cce64344ab482615dfe82a2cbea6eb73be6481
819819
React-Fabric: 1744b2e94f5ed2ab247f3a55fd9762d55ed63f3b

‎scripts/generate-specs.sh

+18-10
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
# Optionally, set these envvars to override defaults:
1212
# - SRCS_DIR: Path to JavaScript sources
1313
# - CODEGEN_MODULES_LIBRARY_NAME: Defaults to FBReactNativeSpec
14-
# - CODEGEN_MODULES_OUTPUT_DIR: Defaults to Libraries/$CODEGEN_MODULES_LIBRARY_NAME/$CODEGEN_MODULES_LIBRARY_NAME
14+
# - CODEGEN_MODULES_OUTPUT_DIR: Defaults to React/$CODEGEN_MODULES_LIBRARY_NAME/$CODEGEN_MODULES_LIBRARY_NAME
1515
# - CODEGEN_COMPONENTS_LIBRARY_NAME: Defaults to rncore
1616
# - CODEGEN_COMPONENTS_OUTPUT_DIR: Defaults to ReactCommon/react/renderer/components/$CODEGEN_COMPONENTS_LIBRARY_NAME
1717
#
@@ -27,7 +27,7 @@ set -e
2727
THIS_DIR=$(cd -P "$(dirname "$(readlink "${BASH_SOURCE[0]}" || echo "${BASH_SOURCE[0]}")")" && pwd)
2828
TEMP_DIR=$(mktemp -d /tmp/react-native-codegen-XXXXXXXX)
2929
RN_DIR=$(cd "$THIS_DIR/.." && pwd)
30-
YARN_BINARY="${YARN_BINARY:-$(command -v yarn)}"
30+
YARN_BINARY="${YARN_BINARY:-$(command -v yarn || true)}"
3131
USE_FABRIC="${USE_FABRIC:-0}"
3232

3333
cleanup () {
@@ -45,7 +45,7 @@ main() {
4545
CODEGEN_MODULES_LIBRARY_NAME=${CODEGEN_MODULES_LIBRARY_NAME:-FBReactNativeSpec}
4646

4747
CODEGEN_COMPONENTS_LIBRARY_NAME=${CODEGEN_COMPONENTS_LIBRARY_NAME:-rncore}
48-
CODEGEN_MODULES_OUTPUT_DIR=${CODEGEN_MODULES_OUTPUT_DIR:-"$RN_DIR/Libraries/$CODEGEN_MODULES_LIBRARY_NAME/$CODEGEN_MODULES_LIBRARY_NAME"}
48+
CODEGEN_MODULES_OUTPUT_DIR=${CODEGEN_MODULES_OUTPUT_DIR:-"$RN_DIR/React/$CODEGEN_MODULES_LIBRARY_NAME/$CODEGEN_MODULES_LIBRARY_NAME"}
4949
# TODO: $CODEGEN_COMPONENTS_PATH should be programmatically specified, and may change with use_frameworks! support.
5050
CODEGEN_COMPONENTS_PATH="ReactCommon/react/renderer/components"
5151
CODEGEN_COMPONENTS_OUTPUT_DIR=${CODEGEN_COMPONENTS_OUTPUT_DIR:-"$RN_DIR/$CODEGEN_COMPONENTS_PATH/$CODEGEN_COMPONENTS_LIBRARY_NAME"}
@@ -56,6 +56,11 @@ main() {
5656
CODEGEN_REPO_PATH="$RN_DIR/packages/react-native-codegen"
5757
CODEGEN_NPM_PATH="$RN_DIR/../react-native-codegen"
5858

59+
if [ -z "$YARN_BINARY" ]; then
60+
echo "Error: Could not find yarn. Make sure it is in bash PATH or set the YARN_BINARY environment variable." 1>&2
61+
exit 1
62+
fi
63+
5964
if [ -d "$CODEGEN_REPO_PATH" ]; then
6065
CODEGEN_PATH=$(cd "$CODEGEN_REPO_PATH" && pwd)
6166
elif [ -d "$CODEGEN_NPM_PATH" ]; then
@@ -67,24 +72,27 @@ main() {
6772

6873
if [ ! -d "$CODEGEN_PATH/lib" ]; then
6974
describe "Building react-native-codegen package"
70-
pushd "$CODEGEN_PATH" >/dev/null || exit
75+
pushd "$CODEGEN_PATH" >/dev/null || exit 1
7176
"$YARN_BINARY"
7277
"$YARN_BINARY" build
73-
popd >/dev/null || exit
78+
popd >/dev/null || exit 1
7479
fi
7580

7681
describe "Generating schema from flow types"
7782
"$YARN_BINARY" node "$CODEGEN_PATH/lib/cli/combine/combine-js-to-schema-cli.js" "$SCHEMA_FILE" "$SRCS_DIR"
7883

7984
describe "Generating native code from schema (iOS)"
80-
pushd "$RN_DIR" >/dev/null || exit
85+
pushd "$RN_DIR" >/dev/null || exit 1
8186
"$YARN_BINARY" --silent node scripts/generate-specs-cli.js ios "$SCHEMA_FILE" "$TEMP_OUTPUT_DIR" "$CODEGEN_MODULES_LIBRARY_NAME"
82-
popd >/dev/null || exit
87+
popd >/dev/null || exit 1
8388

89+
describe "Copying output to final directory"
8490
mkdir -p "$CODEGEN_COMPONENTS_OUTPUT_DIR" "$CODEGEN_MODULES_OUTPUT_DIR"
85-
mv "$TEMP_OUTPUT_DIR/$CODEGEN_MODULES_LIBRARY_NAME.h" "$TEMP_OUTPUT_DIR/$CODEGEN_MODULES_LIBRARY_NAME-generated.mm" "$CODEGEN_MODULES_OUTPUT_DIR"
86-
find "$TEMP_OUTPUT_DIR" -type f | xargs sed -i '' "s/$CODEGEN_MODULES_LIBRARY_NAME/$CODEGEN_COMPONENTS_LIBRARY_NAME/g"
87-
cp -R "$TEMP_OUTPUT_DIR/." "$CODEGEN_COMPONENTS_OUTPUT_DIR"
91+
cp -R "$TEMP_OUTPUT_DIR/$CODEGEN_MODULES_LIBRARY_NAME.h" "$TEMP_OUTPUT_DIR/$CODEGEN_MODULES_LIBRARY_NAME-generated.mm" "$CODEGEN_MODULES_OUTPUT_DIR" || exit 1
92+
find "$TEMP_OUTPUT_DIR" -type f | xargs sed -i.bak "s/$CODEGEN_MODULES_LIBRARY_NAME/$CODEGEN_COMPONENTS_LIBRARY_NAME/g" || exit 1
93+
find "$TEMP_OUTPUT_DIR" -type f -not -iname "$CODEGEN_MODULES_LIBRARY_NAME*" -exec cp '{}' "$CODEGEN_COMPONENTS_OUTPUT_DIR/" ';' || exit 1
94+
95+
echo >&2 'Done.'
8896
}
8997

9098
trap cleanup EXIT

‎scripts/react_native_pods.rb

+18-17
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def use_react_native! (options={})
1818

1919
# The Pods which should be included in all projects
2020
pod 'FBLazyVector', :path => "#{prefix}/Libraries/FBLazyVector"
21-
pod 'FBReactNativeSpec', :path => "#{prefix}/Libraries/FBReactNativeSpec"
21+
pod 'FBReactNativeSpec', :path => "#{prefix}/React/FBReactNativeSpec"
2222
pod 'RCTRequired', :path => "#{prefix}/Libraries/RCTRequired"
2323
pod 'RCTTypeSafety', :path => "#{prefix}/Libraries/TypeSafety"
2424
pod 'React', :path => "#{prefix}/"
@@ -146,6 +146,8 @@ def react_native_post_install(installer)
146146
end
147147

148148
def use_react_native_codegen!(spec, options={})
149+
return if ENV['DISABLE_CODEGEN'] == '1'
150+
149151
# The path to react-native (e.g. react_native_path)
150152
prefix = options[:path] ||= File.join(__dir__, "..")
151153

@@ -154,26 +156,11 @@ def use_react_native_codegen!(spec, options={})
154156

155157
# Library name (e.g. FBReactNativeSpec)
156158
codegen_modules_library_name = spec.name
157-
codegen_modules_output_dir = options[:codegen_modules_output_dir] ||= File.join(prefix, "Libraries/#{codegen_modules_library_name}/#{codegen_modules_library_name}")
159+
codegen_modules_output_dir = options[:codegen_modules_output_dir] ||= File.join(prefix, "React/#{codegen_modules_library_name}/#{codegen_modules_library_name}")
158160

159161
# Run the codegen as part of the Xcode build pipeline.
160162
env_vars = "SRCS_DIR=#{srcs_dir}"
161163
env_vars += " CODEGEN_MODULES_OUTPUT_DIR=#{codegen_modules_output_dir}"
162-
if ENV['USE_FABRIC'] == '1'
163-
# We use a different library name for components, as well as an additional set of files.
164-
# Eventually, we want these to be part of the same library as #{codegen_modules_library_name} above.
165-
codegen_components_library_name = "rncore"
166-
codegen_components_output_dir = File.join(prefix, "ReactCommon/react/renderer/components/#{codegen_components_library_name}")
167-
env_vars += " CODEGEN_COMPONENTS_OUTPUT_DIR=#{codegen_components_output_dir}"
168-
end
169-
spec.script_phase = {
170-
:name => 'Generate Specs',
171-
:input_files => [srcs_dir],
172-
:output_files => ["$(DERIVED_FILE_DIR)/codegen-#{codegen_modules_library_name}.log"],
173-
:script => "bash -c '#{env_vars} CODEGEN_MODULES_LIBRARY_NAME=#{codegen_modules_library_name} #{File.join(__dir__, "generate-specs.sh")}' | tee \"${SCRIPT_OUTPUT_FILE_0}\"",
174-
:execution_position => :before_compile,
175-
:show_env_vars_in_log => true
176-
}
177164

178165
# Since the generated files are not guaranteed to exist when CocoaPods is run, we need to create
179166
# empty files to ensure the references are included in the resulting Pods Xcode project.
@@ -182,6 +169,11 @@ def use_react_native_codegen!(spec, options={})
182169
generated_files = generated_filenames.map { |filename| File.join(codegen_modules_output_dir, filename) }
183170

184171
if ENV['USE_FABRIC'] == '1'
172+
# We use a different library name for components, as well as an additional set of files.
173+
# Eventually, we want these to be part of the same library as #{codegen_modules_library_name} above.
174+
codegen_components_library_name = "rncore"
175+
codegen_components_output_dir = File.join(prefix, "ReactCommon/react/renderer/components/#{codegen_components_library_name}")
176+
env_vars += " CODEGEN_COMPONENTS_OUTPUT_DIR=#{codegen_components_output_dir}"
185177
mkdir_command += " #{codegen_components_output_dir}"
186178
components_generated_filenames = [
187179
"ComponentDescriptors.h",
@@ -196,5 +188,14 @@ def use_react_native_codegen!(spec, options={})
196188
generated_files = generated_files.concat(components_generated_filenames.map { |filename| File.join(codegen_components_output_dir, filename) })
197189
end
198190

191+
spec.script_phase = {
192+
:name => 'Generate Specs',
193+
:input_files => [srcs_dir],
194+
:output_files => ["$(DERIVED_FILE_DIR)/codegen-#{codegen_modules_library_name}.log"].concat(generated_files),
195+
:script => "set -o pipefail\n\nbash -l -c '#{env_vars} CODEGEN_MODULES_LIBRARY_NAME=#{codegen_modules_library_name} #{File.join(__dir__, "generate-specs.sh")}' 2>&1 | tee \"${SCRIPT_OUTPUT_FILE_0}\"",
196+
:execution_position => :before_compile,
197+
:show_env_vars_in_log => true
198+
}
199+
199200
spec.prepare_command = "#{mkdir_command} && touch #{generated_files.reduce() { |str, file| str + " " + file }}"
200201
end

0 commit comments

Comments
 (0)