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

refactor!: update to flutter rust bridge v2 #332

Merged
merged 13 commits into from
Jul 31, 2024

Conversation

mcmah309
Copy link
Contributor

closes #331

@GregoryConrad
Copy link
Owner

GregoryConrad commented Jul 29, 2024

This is a huge help! TYSM!

It'll take awhile to review all of this (especially since I didn't look in-depth at the FRB v2 changes), so please bear with me while I take a look!

Edit: wasn't as bad as the diff made it out to be--mostly just code gen file updates 😄

GregoryConrad
GregoryConrad previously approved these changes Jul 29, 2024
Copy link
Owner

@GregoryConrad GregoryConrad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you have so far is good!

If it's ok with you, can I use this PR to finish up some misc things (like iOS/macOS/CI/CD)? Then I can merge it in once I'm done with that.

@GregoryConrad GregoryConrad changed the title Update flutter rust bridge refactor!: update to FRB v2 Jul 29, 2024
@GregoryConrad GregoryConrad changed the title refactor!: update to FRB v2 refactor!: update to flutter rust bridge v2 Jul 29, 2024
@mcmah309
Copy link
Contributor Author

Yeah that works!

@GregoryConrad
Copy link
Owner

I need to go to bed so I'll leave some notes here on where I left off:

// flutter_interface.dart
  /// Creates a [MimirInstance] from the given path for Flutter
  Future<MimirInstance> getInstanceForPath(String path) =>
      getInstance(path: path); // TODO do we need ioDirectory?
# combine-prs

          cargo build -r
          # TODO update generated files
          # TODO run code formatter
          git config --global user.name 'MimirActionsBot'
          git config --global user.email '[email protected]'
          git commit -am "chore: update generated files"
          git push
  • Try macos-latest in regular build CI file

@GregoryConrad
Copy link
Owner

GregoryConrad commented Jul 30, 2024

Switching to dynamic linking would be a one-line fix on iOS/macOS if it weren't for CocoaPods. Thanks CocoaPods! Now I get to rediscover how to get iOS/macOS builds working again.

Looked into Swift Package Manager integration into Flutter but that appears as though it'll take some time before it reaches stable. So will need to find a new solution that deals with CocoaPods in the meantime.

@GregoryConrad GregoryConrad force-pushed the update-flutter-rust-bridge branch from a02fe32 to a7ce74d Compare July 30, 2024 17:26
@mcmah309
Copy link
Contributor Author

mcmah309 commented Jul 30, 2024

Yup, sounds like flutter development 😅

Btw, for this failure, I manually changed it to just String to get it to work. There was one more spot too where I had to add an int cast. Likely bugs in the current flutter rust bridge version.

edit: I see you just fixed it. Nice timing.

@GregoryConrad
Copy link
Owner

Btw, for this failure, I manually changed it to just String to get it to work. There was one more spot too where I had to add an int cast. Likely bugs in the current flutter rust bridge version.

Hm. I fixed the toString failure by excluding it from codegen (since it was never supposed to be included in codegen anyways). Some bug in FRB made it get included even though it wasn't pub.

Also haven't seen the int casting issue pop-up--maybe something else I did fixed that.

@GregoryConrad
Copy link
Owner

I think I'm going to do a temp fix here for iOS/macOS that reverts to using the discouraged ExternalLibrary.process method until Swift Package Manager support in Flutter rolls. That way I avoid having to rewrite and re-experiment with all of the iOS/macOS build stuff (which is extremely time consuming). I have the basics of dynamic libraries working with CocoaPods but it'll require quite a few hacks to get right, and since we are already using hacks with ExternalLibrary.process, might as well use the hack that enables the most code re-use.

That being said, notes to future self once SPM support comes:

  • update build-apple.sh to use .dylib instead of .a
  • update interface.dart in mimir to remove ExternalLibrary.process workaround

@GregoryConrad GregoryConrad merged commit a9a91fc into GregoryConrad:main Jul 31, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update flutter_rust_bridge To 2.1.0
2 participants