-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[🐛] fix below - New Arch build fail: No member named 'memory_order_relaxed' in 'std::memory_order'; #8082
Comments
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
I had the same problem!! It was the last thing left for me to qualify rn76 while it was in RV. Check my rn76 branch in my rnfbdemo repo and you can see a patch-hack that makes it work but I haven't figured a true fix yet |
@efstathiosntonas - I don't think SPM interferes with cocoapods. They are two separate things. It is more likely that cocoapods doesn't have a release for 1.23.0 which bears out when you look at the Specs: https://github.com/CocoaPods/Specs/tree/master/Specs/a/d/a/leveldb-library That last release was 1.22.5 which is what I am seeing when I run |
It seems 1.23 is (backwards) broken? 🫨 |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
FirebaseFirestoreInternal is using Leveldb 1.22 tag: https://github.com/google/leveldb/blob/1.22/util/env_posix.cc#L816-L818 But I am wondering why RN 76 is breaking it. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
The thing is, 1.23 tag has the same lines (they are just a little lower) What I noticed when I first saw this is that the only lines that have a problem are the ones that are wrapped with:
So I have a different hunch: I think those lines have been broken for a long long time and indeed they were added 6 years ago: https://github.com/google/leveldb/blame/1.22/util/env_posix.cc#L816-L818 So why are they breaking now? Combine that question with the idea the only 2 places that have problems are wrapped with a compiler "NDEBUG" guard and what I think is, that symbol is now being defined by react-native 0.76, and it is just a terribly unlucky coincidence that there was a symbol collision So, 3 things, I think: 1- leveldb has a bug, needs a PR + release + adoption of release inside firebase-ios-sdk / react-native-firebase Indeed, looks like react-native are the ones defining the NDEBUG symbol, and the intention I think was to define it for both architectures but I suspect there is an extra bug where that didn't happen, and it is only defined for Fabric for some reason, so it only triggers now: |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Made a cocoa pods post install script to apply the patch discussed above. Not sure this is the best way to approach patches like this, but seems to work for me at least. target 'App' do
# ...
post_install do |installer|
# Issue info: https://github.com/invertase/react-native-firebase/issues/8082
# Patch info: https://github.com/google/leveldb/pull/965
leveldb_posix_file = 'Pods/leveldb-library/util/env_posix.cc'
if File.exist?(leveldb_posix_file)
old_contents = File.read(leveldb_posix_file)
new_contents = old_contents.gsub(/std::memory_order::memory_order_relaxed/, "std::memory_order_relaxed")
File.chmod(0777, leveldb_posix_file)
File.open(leveldb_posix_file, "w") { |file| file.puts new_contents }
end
end
end |
@simonbengtsson fantastic - that looks like exactly the necessary post_install chunk for people to work around this as needed. The chmod is needed (in case anyone was curious), so the combo of chmod + replace is the minimal + correct solution. It is not great to use workarounds like this, sure, but this one should be effective for anyone that needs it. Thanks for posting |
I did a little more digging as I could not see cocoapods-related files in the google/leveldb repo, it turns out there is a fork living in firebase/leveldb and that is the source for our dependency when using firebase-ios-sdk I have proposed a pull request with the needed change from upstream google/leveldb into the firebase/leveldb repository here: firebase/leveldb#15 Perhaps @ncooke3 - who appears to maintain the repo currently - or @paulb777 who is also associated will take pity on us poor react-native souls dealing with build flags we're not in control of that are exposing these non-C++20 symbols 🙏 |
In order to use the merged result of the pull request before it is released, you may add this to your Podfile: pod 'leveldb-library', :git => 'https://github.com/firebase/leveldb.git', :commit => 'f6272399648046e31749d77e9c548712168afcf1' It is the exact same effect as the post_install block above - just alters those two lines vs the current released version. Either will work for anyone |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Upstream firebase-ios-sdk team has already reviewed and merged the necessary PR, great news and quite quick IMHO. They're testing overnight to make sure it meets all standards but it's on track for release. Next update here will hopefully be the final one as it resolves and closes 🤞 |
Came here because of the same issue in my Flutter app. Many thanks for your efforts! |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
The leveldb-library cocoapod version with the fix was just released. I confirm it works. you will want to remove your Podfile.lock and You should see
...and that means you have the fix and this issue is closed 🚂 ✅ If you see some other errors like:
those errors are unrelated - that is documented with a workaround here #8103 If you see those you have likely updated to cocoapods 1.16.0 (from 1.15.2) and/or xcodeproj 1.26.0 (from 1.25.0). You will want to use cocoapods 1.15.2 + xcodeproj 1.25.0 in order to build react-native successfully, reference facebook/react-native#47237 |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
@mikehardy thank you for your efforts on this! It was fixed really fast 😎 |
@mikehardy so this error Undefined symbols for architecture arm64 is not coming from leveldb-library. I got that error after update to 1.22.6. Do I have to update firebase sdk as well? Many thanks for your support. |
The undefined absl symbol is a problem with CocoaPods 1.16.1 and the Firestore abseil dependency. The workaround is to use CocoaPods 1.15.2 or add a CocoaPods post install script to change the Xcode option More at firebase/firebase-ios-sdk#13989 (comment) [Maintainer edit from @mikehardy 👋 - documented the new abseil build error and the workaround just now here #8103] |
MAINTAINER EDIT - 👋
To get the v1.22.6 of leveldb-library,
cd ios && rm -f Podfile.lock && pod repo update && pod install
and you should see v1.22.6 of leveldb-library in your Podfile.lock. Then you will build without issues.Hope that helps!
What follows is the original bug report:
After upgrading to 0.76 with Fabric enabled, for some reason
leveldb-library
env_posix.cc
file throws this error:No member named 'memory_order_relaxed' in 'std::memory_order'; did you mean 'std::memory_order_relaxed'?
@mikehardy I think this is an upstream issue, it seems that the
leveldb-library
version is1.22
and this has changed/fixed in version1.23
. I've applied the suggested changes from Xcode and the error went away. I'm opening the issue here so other devs are aware, shall we open an issue on upstream?Podfile.lock:
ps. Sorry for skipping issue template, it seems unrelated to us here.
ps2. Why on earth the new arch build throws this error?? 🤔
screenshot:
The text was updated successfully, but these errors were encountered: