From 5ada07857847bc342c6ccb0ce85f1122f43d5f19 Mon Sep 17 00:00:00 2001
From: Janic Duplessis <janicduplessis@gmail.com>
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.

[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 e71831ae100..9aea329a6c3 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 9a2be90c4bc..357c0093c5d 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 0be87832621..f87e08309f2 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 e8106c9217a..b601aae10ca 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: fe973c09b2299b5e8154186ecf1f6554b4f70987
-  FBReactNativeSpec: d0504078deb2ffa0fbee5032382f4ef165a1c8a8
+  FBLazyVector: 91e874a8823933a268c38765a88cbd5dba1fa024
+  FBReactNativeSpec: 58a907f57c40ca74a954abe86862baa5eb423c63
   Flipper: be611d4b742d8c87fbae2ca5f44603a02539e365
   Flipper-DoubleConversion: 38631e41ef4f9b12861c67d17cb5518d06badc41
   Flipper-Folly: e4493b013c02d9347d5e0cb4d128680239f6c78a
@@ -813,7 +813,7 @@ SPEC CHECKSUMS:
   RCTTypeSafety: 4da4f9f218727257c50fd3bf2683a06cdb4fede3
   React: 87b3271d925336a94620915db5845c67c5dbbd77
   React-callinvoker: e9524d75cf0b7ae108868f8d34c0b8d7dc08ec03
-  React-Core: 40874579985cf440232e3f33ce172be1f04d4964
+  React-Core: c63389ffebc383f834a2cae4d1c120968ffb3cdb
   React-CoreModules: 87f011fa87190ffe979e443ce578ec93ec6ff4d4
   React-cxxreact: de6de17eac6bbaa4f9fad46b66e7f0c4aaaf863d
   React-Fabric: 911e4b13fbffce46820f18c3a3b7a2a966e9e425
diff --git a/scripts/generate-specs.sh b/scripts/generate-specs.sh
index cd4ef2902d2..6397a7f3fb0 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 23ed2e59ee6..a31c51dbde3 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