[iOS] Use relative paths for pod spec to keep checksum stable #31195

thaapasa · 2021-03-19T09:29:37Z

Summary

The SPEC CHECKSUM for FBReactNativeSpec in ios/Podfile.lock varies between development machines. This is caused by local paths in the output of pod ipc spec FBReactNativeSpec.podspec, such as output_files and prepare_command. This causes the Podfile.lock to constantly change when pod install is run on different developers' machines.

See issue: #31193

Changelog

[iOS] [Fixed] - Fixed pod spec generation to produce relative paths to keep FBReactNativeSpec pod checksum stable

Test Plan

Apply the changes , run pod install and do a full rebuild for the iOS project (delete ~/Library/Developer/Xcode/DerivedData before building). You should get a working app even with the paths changed.

Also verify that prepare phase creates the correct directory and empty files. I.e., there should not be extranous node_modules/react-native/React/FBReactNativeSpec subpaths anywhere (such as under the actual node_modules/react-native folder).

facebook-github-bot · 2021-03-19T09:29:42Z

Hi @thaapasa!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

facebook-github-bot · 2021-03-19T09:58:52Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

thaapasa · 2021-03-19T11:13:46Z

This still leaves a script path at the end of generated podspec with local paths, so some more changes are still needed.

thaapasa · 2021-03-19T12:13:16Z

I fixed the path to genrate-specs.sh also so the spec should now be stable 🤞

FreekVanDooren · 2021-03-19T17:50:19Z

scripts/react_native_pods.rb

@@ -192,7 +192,7 @@ def use_react_native_codegen!(spec, options={})
    :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}\"",
+    :script => "set -o pipefail\n\nbash -l -c '#{env_vars} CODEGEN_MODULES_LIBRARY_NAME=#{codegen_modules_library_name} ../../node_modules/react-native/scripts/generate-specs.sh' 2>&1 | tee \"${SCRIPT_OUTPUT_FILE_0}\"",


This is just a question to showcase my ignorance: I have fixed this locally by using SRCROOT which will resolve to an absolute path at runtime, because I was under the impression that would be the more 'canonical' way. Do you think it is correct to do so (my build passes, so that's at least 1 criterion) or is it better to have it relative like you have done?

Is this SRCROOT something that is already defined by the build process (and does it point to the same absolute directory in the prepare command and script phase)? If so, that would definitely be better in my opinion; I don't like this relative addressing at all (I just don't know enough about this build system to suggest a good fix for the issue).

It's pretty obscure to me too, luckily in the logs on our build server all environment variables are printed at one point. Here is a small excerpt of ones that I think could be relevant in this context:

export PODS_ROOT\=/Users/runner/work/1/s/ios/Pods export PODS_TARGET_SRCROOT\=/Users/runner/work/1/s/ios/Pods/../../node_modules/react-native/React/FBReactNativeSpec export SOURCE_ROOT\=/Users/runner/work/1/s/ios/Pods export SRCROOT\=/Users/runner/work/1/s/ios/Pods

There is some overlap here and my initial googling hasn't exactly given me any pointers as to which ones are future proof. I actually ended up using PODS_ROOT for the prefix, because on my machine SRCROOT turned out to be pointing to /, which gave me permission errors.

Anyway, here is some documentation as well, which could potentially proof helpful: https://developer.apple.com/library/archive/documentation/DeveloperTools/Reference/XcodeBuildSettingRef/1-Build_Setting_Reference/build_setting_ref.html.

I tried to test this myself and verified that I have similar variables properly set in the script phase. However, when prepare command is run (pod install) all the variables I tested were unset, so the paths would be incorrect in that phase if the paths are based on those variables.

martintreurnicht · 2021-03-19T22:57:50Z

scripts/react_native_pods.rb

@@ -149,7 +149,7 @@ 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__, "..")
+  prefix = options[:path] ||= "../../node_modules/react-native"


This is the correct path for the script_phase but not for prepare_command, prepare_command is relative to the podspec, and not PODS_ROOT like script_phase. Not sure how this is working for you, All your generated code should be in the wrong directory. It might work but is not the expected result. The generated code should exist under node_modules/react-native/React/

