Skip to content
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

Closed
efstathiosntonas opened this issue Oct 24, 2024 · 30 comments
Labels
impact: build-error Behaviour causing build failure Needs Attention platform: ios plugin: database Firebase Realtime Database plugin: firestore Firebase Cloud Firestore resolution: fixed A fix has been merged into main. type: bug New bug report

Comments

@efstathiosntonas
Copy link
Contributor

efstathiosntonas commented Oct 24, 2024

MAINTAINER EDIT - 👋

  1. this was an upstream issue caused by an unfortunate interaction with a new symbol react-native adds in New Architecture builds
  2. there is a PR merged upstream and released, v1.22.6 of leveldb-library will fix when installed on your system

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 is 1.22 and this has changed/fixed in version 1.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:

  - FirebaseDatabase (11.4.0):
    - FirebaseAppCheckInterop (~> 11.0)
    - FirebaseCore (~> 11.0)
    - FirebaseSharedSwift (~> 11.0)
    - GoogleUtilities/UserDefaults (~> 8.0)
    - leveldb-library (~> 1.22)`

ps. Sorry for skipping issue template, it seems unrelated to us here.
ps2. Why on earth the new arch build throws this error?? 🤔

screenshot:
Screenshot 2024-10-24 at 10 32 31

@efstathiosntonas

This comment was marked as resolved.

@russellwheatley russellwheatley added platform: ios blocked: firebase-sdk Pending a confirmed fix landing on the official native sdk's (iOS/Android). and removed Needs Attention labels Oct 24, 2024
@russellwheatley

This comment was marked as resolved.

@efstathiosntonas

This comment was marked as resolved.

@mikehardy
Copy link
Collaborator

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

@russellwheatley
Copy link
Member

russellwheatley commented Oct 24, 2024

@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 pod install.

@efstathiosntonas
Copy link
Contributor Author

efstathiosntonas commented Oct 24, 2024

It seems 1.23 is (backwards) broken? 🫨

google/leveldb#1101

@efstathiosntonas

This comment was marked as resolved.

@mikehardy

This comment was marked as resolved.

@matinzd
Copy link
Contributor

matinzd commented Oct 24, 2024

FirebaseFirestoreInternal is using leveldb-library v1.22 which is broken:

https://github.com/firebase/firebase-ios-sdk/blob/36da449b15619d301302faeba639c7893d576220/FirebaseFirestoreInternal.podspec#L110

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.

@efstathiosntonas

This comment was marked as resolved.

@matinzd

This comment was marked as resolved.

@mikehardy
Copy link
Collaborator

mikehardy commented Oct 24, 2024

The thing is, 1.23 tag has the same lines (they are just a little lower)

https://github.com/google/leveldb/blob/99b3c03b3284f5886f9ef9a4ef703d57373e61be/util/env_posix.cc#L839C1-L841

What I noticed when I first saw this is that the only lines that have a problem are the ones that are wrapped with:

#if !defined(NDEBUG)

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
2- perhaps we can cook up a Podfile postinstall script just for the react-native-firebase pods that use leveldb - until 1 above is done - that removes the NDEBUG symbol from the compile flags? Is that possible? Or perhaps a post-install script that applies the source patch for now like my rn76 make-demo project does?
3- people need to prefix the symbols they use at all times when things aren't namespaced (assuming my hunch here is correct - can be confirmed by looking at react-native for that symbol. These should be REACT_NATIVE_NDEBUG and LEVELDB_NDEBUG, in my humble opinion.

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:

facebook/react-native@1a0e174

@mikehardy

This comment was marked as resolved.

@efstathiosntonas

This comment was marked as resolved.

@simonbengtsson
Copy link
Contributor

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

@mikehardy
Copy link
Collaborator

@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

@mikehardy
Copy link
Collaborator

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 🙏

@mikehardy
Copy link
Collaborator

mikehardy commented Oct 26, 2024

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

@LA-Johan

This comment was marked as resolved.

@mikehardy

This comment was marked as resolved.

@mikehardy mikehardy changed the title [🐛] [email protected] No member named 'memory_order_relaxed' in 'std::memory_order'; [🐛] workaround below - [email protected] No member named 'memory_order_relaxed' in 'std::memory_order'; Oct 27, 2024
@mikehardy mikehardy pinned this issue Oct 27, 2024
@mikehardy mikehardy added plugin: database Firebase Realtime Database impact: build-error Behaviour causing build failure plugin: firestore Firebase Cloud Firestore and removed Needs Attention labels Oct 27, 2024
@mikehardy
Copy link
Collaborator

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 🤞

@raduciobanu22
Copy link

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!

@KevinSinSH

This comment was marked as off-topic.

@mikehardy

This comment was marked as off-topic.

@mikehardy
Copy link
Collaborator

mikehardy commented Oct 29, 2024

The leveldb-library cocoapod version with the fix was just released. I confirm it works.

you will want to remove your Podfile.lock and pod install again in order to get the fix

You should see

Installing leveldb-library (1.22.6)

...and that means you have the fix and this issue is closed

🚂 ✅


If you see some other errors like:

Undefined symbols for architecture arm64
❌ "absl::lts_20240116::CHexEscape(absl::lts_20240116::string_view)", referenced from:

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

@KevinSinSH

This comment was marked as off-topic.

@mikehardy

This comment was marked as off-topic.

@mikehardy mikehardy added resolution: fixed A fix has been merged into main. and removed blocked: firebase-sdk Pending a confirmed fix landing on the official native sdk's (iOS/Android). labels Oct 29, 2024
@efstathiosntonas
Copy link
Contributor Author

@mikehardy thank you for your efforts on this! It was fixed really fast 😎

@phaiminh
Copy link

@mikehardy so this error

Undefined symbols for architecture arm64
❌ "absl::lts_20240116::CHexEscape(absl::lts_20240116::string_view)", referenced from:

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.

@paulb777
Copy link
Contributor

paulb777 commented Oct 30, 2024

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 CLANG_CXX_LANGUAGE_STANDARD to gnu++14 in the abseil build target.

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]

@mikehardy mikehardy changed the title [🐛] workaround below - [email protected] No member named 'memory_order_relaxed' in 'std::memory_order'; [🐛] workaround below - New Arch build fail: No member named 'memory_order_relaxed' in 'std::memory_order'; Oct 30, 2024
@mikehardy mikehardy changed the title [🐛] workaround below - New Arch build fail: No member named 'memory_order_relaxed' in 'std::memory_order'; [🐛] fix below - New Arch build fail: No member named 'memory_order_relaxed' in 'std::memory_order'; Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact: build-error Behaviour causing build failure Needs Attention platform: ios plugin: database Firebase Realtime Database plugin: firestore Firebase Cloud Firestore resolution: fixed A fix has been merged into main. type: bug New bug report
Projects
None yet
Development

No branches or pull requests

10 participants