React Native fix #401
React Native fix #401
Conversation
| @@ -76,7 +76,7 @@ void* GetPC(void* ucontext_in_void) { | |||
| #if (defined(HAVE_UCONTEXT_H) || defined(HAVE_SYS_UCONTEXT_H)) && defined(PC_FROM_UCONTEXT) | |||
| if (ucontext_in_void != NULL) { | |||
| ucontext_t *context = reinterpret_cast<ucontext_t *>(ucontext_in_void); | |||
| return (void*)context->PC_FROM_UCONTEXT; | |||
| return NULL; | |||
There was a problem hiding this comment.
Does this still work on other systems as expected? I'd say you need to check using CMake whether ucontext_t contains the PC_FROM_UCONTEXT member first and only if it is not the case return NULL.
There was a problem hiding this comment.
@sergiud @Alec13355 Hey guys, any progress with PR? This fix is really needed on a few real projects.... Please decide the best way and merge it
There was a problem hiding this comment.
So are you saying something to the effect of if
((void*)context->PC_FROM_UCONTEXT!= NULL)) return Null;
There was a problem hiding this comment.
@oleksandr-dziuban @Alec13355 This PR is most likely not needed, as react-native has a patch to make this work. I had trouble with it many times and turns out all you need is to correctly configure third-party dependencies. Have a look here facebook/react-native#20774
There is a workaround in the issue description, important steps are 4 and 5. Some people recommend to configure glog manually (via. node_modules/react-native/scripts/third-party/glog-x.x.x/configure) but you need to give it parameters as stated here: facebook/react-native#19839 (comment)
There was a problem hiding this comment.
@tomaskallup You shouldn't have to go in and manually configure a dependency of a dependency. That's the issue and that's why I made the PR.
There was a problem hiding this comment.
@Alec13355 This PR wouldn't prevent that tho... you would still need to go in node_modules and run the script to configure third-party dependencies, as it downloads glog and sets it up. I understand, that it's not convenient, but I think that's still issue on react-native part and should be solved there. As far as I know, Xcode should handle this configuration during build, but the new build system broke that part.
The issue I linked facebook/react-native#20774 is about this problem.
|
IMHO, there is no way for this to be merged for a change that fixes nothing and only breaks things. Shall we just close it? |
This allows react native to be built on Xcode10. See facebook/react-native#16106
Closes #382