facebook / flipper

A desktop debugging platform for mobile developers.
https://fbflipper.com/
MIT License
13.33k stars 953 forks source link

πŸ› MacCatalyst Does not Build on RN above 64 because of RCT-Folly (1/2 Fix Included Here!) #3117

Closed ZComwiz closed 2 years ago

ZComwiz commented 2 years ago

πŸ› Bug Report

FLIPPER FOLLY MAC CATALYST SUPPORT BROKE AFTER SWITCHING TO XCFRAMEWORK. HERE IS WHAT HAPPENED: There are two bugs when building RN 65+ targeted to MacCatalyst located in Flipper:

1. Time.h redefinition. There is a hack in here in Time.h in RCT which I was able to consistently fix by adding a TARGET_OS_MACCATALYST flag on line 31. See my comment on the quick, robust, fix here.

2. DoubleConversion Not Found when building: When Flipper/Folly switched to xcFrameworks from the precompiled .a file, it seems they did not include a slice for MacCatalyst. I will include some helpful references here which the team at Flipper who is more familiar with the archiving process of the xcframework can use to quickly rectify this support regression:

a) Mike Hardy's awesome script which may help compile the DoubleConversion Library properly

b) A simple guide to building MacCatalyst slices when using an xcFrameworks

To Reproduce

Try building any react-native on any version 65 or greater with Mac as the target destination (MacCatalyst). You can do that by selecting iPad, then Mac in the General settings of the template HelloWorld Xcode Project. Switch Build Target to Mac at the top.

Recommend using the template in this branch of a release candidate of 67 which adds some robust fixes that will be integrated soon (but you can try on the latest RN 66 build).

Environment

Intel and M1 Macs alike: targeting MacCatalsyt. XCode Version 13.0 (13A233). Any version of React Native above 64, but you can use 66 or the release candidate mention above.

mikehardy commented 2 years ago

Wow, that took a while to find. It's here and needs attention from @priteshrnandgaonkar

https://github.com/priteshrnandgaonkar/double-conversion/blob/master/archive-xcframework.sh

Pritesh, you need to include a macCatalyst slice, easy to build but needs a very very careful re-arrangement of built artifacts after build, before packaging:


# Do not be tempted to use "-sdk macosx" instead of the "destination" argument. It switches you to AppKit from UIKit
# https://developer.apple.com/forums/thread/120542?answerId=374124022#374124022
echo "Archiving for Mac Catalyst"
xcodebuild archive -workspace "./${FRAMEWORK_NAME}.xcworkspace" -scheme ${BUILD_SCHEME} -configuration Release \
  -destination='platform=macOS,arch=x86_64,variant=Mac Catalyst' -archivePath "${CATALYST_ARCHIVE_PATH}" \
  SKIP_INSTALL=NO BUILD_LIBRARY_FOR_DISTRIBUTION=YES | xcpretty -k

echo "Packaging archives into ${FRAMEWORK_NAME}.xcframework bundle"
xcodebuild -create-xcframework \
  -framework "${SIMULATOR_ARCHIVE_PATH}/Products/Library/Frameworks/${FRAMEWORK_NAME}.framework" \
  -framework "${IOS_DEVICE_ARCHIVE_PATH}/Products/Library/Frameworks/${FRAMEWORK_NAME}.framework" \
  -framework "${CATALYST_ARCHIVE_PATH}/Products/Library/Frameworks/${FRAMEWORK_NAME}.framework" \
  -output "${FRAMEWORK_PATH}" | xcpretty

# Catalyst framework directory structure flattening step 1:
# - Copy the framework in a specific way: de-referencing symbolic links on purpose
echo "Installing framework into packages/react-native submodule"
rm -rf "${OUTPUT_FOLDER}/${FRAMEWORK_NAME}.xcframework" && \
  cp -rp "${FRAMEWORK_PATH}" "${OUTPUT_FOLDER}"

# Catalyst framework directory structure flattening step 2:
# - Remove the catalyst framework "Versions" directory structure after symbolic link de-reference
rm -rf "${OUTPUT_FOLDER}/${FRAMEWORK_NAME}.xcframework/ios-arm64_x86_64-maccatalyst/${FRAMEWORK_NAME}.framework/Versions"
lblasa commented 2 years ago

@mikehardy thanks for reporting this and finding the problem. I will provide a fix and update once ready.

mikehardy commented 2 years ago

Fantastic! As someone that does not generally like workarounds (I love to PR upstream and just fix things...) I was surprised by the need to do that catalyst slice repack but please understand I tested that super carefully back and forth and that strategy was the only way I successfully built for catalyst in a published pod, painful reference: https://github.com/invertase/notifee/pull/55 https://github.com/invertase/react-native-notifee/issues/162#issuecomment-754968049

(that was from when Notifee was proprietary, thus this was needed, it's open source now but this was so hard to figure out...)

ZComwiz commented 2 years ago

@mikehardy I am so glad we got this the attention it needs! I am giving you a virtual high five πŸ™Œ

mikehardy commented 2 years ago

In classic "hey, while you're in there..." fashion I'll mention that the tvOS and watchOS folks will be after you next. I had work in that area but didn't have time to merge it in Notifee. The natural extensions on the pbxproj and build framework script did work as expected though, I had a valid xcframework with it

https://github.com/invertase/notifee/pull/56/files#diff-5bdb27f5710cdebde3c248e0e983ee16d5e5c1173cfe0cc8b31011f56505d5f1 https://github.com/invertase/notifee/pull/56/files#diff-0dc5ff7bb16a87c5622e00d8564b828cf33258c54005107f0916460b7e3ae601

It's still on my todo list to actually make a github repo and blog about this so anyone doing xcframeworks just has a script that works and we can all maintain just one...

ZComwiz commented 2 years ago

@lblasa β€” do you have any progress update on this?

Arkkeeper commented 2 years ago

It seems that the problem is deeper than just 2 fixes. I've forked Flipper-DoubleConversion repo and patched xcframework creation script (https://github.com/Arkkeeper/double-conversion). I've also modified react-native/scripts/react_native_pods.rb in order to fetch Flipper-DoubleConversion from that fork, so now it includes a necessary slice for Mac Catalyst. But that's not enough for successful builds. Now I keep getting errors during linking stage about duplicate symbols between Flipper-Folly and RCT-Folly. We need a hint on how this duplication is being avoided when building for iOS or iOS Simulator.

duplicates
exotexot commented 2 years ago

Ah I was sort of hoping that this issue was fixed with the 0.67 release. So it still persists? :/ Hm, then my euphoria for 0.67 is fading :(

Arkkeeper commented 2 years ago

Nope, I was too pessimistic :) Everything seems to be ok after fixing Flipper-DoubleConversion slices.

"Duplicate symbols" error is caused by absence of DEAD_CODE_STRIPPING = YES in project.pbxproj, it was discussed here https://github.com/facebook/react-native/issues/32333#issuecomment-944159768, but there is no necessary fix in RN 0.67. After adding DEAD_CODE_STRIPPING = YES my app compiles successfully.

But... the happy end is still far, because Flipper still can't connect to Catalyst app and debug it. First I thought that it's related to idb, but that's not true.

flipper
mikehardy commented 2 years ago

Hey @Arkkeeper - I don't see a PR for your maccatalyst xcframework fix - you should PR that! It looked good to me. I had to flatten the slice from it's structure when I did the same for Notifee but there is nothing like proof, and if you have it definitely working then you should PR and get those slices published for next release hopefully

Arkkeeper commented 2 years ago

Our team is doing some additional research now, if everything is ok, I'll certainly publish that patch as a PR tomorrow.

ZComwiz commented 2 years ago

@priteshrnandgaonkar β€” looks like @Arkkeeper made a PR :)

@mikehardy β€”what would be the next step(s) to see that added into RN 0.67.2? Can we make a fast patch release of RN that includes this fix?

mikehardy commented 2 years ago

If I understand correctly

First everything needs to be merged on main branches and working fir flipper

Then flipper does a release and you test with a local change to use that version and that should be enough to unblock

Then react native gets a PR to bump flipper version

Then you use the commitly release from that react native merge, then it's in the next react native version

I think since you can specify flipper version it doesn't need to wait for a react native release

ZComwiz commented 2 years ago

@mikehardy, it looks like @priteshrnandgaonkar is MIA. As the most central member to the RN community, do you want to host a new Flipper Double Conversion that incorporates @Arkkeeper's changes and offer a PR to the Flipper project itself? Let's not sleep on this now that it appears fixed.

kelset commented 2 years ago

Can anyone give me a quick overview of the status of the open PRs/fixes to solve this? I feel a bit lost atm. Are the PRs to get this address open/merged or some things still need to be worked on?

mikehardy commented 2 years ago

@kelset this is really thorny unfortunately.

The proposed solution just above of me forking + publishing a new Flipper-DoubleConversion (and Flipper-Glog) based on @priteshrnandgaonkar work but including the work here to add maccatalyst slices to the vendored xcframework is a bad idea.

Using more precise technical language: it would be spending dev time on a compile-time optimization that was made in the form of a 3rd-party personal fork of a major library by making another 3rd-party personal fork of the major library and having this major project then switch dependency from one personal fork to another. That's a bad idea right?

Here's the technical analysis of where we are

So React Native needs Flipper, and Flipper needs the Flipper-DoubleConversion library. It pulls it in here:

https://github.com/facebook/flipper/blob/fbd32129944d2e09da80aa68c11d20ff3cb7cbd7/iOS/Podspecs/Flipper-Folly.podspec#L18

which is just a reference to a specific version of a specific pod that is out-of-tree and live on cocoapods.org and happens to be named "Flipper-Something" but is not actually controlled by facebook/meta or this repo: https://cocoapods.org/pods/Flipper-DoubleConversion

Which is/was just a one-time publish of a personal repo from @priteshrnandgaonkar : https://github.com/CocoaPods/Specs/blob/master/Specs/3/3/c/Flipper-DoubleConversion/3.1.7/Flipper-DoubleConversion.podspec.json

That personal repo is a point in time / unupdated fork of the google/doubleconversion project, but with a pre-compiled xcframework included, you can see the work to do so in commit history https://github.com/priteshrnandgaonkar/double-conversion/commits/v3.1.7

(note, Flipper-Glog is doing something similar, pointing directly here: https://github.com/CocoaPods/Specs/blob/master/Specs/2/7/2/Flipper-Glog/0.3.9/Flipper-Glog.podspec.json)

...which is named Flipper-Glog but is just a one-time publish of an unupdated fork from Pritesh that has a pre-compiled xcframework.

Here is quick list of pain points

So there are a few things going on here:

here's what I think needs to happen

1- the compile optimizations introduced in #2153 by including pre-compiled DoubleConversion and Glog should be reverted and the local build/podspec stuff here related to those two libraries needs to be brought up to date with the whatever the local peer pods from 3rd party native libraries are doing so it's in tune again

That may be enough to solve this issue by itself now, Maccatalyst will likely work at that point

Someone needs to do this, I do not have time. Clone Flipper repo, look at what #2153 changed, manually revert the change so doubleconversion and glog are built again (using the same versions/tags in use now though) and see if Flipper works. PR that. Do a follow-on PR that brings them up to current versions if possible. See if maccatalyst builds for react-native if you depend on that new local flipper, should work?

2- if speed optimizations are desired, in CI here the workflows should enable and use ccache-action (react-native-firebase does this to good effect, as do all my work projects)

3- an audit should be made of dependencies that are spidering out to Pritesh's personal repos, and they should be internalized. A firm policy should be made to never do that again as it makes maintaining the web of dependencies here and moving the project forward impossible when dependencies spider out to personal repos with single maintainers

There quite a few at the moment - it's a bit of a rabbit hole but not quite the abyss yet:

mike@osxvm:~/work/react-random/flipper (main) % grep -r priteshrnandgaonkar *
android/third-party/native.gradle:    src 'https://github.com/priteshrnandgaonkar/rsocket-cpp/archive/0.10.7.tar.gz'
iOS/Podspecs/Flipper-Boost-iOSX.podspec:    s.homepage     = "https://github.com/priteshrnandgaonkar/boost-iosx"
iOS/Podspecs/Flipper-Boost-iOSX.podspec:    s.source       = { :git => "https://github.com/priteshrnandgaonkar/Flipper-Boost-iOSX.git", :tag => "1.76.0.1.11" }
iOS/Podspecs/Flipper-Fmt.podspec:    spec.source   = { :git => "https://github.com/priteshrnandgaonkar/fmt.git",
iOS/Podspecs/Flipper-PeerTalk.podspec:    spec.source   = { :git => "https://github.com/priteshrnandgaonkar/peertalk.git",
lblasa commented 2 years ago

Sorry for the late reply, I will be looking into this very shortly and hopefully will have an update soon.

lblasa commented 2 years ago

First of all, thanks everybody for the discussion!

I've just released new versions of Double-Conversion and Glog which catch up with the upstream repositories and contain slices for Mac Catalyst. https://cocoapods.org/pods/Flipper-Glog https://cocoapods.org/pods/Flipper-DoubleConversion

I'm now the owner of all Flipper-* pods so I should be able to make any necessary updates and adjustments in the future.

Unfortunately, right now, I don't have the capacity to make our dependency consumption better. Having said that, valid points were said and we will definitely look into making things better in future releases.

mikehardy commented 2 years ago

I am all for incremental progress :-), changing dependency consumption is not the easiest, no, but at least now we're all aware. Having the catalyst slice in the new pods will be great - it appears to me that the Flipper-Glog and Flipper-DoubleConversion pods are both specified inside Flipper-Folly without version constraints, so simply by publishing them, catalyst builds may start working for people if they pod update --repo-update ? Just a guess - I need to test, but people may be unblocked right now, by that publish action on cocoapods...

Edited: just tested, I see the new pods but pod update does not access them and rm -f Podfile.lock && pod install does not either, so the versions are pinned somewhere.

So if my tests are valid and I understand correctly, I think there will need to be a Flipper release process in order to ingest this work before catalyst will work for people.

pod outdated output after attempting tests - new pods visible, but they are pinned

- Flipper 0.125.0 -> 0.125.0 (latest version 0.135.0)
- Flipper-DoubleConversion 3.1.7 -> 3.1.7 (latest version 3.2.0)
- Flipper-Glog 0.3.9 -> 0.3.9 (latest version 0.5.0.3)
ZComwiz commented 2 years ago

@mikehardy @lblasa @kelset β€” hey all. We are so close on this issue. Let's not drop the ball on it now.

lblasa commented 2 years ago

I’ll check on this early next week, sorry about the delay!

lblasa commented 2 years ago

I found we had pinned some versions in our ReactNativeExample and this was fixed here: https://github.com/facebook/flipper/commit/017bb77aafc9b10cea1bd5450d7a7c1575bb2883

After the change, I had a look at what versions were being pulled for our iOS Sample and ReactNativeExample and they all seem to point to the right versions.

mikehardy commented 2 years ago

Ah ha

The difference is that your use_flipper line in flipper's react-native example is specifying versions. So you were able to access them. Great that works!

So, for anyone suffering from this problem right now you may specify the same versions in that commit and you will have a working catalyst build.

However in the empty / default use_flipper! case where versions are not specified manually - used by majority of react-native users - this is not working. Why not? It took me a little bit, but I found the version pin!

https://github.com/facebook/react-native/blob/325be429fd33bd79234bb4c79523a0b476121645/scripts/react_native_pods.rb#L138-L146

Those versions need a change - @lblasa what versions should we put in there? Last person to do it was @cortinico in https://github.com/facebook/react-native/pull/32923 and it appears we need to pick versions carefully. Should we just update the ones you touched?

Assuming answer is yes - update only the ones updated here - I've posted a PR that attempts to do it, we'll see how CI likes it...

https://github.com/facebook/react-native/pull/33406

lblasa commented 2 years ago

@mikehardy yes, correct. @cortinico was correct too, there's some issues with newer versions of Flipper and RN but that shouldn't have any impact with these dependencies in question. I've seen your PR and looks sensible :)

mikehardy commented 2 years ago

@ZComwiz / @Arkkeeper - if you have a moment, it would be really useful if you followed the style of the commit Lorenzo linked where the use_flipper line specifies these new versions for DoubleVersion and Glog. If you do that, you should get a successful catalyst build, and you would help satisfy the necessary test matrix to approve the PR that fixes this for everyone, allowing this issue to close

Pajn commented 2 years ago

After specifying

    use_flipper!({ 
      'Flipper' => '0.138.0',
      'Flipper-Folly' => '2.6.10',
      'Flipper-DoubleConversion' => '3.2.0',
      'Flipper-Glog' => '0.5.0.3',
      'OpenSSL-Universal' => '1.1.1100',
    })

and DEAD_CODE_STRIPPING = YES; I can get it to both build and link, but while running it still can't find double-conversion:

dyld[10257]: Library not loaded: @rpath/double-conversion.framework/Versions/A/double-conversion
  Referenced from: /Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/MacOS/Project
  Reason: tried: '/Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/double-conversion.framework/Versions/A/double-conversion' (no such file), '/usr/lib/swift/double-conversion.framework/Versions/A/double-conversion' (no such file), '/usr/lib/swift/double-conversion.framework/Versions/A/double-conversion' (no such file), '/Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/MacOS/../Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file), '/usr/lib/swift/double-conversion.framework/Versions/A/double-conversion' (no such file), '/Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/MacOS/Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file), '/Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/MacOS/Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file), '/Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/MacOS/Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file), '/usr/lib/swift/double-conversion.framework/Versions/A/double-conversion' (no such file), '/usr/lib/swift/double-conversion.framework/Versions/A/double-conversion' (no such file), '/Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/MacOS/../Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file), '/usr/lib/swift/double-conversion.framework/Versions/A/double-conversion' (no such file), '/Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/MacOS/Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file), '/Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/MacOS/Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file), '/Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/MacOS/Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file), '/System/Library/Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file)
(lldb) 

It's looking at /Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/MacOS/../Frameworks/double-conversion.framework/Versions/A/double-conversion however it's actually located at /Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/MacOS/../Frameworks/double-conversion.framework/double-conversion, what could be the reason for that?

mikehardy commented 2 years ago

Because for some reason catalyst slices need to be "flattened" after generation to work correctly. Why? Zero idea. But I noted that explicitly here: https://github.com/facebook/flipper/issues/3117#issuecomment-983662943

Deeper links:

@lblasa I would guess you need to try generating the slices in this way, with a flat directory structure, and publish again with a new point release, then we after @Pajn retests as a catalyst build with the new point release and confirms it's working, we do the version bumps here and in react-native ?

mikehardy commented 2 years ago

To be most direct - there is something actionable but needs testing @Pajn - can you please :pray: check the command line I posted in the linked issue 162 from archived notifee repo, to manually flatten the directories on your local filesystem post-install, and see if it builds + runs? That will prove the strategy prior to spending effort on a re-publish and all that

It's the cp -rv Versions/A/* . bit from inside the .xcframework directories that does the "flattening" and should make the binaries available to cocoapods/xcode

Pajn commented 2 years ago

@mikehardy I'm unsure what to "flatten" Flutter-DoubleConversion is already flattened, however I tried to "unflatten" it by creating Versions/A and copying double-coversion into it. It causes the library to be found but failing due to signing:

./Frameworks/double-conversion.framework/Versions/A/double-conversion' (code signature in <FDC7F41B-35FE-3EDA-99E1-23954B6AAAAB> '/Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/Frameworks/double-conversion.framework/Versions/A/double-conversion' not valid for use in process: Trying to load an unsigned library),
ZComwiz commented 2 years ago

Got pretty close but stuck here:

duplicate symbol 'folly::detail::str_to_bool(folly::Range<char const*>*)' in:
    /Users/user/Library/Developer/Xcode/DerivedData/App/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(Conv.o)
    /Users/user/Library/Developer/Xcode/DerivedData/App/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(Conv.o)
duplicate symbol 'folly::makeConversionError(folly::ConversionCode, folly::Range<char const*>)' in:
    /Users/user/Library/Developer/Xcode/DerivedData/App/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(Conv.o)
    /Users/user/Library/Developer/Xcode/DerivedData/App/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(Conv.o)
mikehardy commented 2 years ago

Fascinating. I did not understand why Notifee (in it's previous closed-source incarnation, where I had to generate an .xcframework) needed flattening, so it is a really curious result that for this build the xcframework needed unflattening. Really odd. There's something to understand here, I just don't know what, perhaps there is some kind of manifest/descriptor in the xcframework that can be generated differently or altered - I haven't inspected.

How did you get around the signing thing? Finally, it appears that the folly thing would be unrelated? that's RCT's version of folly colliding with Flipper's version of Folly somehow. Deleting ~/Library/Developer/Xcode/DerivedData (or wherever your Xcode build objects are going) might help as that could be some partial build artifact hanging around? That's a hunch, may work, may not

ZComwiz commented 2 years ago

@mikehardy β€” tried deleting DerivedData, deintegrating pods and reinstalling pods. I never had an issue with code signing on Mac (you just go into the pods file and give your signing credentials after a fresh pod install).

My podfile looks like this:


require_relative '../node_modules/react-native/scripts/react_native_pods'
require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules'

platform :ios, '11.0'
# used for automatic bumping
flipperkit_version = '0.138.0'

target 'App' do
    config = use_native_modules!

    use_react_native!(
        :path => config[:reactNativePath],
        # to enable hermes on iOS, change `false` to `true` and then install pods
        :hermes_enabled => false
)   

    # Pods for App ...

    target 'AppTests' do
        inherit! :complete
        # Pods for testing
    end

    # MAC CATALYST FIX:
    use_flipper!({ 'Flipper' => flipperkit_version, 'Flipper-Folly' => '2.6.10', 'Flipper-DoubleConversion' => '3.2.0', 'Flipper-Glog' => '0.5.0.3', 'Flipper-PeerTalk' => '0.0.4', 'OpenSSL-Universal' => '1.1.1100' })

    post_install do |installer|
        react_native_post_install(installer)
        __apply_Xcode_12_5_M1_post_install_workaround(installer)
    end
end

and when Hermes turned on we get the following errors on Mac Catalyst (works on iOS):

duplicate symbol 'folly::detail::str_to_bool(folly::Range<char const*>*)' in:
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(Conv.o)
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(Conv.o)
duplicate symbol 'folly::makeConversionError(folly::ConversionCode, folly::Range<char const*>)' in:
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(Conv.o)
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(Conv.o)
duplicate symbol 'folly::exception_wrapper::from_exception_ptr(std::exception_ptr&&)' in:
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(ExceptionWrapper.o)
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(ExceptionWrapper.o)
duplicate symbol 'folly::exception_wrapper::from_exception_ptr(std::exception_ptr const&)' in:
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(ExceptionWrapper.o)
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(ExceptionWrapper.o)
duplicate symbol 'folly::exception_wrapper::onNoExceptionError(char const*)' in:
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(ExceptionWrapper.o)
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(ExceptionWrapper.o)
duplicate symbol 'folly::exceptionStr(folly::exception_wrapper const&)' in:
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(ExceptionWrapper.o)
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(ExceptionWrapper.o)
duplicate symbol 'folly::exception_wrapper::uninit_' in:
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(ExceptionWrapper.o)
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(ExceptionWrapper.o)
duplicate symbol 'folly::exception_wrapper::ExceptionPtr::ops_' in:
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(ExceptionWrapper.o)
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(ExceptionWrapper.o)
duplicate symbol 'folly::exception_wrapper::SharedPtr::ops_' in:
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(ExceptionWrapper.o)
    /Users/USER/Library/Developer/Xcode/DerivedData/APP/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(ExceptionWrapper.o)
ld: 9 duplicate symbols for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

However! removing the line: use_flipper!({ 'Flipper' => flipperkit_version, 'Flipper-Folly' => '2.6.10', 'Flipper-DoubleConversion' => '3.2.0', 'Flipper-Glog' => '0.5.0.3', 'Flipper-PeerTalk' => '0.0.4', 'OpenSSL-Universal' => '1.1.1100' }) and references to Flipper in AppDelegate.m allows the app to build on RN 0.67.3 to MacCatalyst with Hermes turned on! Amazing Progress!!!

Pajn commented 2 years ago

@ZComwiz you can fix the duplicate symbol errors by explicitly adding DEAD_CODE_STRIPPING = YES; xcode will argue not having it is the same as yes but that's apparently not correct.

Interesting that you got it to work without Flipper. Will try that, that could be a temporary solution to allow us to upgrade.

@mikehardy I haven't got around the signing problem for Flipper-DoubleConcersion yet. Other pods haven't been any problem with.

mikehardy commented 2 years ago

I had to do this for signing resources at least, but your error with regard to codesigning looks like a step past mine:

    installer.pods_project.targets.each do |target|
      if target.respond_to?(:product_type) and target.product_type == "com.apple.product-type.bundle"
        target.build_configurations.each do |config|
          config.build_settings["CODE_SIGN_IDENTITY[sdk=macosx*]"] = "-"
        end
      end
    end

...I haven't gotten it to build with Flipper on yet because I'm currently stuck here:


⚠️ ld: Could not find or use auto-linked library 'swiftUIKit'
❌ Undefined symbols for architecture x86_64
❌   "__swift_FORCE_LOAD_$_swiftUIKit", referenced from:
⚠️       __swift_FORCE_LOAD_$_swiftUIKit_$_YogaKit in libYogaKit.a(YGLayoutExtensions.o)
❌ ld: symbol(s) not found for architecture x86_64
❌ clang: error: linker command failed with exit code 1 (use -v to see invocation)

YogaKit comes in as one of the Pods associated with Flipper, as I attempt to use this in my Podfile:

  use_flipper!({
    'Flipper' => '0.138.0',
    'Flipper-Folly' => '2.6.10',
    'Flipper-DoubleConversion' => '3.2.0',
    'Flipper-Glog' => '0.5.0.3',
   'OpenSSL-Universal' => '1.1.1100',
  })

I can successfully build and run with hermes on as well, as long as I disable Flipper.

So I spent some time getting a reproduction script ready for this but I haven't progressed as far as you all yet since I haven't even gotten to the DoubleConversion xcframework slice structural / codesigning issue

mikehardy commented 2 years ago

@Pajn This looked interesting https://github.com/dart-lang/sdk/issues/38314#issuecomment-577742824 You might try adding this to your entitlements file:

        <key>com.apple.security.cs.disable-library-validation</key>
        <true/>

...perhaps that will let you load the double conversion library. Should it be required? Unsure, perhaps in the time between when this worked (react-native 0.64) and now, the Flipper stuff has enabled signing or react-native has enabled signing for catalyst apps, or Xcode has started doing it by default or who knows, and then perhaps the DoubleConversion pod that was published either has no key or a different key than the other libraries if they have one, etc. It looks a little like a mess, and my prescription (turn off the checks for signing at the dylib layer) are a security regression but might enable the testing continue

Pajn commented 2 years ago

Removing flipper (use_flipper, references in AppDelegate.m and uninstalling react-native-flipper npm module) makes it run in catalyst for me too. I'm also using hermes btw.

Haven't seen your swiftUIKit problem so can't unfortunately say what in our setup fixes/workaround it :/ But these are the YogaKit versions that gets pulled in:

  - Yoga (1.14.0)
  - YogaKit (1.18.1):
    - Yoga (~> 1.14)

@mikehardy with that entitlement I get this crash on startup instead:

dyld[23098]: Library not loaded: @rpath/OpenSSL.framework/Versions/A/OpenSSL
  Referenced from: /Users/rasmus/Library/Developer/Xcode/DerivedData/Project/Build/Products/Debug-maccatalyst/Project.app/Contents/MacOS/Project
  Reason: tried: '/usr/lib/swift/OpenSSL.framework/Versions/A/OpenSSL' (no such file), '/usr/lib/swift/OpenSSL.framework/Versions/A/OpenSSL' (no such file), '/usr/lib/swift/OpenSSL.framework/Versions/A/OpenSSL' (no such file), '/usr/lib/swift/OpenSSL.framework/Versions/A/OpenSSL' (no such file), '/usr/lib/swift/OpenSSL.framework/Versions/A/OpenSSL' (no such file), '/usr/lib/swift/OpenSSL.framework/Versions/A/OpenSSL' (no such file), '/System/Library/Frameworks/OpenSSL.framework/Versions/A/OpenSSL' (no such file), (security policy does not allow @ path expansion)
lblasa commented 2 years ago

@mikehardy the error:

⚠️ ld: Could not find or use auto-linked library 'swiftUIKit' ❌ Undefined symbols for architecture x86_64 ❌ "__swift_FORCE_LOAD_$_swiftUIKit", referenced from: ⚠️ __swift_FORCE_LOAD_$_swiftUIKit_$_YogaKit in libYogaKit.a(YGLayoutExtensions.o) ❌ ld: symbol(s) not found for architecture x86_64 ❌ clang: error: linker command failed with exit code 1 (use -v to see invocation)

There is a library in Xcode that needs to be added to the Link Binary with Libraries phase:

I mention both because I've seen some undefined symbols from WebKit, but the error you're showing is from UIKit.

I believe that should address that issue.

mikehardy commented 2 years ago

@lblasa yeah I have a manual link command that I extracted from the failed command and altered slightly, and I can just squeeze (manually) by that error - the alteration was a pathing issue on linker command line, so I should be able to automate it somehow via post-install hook in Podfile and tweaking something about library paths or something. But then I get to duplicate symbols here:

duplicate symbol 'folly::detail::str_to_bool(folly::Range<char const*>*)' in:
    /Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-dwlryiowautyphccrrwxfbsrbwxk/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(Conv.o)
    /Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-dwlryiowautyphccrrwxfbsrbwxk/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(Conv.o)
duplicate symbol 'folly::makeConversionError(folly::ConversionCode, folly::Range<char const*>)' in:
    /Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-dwlryiowautyphccrrwxfbsrbwxk/Build/Products/Debug-maccatalyst/Flipper-Folly/libFlipper-Folly.a(Conv.o)
    /Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-dwlryiowautyphccrrwxfbsrbwxk/Build/Products/Debug-maccatalyst/RCT-Folly/libRCT-Folly.a(Conv.o)
ld: 2 duplicate symbols for architecture x86_64

...which has been described as fixable by adding DEAD_CODE_STRIPPING = YES to the pbxproj but I did not achieve a positive result either via altering DEAD_CODE_STRIPPING via the Xcode UI or altering it via the python pbxproj package (and verifying in the UI that it was on). I'm not certain I'm doing this step correctly and have valid results testing whether it's actually on or not though, this is the vanguard of my effort at the moment

My test effort is here https://github.com/mikehardy/rnfbdemo/blob/catalyst-build/make-demo.sh - I'll remove the firebase stuff from it so that it's a bit more focused on the catalyst effort and maybe we can use it for share-able reproduction

mikehardy commented 2 years ago

Also, since it's scripted, I was going to try go back to react-native 0.64 since people reported that succeeds, as a control on the whole experiment

mikehardy commented 2 years ago

I think it is all working with the two Pods that @lblasa published, at least on an x86_64 mac (the arm64 slices exist but I haven't run them on my M1 yet, assume they work unless I report back otherwise)

The script delivers a verifiable running catalyst app every time.

Full script right here, it does not require any react-native-firebase now, just a lot of clock time as it works through these builds:

Final changes from a default template init to run catalyst + hermes + flipper are:

After that, it'll run while targeting your machine's name: npx react-native run-ios --device "$(scutil --get LocalHostName)"

mikehardy commented 2 years ago

@Pajn I finally got a chance to try this on an M1 machine, I have it fully scripted and working on x86_64, but on M1 this morning, I got the same error you received. Are you on an M1 machine?

dyld[88732]: Library not loaded: @rpath/double-conversion.framework/Versions/A/double-conversion
  Referenced from: /Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-dwlryiowautyphccrrwxfbsrbwxk/Build/Products/Debug-maccatalyst/rnfbdemo.app/Contents/MacOS/rnfbdemo
  Reason: tried: '/Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-dwlryiowautyphccrrwxfbsrbwxk/Build/Products/Debug-maccatalyst/double-conversion.framework/Versions/A/double-conversion' (no such file), '/usr/lib/swift/double-conversion.framework/Versions/A/double-conversion' (no such file), '/Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-dwlryiowautyphccrrwxfbsrbwxk/Build/Products/Debug-maccatalyst/rnfbdemo.app/Contents/MacOS/../Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file), '/Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-dwlryiowautyphccrrwxfbsrbwxk/Build/Products/Debug-maccatalyst/rnfbdemo.app/Contents/MacOS/Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file), '/usr/lib/swift/double-conversion.framework/Versions/A/double-conversion' (no such file), '/Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-dwlryiowautyphccrrwxfbsrbwxk/Build/Products/Debug-maccatalyst/rnfbdemo.app/Contents/MacOS/../Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file), '/Users/mike/Library/Developer/Xcode/DerivedData/rnfbdemo-dwlryiowautyphccrrwxfbsrbwxk/Build/Products/Debug-maccatalyst/rnfbdemo.app/Contents/MacOS/Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file), '/System/Library/Frameworks/double-conversion.framework/Versions/A/double-conversion' (no such file)
(lldb) 
Pajn commented 2 years ago

@mikehardy yes, I am. Forgot to say that, sorry;

mikehardy commented 2 years ago

Okay - for anyone else following along, @Pajn and I have gotten to this point on an M1 machine for catalyst (it works for x86_64):

And that is where I am stuck right now 🀷 - I hit the end of my timebox on this for a while so I may not be able to look again for some time.

macCatalyst +flipper works for x86_64, stuck on arm64 until someone can bust through the block here.

cyberdude commented 2 years ago

Do we have an update on this ?

mikehardy commented 2 years ago

@cyberdude Please read this and reflect: https://hackernoon.com/i-thought-i-understood-open-source-i-was-wrong-cf54999c097b

There are no secret updates, I can say that. Catalyst works great everywhere now except M1 mac, but you could be the person to move that forward based on comments above in this issue

Arkkeeper commented 2 years ago

@mikehardy Finally I've got some time for experiments. I've edited archive-xcframework.sh inside Flipper-Glog and Flipper-DoubleConversion in this manner:

archive-xcframework

And I can confirm that now everything is ok on my M1 Mac with Catalyst (even with the new architecture enabled on RN 0.68.1).

In order to try this on Intel Mac, one can patch node_modules/react-native/scripts/react_native_pods.rb and replace corresponding lines inside use_flipper! to:

pod 'Flipper-DoubleConversion', :git => 'https://github.com/Arkkeeper/double-conversion.git', :branch => 'flipper-v3.2.0', :configurations => configurations pod 'Flipper-Glog', :git => 'https://github.com/Arkkeeper/glog.git', :branch => 'flipper-v0.5.0.1', :configurations => configurations

Then delete ios/Pods and ios/Podfile.lock and do pod install or RCT_NEW_ARCH_ENABLED=1 pod install.

mikehardy commented 2 years ago

@Arkkeeper excellent! I think you could propose PRs to these two repositories as they are the source of published pods currently and then hopefully @lblasa could help integrate + release?

https://github.com/lblasa/double-conversion https://github.com/lblasa/glog

Arkkeeper commented 2 years ago

I have tested new xcframework on Intel Mac by myself, it works as expected. My PRs are published to @lblasa forks.

lblasa commented 2 years ago

Hi @Arkkeeper thanks for the PRs and sorry about the late reply. I will be integrating these changes very soon and will push a new pod version for both dependencies :)

lblasa commented 2 years ago

Just a very quick follow-up. I've published new versions for both libraries based of @Arkkeeper PR's.

@Arkkeeper I think is good for a re-test. If it does work, then I believe we could close this long-standing issue.