From 12fccdeea33324b8ddaa3ac0e2dbf81a44ca1eb2 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Mon, 1 Feb 2021 14:36:52 -0800 Subject: [PATCH] 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: https://github.com/facebook/react-native/pull/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 --- .gitignore | 4 +-- React-Core.podspec | 1 + .../FBReactNativeSpec.podspec | 3 +- packages/rn-tester/Podfile.lock | 14 ++++---- scripts/generate-specs.sh | 28 +++++++++------ scripts/react_native_pods.rb | 35 ++++++++++--------- 6 files changed, 47 insertions(+), 38 deletions(-) rename {Libraries => React}/FBReactNativeSpec/FBReactNativeSpec.podspec (94%) diff --git a/.gitignore b/.gitignore index e71831ae100198..9aea329a6c35fd 100644 --- a/.gitignore +++ b/.gitignore @@ -85,9 +85,9 @@ package-lock.json # Libs that shouldn't have Xcode project /Libraries/FBLazyVector/**/*.xcodeproj -/Libraries/FBReactNativeSpec/**/*.xcodeproj /Libraries/RCTRequired/**/*.xcodeproj /React/CoreModules/**/*.xcodeproj +/React/FBReactNativeSpec/**/*.xcodeproj /packages/react-native-codegen/**/*.xcodeproj # CocoaPods @@ -100,7 +100,7 @@ package-lock.json !/packages/rn-tester/Pods/__offline_mirrors__ # react-native-codegen -/Libraries/FBReactNativeSpec/FBReactNativeSpec +/React/FBReactNativeSpec/FBReactNativeSpec /packages/react-native-codegen/lib /ReactCommon/react/renderer/components/rncore/ diff --git a/React-Core.podspec b/React-Core.podspec index 07f0ec398b3536..8ffb65908f60db 100644 --- a/React-Core.podspec +++ b/React-Core.podspec @@ -57,6 +57,7 @@ Pod::Spec.new do |s| ss.exclude_files = "React/CoreModules/**/*", "React/DevSupport/**/*", "React/Fabric/**/*", + "React/FBReactNativeSpec/**/*", "React/Tests/**/*", "React/Inspector/**/*" ss.private_header_files = "React/Cxx*/*.h" diff --git a/Libraries/FBReactNativeSpec/FBReactNativeSpec.podspec b/React/FBReactNativeSpec/FBReactNativeSpec.podspec similarity index 94% rename from Libraries/FBReactNativeSpec/FBReactNativeSpec.podspec rename to React/FBReactNativeSpec/FBReactNativeSpec.podspec index 0be878326215b2..f87e08309f225d 100644 --- a/Libraries/FBReactNativeSpec/FBReactNativeSpec.podspec +++ b/React/FBReactNativeSpec/FBReactNativeSpec.podspec @@ -31,13 +31,12 @@ Pod::Spec.new do |s| s.compiler_flags = folly_compiler_flags + ' -Wno-nullability-completeness' s.source = source s.source_files = "**/*.{c,h,m,mm,cpp}" - s.exclude_files = "jni" s.header_dir = "FBReactNativeSpec" s.pod_target_xcconfig = { "USE_HEADERMAP" => "YES", "CLANG_CXX_LANGUAGE_STANDARD" => "c++14", - "HEADER_SEARCH_PATHS" => "\"$(PODS_TARGET_SRCROOT)/Libraries/FBReactNativeSpec\" \"$(PODS_ROOT)/RCT-Folly\"" + "HEADER_SEARCH_PATHS" => "\"$(PODS_TARGET_SRCROOT)/React/FBReactNativeSpec\" \"$(PODS_ROOT)/RCT-Folly\"" } s.dependency "RCT-Folly", folly_version diff --git a/packages/rn-tester/Podfile.lock b/packages/rn-tester/Podfile.lock index 3181e51504f2d0..2d7c3de9247830 100644 --- a/packages/rn-tester/Podfile.lock +++ b/packages/rn-tester/Podfile.lock @@ -1,6 +1,6 @@ PODS: - boost-for-react-native (1.63.0) - - CocoaAsyncSocket (7.6.4) + - CocoaAsyncSocket (7.6.5) - CocoaLibEvent (1.0.0) - DoubleConversion (1.1.6) - FBLazyVector (1000.0.0) @@ -653,7 +653,7 @@ PODS: DEPENDENCIES: - DoubleConversion (from `../../third-party-podspecs/DoubleConversion.podspec`) - FBLazyVector (from `../../Libraries/FBLazyVector`) - - FBReactNativeSpec (from `../../Libraries/FBReactNativeSpec`) + - FBReactNativeSpec (from `../../React/FBReactNativeSpec`) - Flipper (~> 0.54.0) - Flipper-DoubleConversion (= 1.1.7) - Flipper-Folly (~> 2.2) @@ -730,7 +730,7 @@ EXTERNAL SOURCES: FBLazyVector: :path: "../../Libraries/FBLazyVector" FBReactNativeSpec: - :path: "../../Libraries/FBReactNativeSpec" + :path: "../../React/FBReactNativeSpec" glog: :podspec: "../../third-party-podspecs/glog.podspec" RCT-Folly: @@ -794,11 +794,11 @@ EXTERNAL SOURCES: SPEC CHECKSUMS: boost-for-react-native: 39c7adb57c4e60d6c5479dd8623128eb5b3f0f2c - CocoaAsyncSocket: 694058e7c0ed05a9e217d1b3c7ded962f4180845 + CocoaAsyncSocket: 065fd1e645c7abab64f7a6a2007a48038fdc6a99 CocoaLibEvent: 2fab71b8bd46dd33ddb959f7928ec5909f838e3f DoubleConversion: cde416483dac037923206447da6e1454df403714 FBLazyVector: 91e874a8823933a268c38765a88cbd5dba1fa024 - FBReactNativeSpec: 17a863c5e24969051850a3acab3a06069bb06e7f + FBReactNativeSpec: 58a907f57c40ca74a954abe86862baa5eb423c63 Flipper: be611d4b742d8c87fbae2ca5f44603a02539e365 Flipper-DoubleConversion: 38631e41ef4f9b12861c67d17cb5518d06badc41 Flipper-Folly: e4493b013c02d9347d5e0cb4d128680239f6c78a @@ -808,12 +808,12 @@ SPEC CHECKSUMS: FlipperKit: ab353d41aea8aae2ea6daaf813e67496642f3d7d glog: 40a13f7840415b9a77023fbcae0f1e6f43192af3 OpenSSL-Universal: ff34003318d5e1163e9529b08470708e389ffcdd - RCT-Folly: b39288cedafe50da43317ec7d91bcc8cc0abbf33 + RCT-Folly: ec7a233ccc97cc556cf7237f0db1ff65b986f27c RCTRequired: d3d4ce60e1e2282864d7560340690a3c8c646de1 RCTTypeSafety: 4da4f9f218727257c50fd3bf2683a06cdb4fede3 React: 63b1f2a4e0e908c95416fd54e9dcca5d409e2a45 React-callinvoker: e9524d75cf0b7ae108868f8d34c0b8d7dc08ec03 - React-Core: 2b2a8ac8bfb65779965456570a871f4cf5e5e03a + React-Core: c63389ffebc383f834a2cae4d1c120968ffb3cdb React-CoreModules: 87f011fa87190ffe979e443ce578ec93ec6ff4d4 React-cxxreact: 14cce64344ab482615dfe82a2cbea6eb73be6481 React-Fabric: 1744b2e94f5ed2ab247f3a55fd9762d55ed63f3b diff --git a/scripts/generate-specs.sh b/scripts/generate-specs.sh index cd4ef2902d2bdf..6397a7f3fb01f7 100755 --- a/scripts/generate-specs.sh +++ b/scripts/generate-specs.sh @@ -11,7 +11,7 @@ # Optionally, set these envvars to override defaults: # - SRCS_DIR: Path to JavaScript sources # - CODEGEN_MODULES_LIBRARY_NAME: Defaults to FBReactNativeSpec -# - CODEGEN_MODULES_OUTPUT_DIR: Defaults to Libraries/$CODEGEN_MODULES_LIBRARY_NAME/$CODEGEN_MODULES_LIBRARY_NAME +# - CODEGEN_MODULES_OUTPUT_DIR: Defaults to React/$CODEGEN_MODULES_LIBRARY_NAME/$CODEGEN_MODULES_LIBRARY_NAME # - CODEGEN_COMPONENTS_LIBRARY_NAME: Defaults to rncore # - CODEGEN_COMPONENTS_OUTPUT_DIR: Defaults to ReactCommon/react/renderer/components/$CODEGEN_COMPONENTS_LIBRARY_NAME # @@ -27,7 +27,7 @@ set -e THIS_DIR=$(cd -P "$(dirname "$(readlink "${BASH_SOURCE[0]}" || echo "${BASH_SOURCE[0]}")")" && pwd) TEMP_DIR=$(mktemp -d /tmp/react-native-codegen-XXXXXXXX) RN_DIR=$(cd "$THIS_DIR/.." && pwd) -YARN_BINARY="${YARN_BINARY:-$(command -v yarn)}" +YARN_BINARY="${YARN_BINARY:-$(command -v yarn || true)}" USE_FABRIC="${USE_FABRIC:-0}" cleanup () { @@ -45,7 +45,7 @@ main() { CODEGEN_MODULES_LIBRARY_NAME=${CODEGEN_MODULES_LIBRARY_NAME:-FBReactNativeSpec} CODEGEN_COMPONENTS_LIBRARY_NAME=${CODEGEN_COMPONENTS_LIBRARY_NAME:-rncore} - CODEGEN_MODULES_OUTPUT_DIR=${CODEGEN_MODULES_OUTPUT_DIR:-"$RN_DIR/Libraries/$CODEGEN_MODULES_LIBRARY_NAME/$CODEGEN_MODULES_LIBRARY_NAME"} + CODEGEN_MODULES_OUTPUT_DIR=${CODEGEN_MODULES_OUTPUT_DIR:-"$RN_DIR/React/$CODEGEN_MODULES_LIBRARY_NAME/$CODEGEN_MODULES_LIBRARY_NAME"} # TODO: $CODEGEN_COMPONENTS_PATH should be programmatically specified, and may change with use_frameworks! support. CODEGEN_COMPONENTS_PATH="ReactCommon/react/renderer/components" CODEGEN_COMPONENTS_OUTPUT_DIR=${CODEGEN_COMPONENTS_OUTPUT_DIR:-"$RN_DIR/$CODEGEN_COMPONENTS_PATH/$CODEGEN_COMPONENTS_LIBRARY_NAME"} @@ -56,6 +56,11 @@ main() { CODEGEN_REPO_PATH="$RN_DIR/packages/react-native-codegen" CODEGEN_NPM_PATH="$RN_DIR/../react-native-codegen" + if [ -z "$YARN_BINARY" ]; then + echo "Error: Could not find yarn. Make sure it is in bash PATH or set the YARN_BINARY environment variable." 1>&2 + exit 1 + fi + if [ -d "$CODEGEN_REPO_PATH" ]; then CODEGEN_PATH=$(cd "$CODEGEN_REPO_PATH" && pwd) elif [ -d "$CODEGEN_NPM_PATH" ]; then @@ -67,24 +72,27 @@ main() { if [ ! -d "$CODEGEN_PATH/lib" ]; then describe "Building react-native-codegen package" - pushd "$CODEGEN_PATH" >/dev/null || exit + pushd "$CODEGEN_PATH" >/dev/null || exit 1 "$YARN_BINARY" "$YARN_BINARY" build - popd >/dev/null || exit + popd >/dev/null || exit 1 fi describe "Generating schema from flow types" "$YARN_BINARY" node "$CODEGEN_PATH/lib/cli/combine/combine-js-to-schema-cli.js" "$SCHEMA_FILE" "$SRCS_DIR" describe "Generating native code from schema (iOS)" - pushd "$RN_DIR" >/dev/null || exit + pushd "$RN_DIR" >/dev/null || exit 1 "$YARN_BINARY" --silent node scripts/generate-specs-cli.js ios "$SCHEMA_FILE" "$TEMP_OUTPUT_DIR" "$CODEGEN_MODULES_LIBRARY_NAME" - popd >/dev/null || exit + popd >/dev/null || exit 1 + describe "Copying output to final directory" mkdir -p "$CODEGEN_COMPONENTS_OUTPUT_DIR" "$CODEGEN_MODULES_OUTPUT_DIR" - mv "$TEMP_OUTPUT_DIR/$CODEGEN_MODULES_LIBRARY_NAME.h" "$TEMP_OUTPUT_DIR/$CODEGEN_MODULES_LIBRARY_NAME-generated.mm" "$CODEGEN_MODULES_OUTPUT_DIR" - find "$TEMP_OUTPUT_DIR" -type f | xargs sed -i '' "s/$CODEGEN_MODULES_LIBRARY_NAME/$CODEGEN_COMPONENTS_LIBRARY_NAME/g" - cp -R "$TEMP_OUTPUT_DIR/." "$CODEGEN_COMPONENTS_OUTPUT_DIR" + 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 + find "$TEMP_OUTPUT_DIR" -type f | xargs sed -i.bak "s/$CODEGEN_MODULES_LIBRARY_NAME/$CODEGEN_COMPONENTS_LIBRARY_NAME/g" || exit 1 + find "$TEMP_OUTPUT_DIR" -type f -not -iname "$CODEGEN_MODULES_LIBRARY_NAME*" -exec cp '{}' "$CODEGEN_COMPONENTS_OUTPUT_DIR/" ';' || exit 1 + + echo >&2 'Done.' } trap cleanup EXIT diff --git a/scripts/react_native_pods.rb b/scripts/react_native_pods.rb index 2e01cfaebfae46..dde169e9be1d6f 100644 --- a/scripts/react_native_pods.rb +++ b/scripts/react_native_pods.rb @@ -18,7 +18,7 @@ def use_react_native! (options={}) # The Pods which should be included in all projects pod 'FBLazyVector', :path => "#{prefix}/Libraries/FBLazyVector" - pod 'FBReactNativeSpec', :path => "#{prefix}/Libraries/FBReactNativeSpec" + pod 'FBReactNativeSpec', :path => "#{prefix}/React/FBReactNativeSpec" pod 'RCTRequired', :path => "#{prefix}/Libraries/RCTRequired" pod 'RCTTypeSafety', :path => "#{prefix}/Libraries/TypeSafety" pod 'React', :path => "#{prefix}/" @@ -146,6 +146,8 @@ def react_native_post_install(installer) end def use_react_native_codegen!(spec, options={}) + return if ENV['DISABLE_CODEGEN'] == '1' + # The path to react-native (e.g. react_native_path) prefix = options[:path] ||= File.join(__dir__, "..") @@ -154,26 +156,11 @@ def use_react_native_codegen!(spec, options={}) # Library name (e.g. FBReactNativeSpec) codegen_modules_library_name = spec.name - codegen_modules_output_dir = options[:codegen_modules_output_dir] ||= File.join(prefix, "Libraries/#{codegen_modules_library_name}/#{codegen_modules_library_name}") + codegen_modules_output_dir = options[:codegen_modules_output_dir] ||= File.join(prefix, "React/#{codegen_modules_library_name}/#{codegen_modules_library_name}") # Run the codegen as part of the Xcode build pipeline. env_vars = "SRCS_DIR=#{srcs_dir}" env_vars += " CODEGEN_MODULES_OUTPUT_DIR=#{codegen_modules_output_dir}" - if ENV['USE_FABRIC'] == '1' - # We use a different library name for components, as well as an additional set of files. - # Eventually, we want these to be part of the same library as #{codegen_modules_library_name} above. - codegen_components_library_name = "rncore" - codegen_components_output_dir = File.join(prefix, "ReactCommon/react/renderer/components/#{codegen_components_library_name}") - env_vars += " CODEGEN_COMPONENTS_OUTPUT_DIR=#{codegen_components_output_dir}" - end - spec.script_phase = { - :name => 'Generate Specs', - :input_files => [srcs_dir], - :output_files => ["$(DERIVED_FILE_DIR)/codegen-#{codegen_modules_library_name}.log"], - :script => "bash -c '#{env_vars} CODEGEN_MODULES_LIBRARY_NAME=#{codegen_modules_library_name} #{File.join(__dir__, "generate-specs.sh")}' | tee \"${SCRIPT_OUTPUT_FILE_0}\"", - :execution_position => :before_compile, - :show_env_vars_in_log => true - } # Since the generated files are not guaranteed to exist when CocoaPods is run, we need to create # 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={}) generated_files = generated_filenames.map { |filename| File.join(codegen_modules_output_dir, filename) } if ENV['USE_FABRIC'] == '1' + # We use a different library name for components, as well as an additional set of files. + # Eventually, we want these to be part of the same library as #{codegen_modules_library_name} above. + codegen_components_library_name = "rncore" + codegen_components_output_dir = File.join(prefix, "ReactCommon/react/renderer/components/#{codegen_components_library_name}") + env_vars += " CODEGEN_COMPONENTS_OUTPUT_DIR=#{codegen_components_output_dir}" mkdir_command += " #{codegen_components_output_dir}" components_generated_filenames = [ "ComponentDescriptors.h", @@ -196,5 +188,14 @@ def use_react_native_codegen!(spec, options={}) generated_files = generated_files.concat(components_generated_filenames.map { |filename| File.join(codegen_components_output_dir, filename) }) end + spec.script_phase = { + :name => 'Generate Specs', + :input_files => [srcs_dir], + :output_files => ["$(DERIVED_FILE_DIR)/codegen-#{codegen_modules_library_name}.log"].concat(generated_files), + :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}\"", + :execution_position => :before_compile, + :show_env_vars_in_log => true + } + spec.prepare_command = "#{mkdir_command} && touch #{generated_files.reduce() { |str, file| str + " " + file }}" end