-
Notifications
You must be signed in to change notification settings - Fork 404
run-ios
in release configuration
#249
Comments
Action Items:
|
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 😁 |
Xcode 13 only works with same workarounds, I've tested. Also needs the swift hack I think, to set library path |
@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) |
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 https://github.com/facebook/react-native/blob/main/scripts/react-native-xcode.sh#L77 We'll fix forward and pick to 0.66 RC |
@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 Thanks for all your help! |
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 |
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! Cheers! |
@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. |
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 |
(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...) |
Yeah understood, we're just trying to buy some time atm :). I'll add your workaround with a big warning |
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
Alright, ENTRY_FILE fix: facebook/react-native@945c5f7 |
Keeping this open until commits are picked and in case you wanna verify @mikehardy |
Hey @fkgozali i checked and the commit listed as fixing m1 builds does not work 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 |
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 :) |
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 🤞 |
Alright, here's the commit: facebook/react-native@329b026 -- will request pick |
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
Closing as this is now fixed in 0.66.0-rc.2 |
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
and getting error:
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:
and getting the error:
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.
The text was updated successfully, but these errors were encountered: