Skip to content
This repository has been archived by the owner on Jun 17, 2022. It is now read-only.

run-ios in release configuration #249

Closed
lunaleaps opened this issue Aug 31, 2021 · 19 comments
Closed

run-ios in release configuration #249

lunaleaps opened this issue Aug 31, 2021 · 19 comments

Comments

@lunaleaps
Copy link
Collaborator

There are a couple of issues with npx react-native run-ios --configuration 'Release'

1. index.js does not exist

@mikehardy has reported an error with

npx react-native init TestProject66 --version 0.66.0-rc.0
cd TestProject66
npx react-native run-ios --configuration 'Release'

and getting error:

❌  error: Entry file index.js does not exist. If you use another file as your entry point, pass ENTRY_FILE=myindex.js

2. thread-local storage is not supported for the current target

@lunaleaps has issue with Xcode 12.5 for both 0.65.1 and 0.66.0-rc.0 when running:

npx react-native init TestProject [--version 0.66.0-rc.0]
cd TestProject
npx react-native run-ios --configuration 'Release'

and getting the error:

warning: Capabilities for Signing & Capabilities may not function correctly because its entitlements use a placeholder team ID. To resolve this, select a development team in the MyTestProject66 editor. (in target 'MyTestProject66' from project 'MyTestProject66')
/Users/luwe/MyTestProject66/ios/Pods/Pods.xcodeproj: warning: The iOS Simulator deployment target 'IPHONEOS_DEPLOYMENT_TARGET' is set to 8.0, but the range of supported deployment target versions is 9.0 to 14.5.99. (in target 'YogaKit' from project 'Pods')
/Users/luwe/MyTestProject66/ios/Pods/Pods.xcodeproj: warning: The iOS Simulator deployment target 'IPHONEOS_DEPLOYMENT_TARGET' is set to 8.0, but the range of supported deployment target versions is 9.0 to 14.5.99. (in target 'Flipper-Glog' from project 'Pods')

The following build commands failed:
    CompileC /Users/luwe/Library/Developer/Xcode/DerivedData/MyTestProject66-efwtfjnninejtqfidwsnxehgajhj/Build/Intermediates.noindex/Pods.build/Release-iphonesimulator/RCT-Folly.build/Objects-normal/i386/F14Table.o /Users/luwe/MyTestProject66/ios/Pods/RCT-Folly/folly/container/detail/F14Table.cpp normal i386 c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

This seems like a known issue and has some discussion here: facebook/react-native#31480 (comment)

@lunaleaps has tried following @mikehardy's suggestions by adding this to Podfile: facebook/react-native#31480 (comment) but hasn't worked.

@lunaleaps lunaleaps mentioned this issue Aug 31, 2021
30 tasks
@lunaleaps
Copy link
Collaborator Author

lunaleaps commented Aug 31, 2021

Action Items:

  • If anyone can repro number 1, please comment.
  • For number 2, @lunaleaps is going to try with Xcode 13 and see if run into same issues

@mikehardy
Copy link

I'm surprised the post install doesn't work, it must not have been applied correctly, if it really moved deployment target up, then the symbol exists and f14table compiles this is a rare 'really really does work around it' 100% certainty workaround. That sad, still needs a real fix 😁

@mikehardy
Copy link

Xcode 13 only works with same workarounds, I've tested. Also needs the swift hack I think, to set library path

@fkgozali
Copy link
Collaborator

fkgozali commented Sep 1, 2021

@mikehardy I did a clean try on Xcode 12.5. With your post_install workaround, I was able to successfully build the Release flavor. I couldn't repro the index.js issue though

The post_install hack: facebook/react-native#31480 (comment)

@fkgozali
Copy link
Collaborator

fkgozali commented Sep 1, 2021

I couldn't repro the index.js issue though

oops, looks like I accidentally had an unrelated index.js in a parent dir, so that "avoided" the problem but incorrectly. The issue is this line is looking for index.js at the wrong directory level...:

https://github.com/facebook/react-native/blob/main/scripts/react-native-xcode.sh#L77

We'll fix forward and pick to 0.66 RC

@lunaleaps
Copy link
Collaborator Author

@mikehardy Yup as Kevin mentioned we were able to repro your issue and found the cause to be changes in facebook/react-native#29263

Additionally, I got around the thread-local storage is not supported for the current target error with your "dance" post_install hook. I just didn't realize I needed to run pod install after updating the Podfile. I thought npx react-native run-ios.. would run it..whoops!

Thanks for all your help!

@lunaleaps
Copy link
Collaborator Author

But as a side-note.. we should figure out how to not have to do this "dance".. I'm not exactly sure what that would be.. but on my radar

@mikehardy
Copy link

I also would really love to avoid the intricate post-install hackery but:

a) I am happy that I'm not crazy / I'm generating reproducible reports at all. I mean, that's like 90% of the job and it's quite frustrating at times!
b) I am really happy that you were able to see the full requirement of post-install hackery needed, and also see that when you put it all together, it really does work, see a) about reproduction :-)
b) I am really really happy that you were able to find the release mode issue - since that's the only issue I saw, makes me excited to see 0.66 wind its way to release