Thank you @martintreurnicht, you are correct. The prepare command was run for the wrong path; after running pod install an extra node_modules directory was created under node_modules/react-native.

The was working for me because apparently the prepare command is a bit redundant; after checking out react-native the empty files are already there so just the timestamps were not updated.

I've fixed the script now. The end result is a hack that I do not like, but at least it should work now. If someone knows better how the Pod build works and how this problem should be properly addressed please let me know (or make another PR and this one can be closed).

For reference (TIL): the prepare_command is run with pod install; you can check the working directory by adding pwd && exit 1 at the beginning of it (spoiler: it's /local/dev/path/node_modules/react-native/React/FBReactNativeSpec). Also, the script_phase is run with the actual app build (yarn run-ios).

First off, thanks for bringing this issue to my attention! I agree this needs to be fixed.
I am not sure at this moment that hard-coding this prefix to "../../node_modules/react-native" will work in every scenario. While this path is valid for standalone apps that consume react-native from npm, the path will not exist if this script is invoked when pod install is run within packages/rn-tester (e.g. when building RNTester as a contributor to React Native).

FreekVanDooren · 2021-03-22T11:35:10Z

scripts/react_native_pods.rb

@@ -192,10 +192,10 @@ def use_react_native_codegen!(spec, options={})
    :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}\"",
+    :script => "set -o pipefail\n\nbash -l -c '#{env_vars} CODEGEN_MODULES_LIBRARY_NAME=#{codegen_modules_library_name} ../../node_modules/react-native/scripts/generate-specs.sh' 2>&1 | tee \"${SCRIPT_OUTPUT_FILE_0}\"",
    :execution_position => :before_compile,
    :show_env_vars_in_log => true


This setting is probably the reason those variables are printed.

franciscomorais · 2021-03-28T15:54:05Z

@thaapasa Ref this issue #31121 too. Thanks.

fabOnReact

sorry, I notice after posting that it does not fix the issue https://github.com/facebook/react-native/pull/31195/files#r605862106. I'll keep working in finding a solution to the issue experienced while building packages/rn-tester. Thanks and sorry 🙏 ☮️

CLICK TO OPEN THE COMMENT

thanks a lot for the pr @thaapasa 🙏
I gave a quick look and wanted to help you with this issue, I tested this solution, relative_path_from converts the absolute path in a relative_path https://stackoverflow.com/a/9416682

Pathname.new(File.join(__dir__, "..")).relative_path_from(File.join(__dir__))

returns .. which refers to the relative parent folder. It should be correct because the script is in the folder react-native/scripts/react_native_pods.rb

The output from running

cd node_modules/react-native/React/FBReactNativeSpec
pod ipc spec FBReactNativeSpec.podspec

CLICK TO OPEN THE FULL OUTPUT

{
  "name": "FBReactNativeSpec",
  "version": "0.64.0",
  "summary": "-",
  "homepage": "https://reactnative.dev/",
  "license": "MIT",
  "authors": "Facebook, Inc. and its affiliates",
  "platforms": {
    "ios": "10.0"
  },
  "compiler_flags": "-DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1 -Wno-comma -Wno-shorten-64-to-32 -Wno-nullability-completeness",
  "source": {
    "git": "https://github.com/facebook/react-native.git",
    "tag": "v0.64.0"
  },
  "source_files": "**/*.{c,h,m,mm,cpp}",
  "header_dir": "FBReactNativeSpec",
  "pod_target_xcconfig": {
    "USE_HEADERMAP": "YES",
    "CLANG_CXX_LANGUAGE_STANDARD": "c++14",
    "HEADER_SEARCH_PATHS": "\"$(PODS_TARGET_SRCROOT)/React/FBReactNativeSpec\" \"$(PODS_ROOT)/RCT-Folly\""
  },
  "dependencies": {
    "RCT-Folly": [
      "2020.01.13.00"
    ],
    "RCTRequired": [
      "0.64.0"
    ],
    "RCTTypeSafety": [
      "0.64.0"
    ],
    "React-Core": [
      "0.64.0"
    ],
    "React-jsi": [
      "0.64.0"
    ],
    "ReactCommon/turbomodule/core": [
      "0.64.0"
    ]
  },
  "script_phases": {
    "name": "Generate Specs",
    "input_files": [
      "../Libraries"
    ],
    "output_files": [
      "$(DERIVED_FILE_DIR)/codegen-FBReactNativeSpec.log",
      "../React/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h",
      "../React/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm"
    ],
    "script": "set -o pipefail\n\nbash -l -c 'SRCS_DIR=../Libraries CODEGEN_MODULES_OUTPUT_DIR=../React/FBReactNativeSpec/FBReactNativeSpec CODEGEN_MODULES_LIBRARY_NAME=FBReactNativeSpec /Users/fabriziobertoglio/Documents/sourcecode/opensource/TestApp/node_modules/react-native/scripts/generate-specs.sh' 2>&1 | tee \"${SCRIPT_OUTPUT_FILE_0}\"",
    "execution_position": "before_compile",
    "show_env_vars_in_log": true
  },
  "prepare_command": "mkdir -p ../React/FBReactNativeSpec/FBReactNativeSpec && touch ../React/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h ../React/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm"
}

Running pod install in the test project did not trigger any error.

But it does not fix the issue reported in https://github.com/facebook/react-native/pull/31195/files#r605862106, running rn-tester app on iOS will cause the following error.

bash: ../../node_modules/react-native/scripts/generate-specs.sh: No such file or directory

RESULT BEFORE APPLYING THIS CHANGE
The line:

prefix = options[:path] ||= File.join(__dir__, "..")

causes input_files being an absolute path of value "/Users/fabriziobertoglio/Documents/sourcecode/opensource/TestApp/node_modules/react-native/scripts/../Libraries"

CLICK TO OPEN THE FULL OUTPUT

"input_files": [
  "/Users/fabriziobertoglio/Documents/sourcecode/opensource/TestApp/node_modules/react-native/scripts/../Libraries"
],
"output_files": [
  "$(DERIVED_FILE_DIR)/codegen-FBReactNativeSpec.log",
  "/Users/fabriziobertoglio/Documents/sourcecode/opensource/TestApp/node_modules/react-native/scripts/../React/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h",
  "/Users/fabriziobertoglio/Documents/sourcecode/opensource/TestApp/node_modules/react-native/scripts/../React/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm"
],

RESULT AFTER APPLYING THIS CHANGE
the value input_files is a relative path "../Libraries"

CLICK TO OPEN THE FULL OUTPUT

"script_phases": {
"name": "Generate Specs",
"input_files": [
  "../Libraries"
],
"output_files": [
  "$(DERIVED_FILE_DIR)/codegen-FBReactNativeSpec.log",
  "../React/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h",
  "../React/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm"
],

fabOnReact · 2021-04-02T17:35:14Z

scripts/react_native_pods.rb

@@ -149,7 +149,7 @@ 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__, "..")
+  prefix = options[:path] ||= "../../node_modules/react-native"


Suggested change

prefix = options[:path] ||= "../../node_modules/react-native"

prefix = options[:path] ||= Pathname.new(File.join(__dir__, "..")).relative_path_from(File.join(__dir__))

hramos · 2021-04-02T18:52:14Z

sorry, I notice after posting that it does not fix the issue https://github.com/facebook/react-native/pull/31195/files#r605862106. I'll keep working in finding a solution to the issue experienced while building packages/rn-tester. Thanks and sorry 🙏 ☮️

CLICK TO OPEN THE COMMENT
thanks a lot for the pr @thaapasa 🙏
I gave a quick look and wanted to help you with this issue, I tested this solution, relative_path_from converts the absolute path in a relative_path https://stackoverflow.com/a/9416682
Pathname.new(File.join(__dir__, "..")).relative_path_from(File.join(__dir__))
returns .. which refers to the relative parent folder. It should be correct because the script is in the folder react-native/scripts/react_native_pods.rb

The output from running
cd node_modules/react-native/React/FBReactNativeSpec
pod ipc spec FBReactNativeSpec.podspec
CLICK TO OPEN THE FULL OUTPUT
Running pod install in the test project did not trigger any error.

But it does not fix the issue reported in https://github.com/facebook/react-native/pull/31195/files#r605862106, running rn-tester app on iOS will cause the following error.
bash: ../../node_modules/react-native/scripts/generate-specs.sh: No such file or directory
RESULT BEFORE APPLYING THIS CHANGE
The line:
prefix = options[:path] ||= File.join(__dir__, "..")
causes input_files being an absolute path of value "/Users/fabriziobertoglio/Documents/sourcecode/opensource/TestApp/node_modules/react-native/scripts/../Libraries"

CLICK TO OPEN THE FULL OUTPUT
RESULT AFTER APPLYING THIS CHANGE
the value input_files is a relative path "../Libraries"

CLICK TO OPEN THE FULL OUTPUT

Thanks for looking into this approach, even if the end result is not the right solution, it is useful to know.

If @thaapasa's proposal works for standalone apps but not RNTester, perhaps we could have RNTester pass the correct path via options. I'll take a look today.

hramos · 2021-04-16T21:58:01Z

Hey folks, I believe bdfe2a5 should address the root cause. I'll go ahead and close this PR.

Use relative paths for pod spec to keep checksum stable

2badc6f

thaapasa mentioned this pull request Mar 19, 2021

[iOS] FBReactNativeSpec pod checksum is not stable (RN 0.64.0) #31193

Closed

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 19, 2021

mlazari mentioned this pull request Mar 19, 2021

fix: codegen - project paths with spaces #31141

Closed

thaapasa marked this pull request as draft March 19, 2021 11:12

Fix path to generate-specs.sh to be relative

328bdec

thaapasa marked this pull request as ready for review March 19, 2021 12:13

Simplify

513501e

FreekVanDooren reviewed Mar 19, 2021

View reviewed changes

martintreurnicht reviewed Mar 19, 2021

View reviewed changes

Adjust working directory to match relative path

c2211d5

FreekVanDooren reviewed Mar 22, 2021

View reviewed changes

fabOnReact reviewed Apr 2, 2021

View reviewed changes

grabbou mentioned this pull request Apr 6, 2021

Upcoming v0.64.1 cherrypicks discussion react-native-community/releases#221

Closed

15 tasks

hramos closed this Apr 16, 2021

kelset mentioned this pull request May 5, 2021

Upcoming v0.64.2 cherrypicks discussion react-native-community/releases#224

Closed

14 tasks

stianjensen mentioned this pull request Jul 18, 2023

Podfile hashes encode absolute file path #38492

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[iOS] Use relative paths for pod spec to keep checksum stable #31195

[iOS] Use relative paths for pod spec to keep checksum stable #31195

thaapasa commented Mar 19, 2021 •

edited

Loading

facebook-github-bot commented Mar 19, 2021

facebook-github-bot commented Mar 19, 2021

thaapasa commented Mar 19, 2021

thaapasa commented Mar 19, 2021

FreekVanDooren Mar 19, 2021

thaapasa Mar 22, 2021

FreekVanDooren Mar 22, 2021

thaapasa Mar 22, 2021

martintreurnicht Mar 19, 2021 •

edited

Loading

thaapasa Mar 22, 2021

hramos Apr 1, 2021

FreekVanDooren Mar 22, 2021

franciscomorais commented Mar 28, 2021

fabOnReact left a comment •

edited

Loading

fabOnReact Apr 2, 2021

hramos commented Apr 2, 2021

hramos commented Apr 16, 2021

	prefix = options[:path] \|\|= "../../node_modules/react-native"
	prefix = options[:path] \|\|= Pathname.new(File.join(__dir__, "..")).relative_path_from(File.join(__dir__))

[iOS] Use relative paths for pod spec to keep checksum stable #31195

[iOS] Use relative paths for pod spec to keep checksum stable #31195

Conversation

thaapasa commented Mar 19, 2021 • edited Loading

Summary

Changelog

Test Plan

facebook-github-bot commented Mar 19, 2021

Action Required

Process

facebook-github-bot commented Mar 19, 2021

thaapasa commented Mar 19, 2021

thaapasa commented Mar 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martintreurnicht Mar 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

franciscomorais commented Mar 28, 2021

fabOnReact left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hramos commented Apr 2, 2021

hramos commented Apr 16, 2021

thaapasa commented Mar 19, 2021 •

edited

Loading

martintreurnicht Mar 19, 2021 •

edited

Loading

fabOnReact left a comment •

edited

Loading