Cheers!

@fkgozali
Copy link
Collaborator

fkgozali commented Sep 1, 2021

@mikehardy - any concern if we include your post_install workaround in the app template for 0.66 for now? Yes it's not ideal, but it would unblock many ppl atm.

@mikehardy
Copy link

None at all - I've been using it personally and hearing success feedback on it for all the major configs (M1 or intel, hermes or no) so it should improve dev experience, and worst case it's just the template, people can rip it out

@mikehardy
Copy link

(although that said, no good deed will be unpunished, I'm sure we'll discover some new corner cases if/when it goes in - still, incremental improvement...)

@fkgozali
Copy link
Collaborator

fkgozali commented Sep 1, 2021

Yeah understood, we're just trying to buy some time atm :). I'll add your workaround with a big warning

facebook-github-bot pushed a commit to facebook/react-native that referenced this issue Sep 1, 2021
Summary:
The original $ENTRY_FILE check was added in #29012 to help catch misconfiguration for the entry JS file. That turned out breaking some RNTester builds/tests, so #29263 was added to accommodate the fact that RNTester .xcodeproj file has its own directory hierarchy.

The 2nd PR had multiple issues:
* It is incorrect to assume that the $ENTRY_FILE always exists in the parent dir of the .xcodeproj location. This caused an issue in RC 0.66: react-native-community/releases#249 (comment)
* RNTester has since moved to packages/rn-tester/ (from RNTester/), hence breaking that assumption

It turns out RNTester .xcodeproj has incorrectly misconfigured this JS bundling step (not sure since when). The original script invocation passed in the correct path for `RNTesterApp.ios.js`, but as an arg to the `react-native-xcode.sh` instead of by setting `ENTRY_FILE` env var.

So this diff does 2 things:
* Undid #29263
* Fix RNTester JS bundling invocation to set the ENTRY_FILE correctly

{F659123377}

Changelog: [iOS][Fixed] Unbreak $ENTRY_FILE handling for JS bundling

Reviewed By: lunaleaps

Differential Revision: D30690900

fbshipit-source-id: 7c5802b3eac56c0456edcd4b7478bfa4af48fc27
@fkgozali
Copy link
Collaborator

fkgozali commented Sep 1, 2021

Alright, ENTRY_FILE fix: facebook/react-native@945c5f7
And post_install recipe: facebook/react-native@ac4ddec

@fkgozali
Copy link
Collaborator

fkgozali commented Sep 1, 2021

Keeping this open until commits are picked and in case you wanna verify @mikehardy

@mikehardy
Copy link

Hey @fkgozali i checked and the commit listed as fixing m1 builds does not work

facebook/react-native@eb93886

That might fix xcode 13 on Intel, but m1 requires swift libs first not last. Of course that seems strange but it's a real thing, I tested both variations, only swift first builds on m1 in my experience. Unexpected result but that's what I saw, and there's positive results with it on the Apple m1 issue.

Discussion visible on ios discord channel

@fkgozali
Copy link
Collaborator

fkgozali commented Sep 1, 2021

That might fix xcode 13 on Intel, but m1 requires swift libs first not last.

Yeah I saw your discussion with @sammy-SC. In fact, a commit is landing as of now to switch that order in app template and RNTester. So with those picked to RC 0.66, hopefully that's all we need :)

@mikehardy
Copy link

Fantastic! Then yes - with that order being swift-first and the Folly "dance" (ios version bump + avoiding redefining time symbol) everything on Apple machines should be happy 🤞

@fkgozali
Copy link
Collaborator

fkgozali commented Sep 1, 2021

Alright, here's the commit: facebook/react-native@329b026 -- will request pick

lunaleaps pushed a commit to facebook/react-native that referenced this issue Sep 1, 2021
Summary:
The original $ENTRY_FILE check was added in #29012 to help catch misconfiguration for the entry JS file. That turned out breaking some RNTester builds/tests, so #29263 was added to accommodate the fact that RNTester .xcodeproj file has its own directory hierarchy.

The 2nd PR had multiple issues:
* It is incorrect to assume that the $ENTRY_FILE always exists in the parent dir of the .xcodeproj location. This caused an issue in RC 0.66: react-native-community/releases#249 (comment)
* RNTester has since moved to packages/rn-tester/ (from RNTester/), hence breaking that assumption

It turns out RNTester .xcodeproj has incorrectly misconfigured this JS bundling step (not sure since when). The original script invocation passed in the correct path for `RNTesterApp.ios.js`, but as an arg to the `react-native-xcode.sh` instead of by setting `ENTRY_FILE` env var.

So this diff does 2 things:
* Undid #29263
* Fix RNTester JS bundling invocation to set the ENTRY_FILE correctly

{F659123377}

Changelog: [iOS][Fixed] Unbreak $ENTRY_FILE handling for JS bundling

Reviewed By: lunaleaps

Differential Revision: D30690900

fbshipit-source-id: 7c5802b3eac56c0456edcd4b7478bfa4af48fc27
@lunaleaps
Copy link
Collaborator Author

lunaleaps commented Sep 10, 2021

Closing as this is now fixed in 0.66.0-rc.2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants