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

build: Swift Package Manager support #1150 #1155

Conversation

Xa69
Copy link

@Xa69 Xa69 commented Aug 19, 2024

  • This PR is for including changes made as part of the work item mentioned in the heading.
  • This is a draft PR and has the following known issues :
  1. You will encounter the error 'Parse Issue (Xcode): Module 'image_picker_ios' not found’ when attempting to build with SPM, this is since the ‘image_picker_ios’ does not have the required support for SPM yet (issue is also mentioned in the comments of the work item)
  2. If you attempt to remove the ‘image_picker_ios’ module you might encounter another build issue and will also limit the functionality of the plugin.
  3. You may also encounter the error 'Swift Compiler Error (Xcode): No such module ‘MLKitVision’’ this is related to cocoapods, this should be moved to SPM eventually. But a clean and re build should resolve this for you.

Leaving this PR ‘Open’ until ‘image_picker_ios’ has the SPM support and we can further proceed to make the required changes to transition this into SPM and away from Cocoapods.

@Xa69 Xa69 changed the title Swift Package Manager support #1150 build:Swift Package Manager support #1150 Aug 19, 2024
@Xa69 Xa69 changed the title build:Swift Package Manager support #1150 build: Swift Package Manager support #1150 Aug 19, 2024
@navaronbracke navaronbracke self-requested a review August 19, 2024 16:41
Copy link
Collaborator

@navaronbracke navaronbracke left a comment

Choose a reason for hiding this comment

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

Mainly TODO's that can now be removed, a comment about MacOS and a question about a dependency on MLKit.

Otherwise, this looks good. I'll test it out later this week, to see if it runs!

ios/mobile_scanner/Package.swift Outdated Show resolved Hide resolved
ios/mobile_scanner/Package.swift Outdated Show resolved Hide resolved
ios/mobile_scanner/Package.swift Outdated Show resolved Hide resolved
ios/mobile_scanner/Package.swift Outdated Show resolved Hide resolved
// If the plugin name contains "_", replace with "-" for the library name.
.library(name: "mobile-scanner", targets: ["mobile_scanner"])
],
dependencies: [],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we need to wait for MLKit to publish an SPM version of their package, before adding it here?

If your ios/plugin_name.podspec file has [CocoaPods dependency](https://guides.cocoapods.org/syntax/podspec.html#dependency)s, add the corresponding [Swift Package Manager dependencies](https://developer.apple.com/documentation/packagedescription/package/dependency) to your Package.swift file.

Copy link
Author

Choose a reason for hiding this comment

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

Looks like MLKit is only supported by Cocoapods, I do have a work around here https://github.com/kientux/google-mlkit-spm

But it looks like they are not planning to support SPM based off the interaction here (This is a very old conversation) : googlesamples/mlkit#627

Let me know if we should try the workaround

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, I'm not sure if we should start using a third-party dependency to work around this.
I'll ask in the Flutter #package-authors chat with Loïc (who is working on SPM support in flutter/packages) maybe they can reach out to other Googlers for this (since MLKit is a Google product)

cc @juliansteenbakker for an additional opinion.

ios/mobile_scanner/Package.swift Outdated Show resolved Hide resolved
ios/mobile_scanner/Package.swift Outdated Show resolved Hide resolved
ios/mobile_scanner/Package.swift Outdated Show resolved Hide resolved
Copy link
Collaborator

@navaronbracke navaronbracke left a comment

Choose a reason for hiding this comment

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

Some leftover comments that can be removed.

Thanks for including MacOS too!
As for the MLKit issue, I'll see what I can do about that.

macos/mobile_scanner/Package.swift Outdated Show resolved Hide resolved
macos/mobile_scanner/Package.swift Outdated Show resolved Hide resolved
ios/mobile_scanner/Package.swift Outdated Show resolved Hide resolved
ios/mobile_scanner/Package.swift Show resolved Hide resolved
ios/mobile_scanner/Package.swift Outdated Show resolved Hide resolved
// If the plugin name contains "_", replace with "-" for the library name.
.library(name: "mobile-scanner", targets: ["mobile_scanner"])
],
dependencies: [],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, I'm not sure if we should start using a third-party dependency to work around this.
I'll ask in the Flutter #package-authors chat with Loïc (who is working on SPM support in flutter/packages) maybe they can reach out to other Googlers for this (since MLKit is a Google product)

cc @juliansteenbakker for an additional opinion.

@navaronbracke
Copy link
Collaborator

@Xa69 I did reach out to Loïc Sharma in Flutter's #hackers-ios channel on the Flutter Discord. He sent an email to the respective people for MLKit to see what we can do about it.

In the meantime, what if you split off the MacOS SPM support into a separate PR? We can already land that. But for iOS we will have to wait until the MLKit issue is resolved.

@Xa69
Copy link
Author

Xa69 commented Aug 21, 2024

@navaronbracke I agree we can at least provide support for MacOS until the issue with iOS is dealt with.

Raised a PR only for MacOS : #1161

If everything is in order, it will be merged.

@navaronbracke
Copy link
Collaborator

@Xa69 I did reach out to a Googler for the MLKit problem. Unfortunately, the MLKit repository is closed-source, and they don't have plans to add SPM support right now.

According to the migration guide for SPM, the tool will fall back to using CocoaPods for dependencies that do not have SwiftPM support, though. That is a mixed mode that is supported. (Not sure if this will stay supported indefinitely)

If you enable SPM support for the iOS portion of this plugin, while keeping the MLKit version as-is, does the example app work properly? If that isn't the case, we will have to see if we can explore other options (one of which is dropping MLKit for iOS and using the Vision API there too)

@navaronbracke
Copy link
Collaborator

According to the migration guide for SPM, the tool will fall back to using CocoaPods for dependencies that do not have SwiftPM support, though. That is a mixed mode that is supported. (Not sure if this will stay supported indefinitely)

If you enable SPM support for the iOS portion of this plugin, while keeping the MLKit version as-is, does the example app work properly? If that isn't the case, we will have to see if we can explore other options (one of which is dropping MLKit for iOS and using the Vision API there too)

@Xa69 did you manage to look into this already? If we could support SPM by using the mixed mode, that would be a good interim solution. If the mixed mode becomes unsupported in the future, then we will have to either hope for an SPM version of MLKit or switch to the Vision API for iOS too.

@Xa69
Copy link
Author

Xa69 commented Aug 31, 2024

Hi @navaronbracke sorry I missed this, I’ll check and get back to you.
I’m trying to get the repro steps for the ‘image_picker_ios’ issue for Loïc.

@navaronbracke
Copy link
Collaborator

No problem! Yes, unblocking that image_picker issue seems more important at the moment. We can get back to this when you have the time.

@navaronbracke
Copy link
Collaborator

navaronbracke commented Sep 3, 2024

@Xa69 The image_picker issue was closed. Apparently the other user that had the issue, had to recreate the iOS portion of their project. Does deleting the example/ios directory and running flutter create . fix the issue for you? If it does, please commit the diff (if there is any reasonable output from that diff) to this PR.

@navaronbracke
Copy link
Collaborator

@Xa69 I got word back from Loïc and unfortunately running in mixed mode does not mean that we can use the MLKit Cocoapods dependency when adding SPM support to this plugin.

@Xa69
Copy link
Author

Xa69 commented Sep 6, 2024

@navaronbracke So if we want to use MLKit we will need to depend on Cocoapods and cannot use SPM to build the project?

Although I still am confused on how I was getting the ‘image_picker’ issue without the MLKit error, it is possible that it was a caching the dependency from previous Cocoapods build commands.

@navaronbracke
Copy link
Collaborator

navaronbracke commented Sep 6, 2024

So if we want to use MLKit we will need to depend on Cocoapods and cannot use SPM to build the project?

Yes, that is the case. To quote Loïc's answer from the thread on the Flutter Discord: "Unfortunately you can only mix CocoaPods and SwiftPM across different plugins, not within a single plugin. AKA, plugin foo can use CocoaPods and bar can use SwiftPM. But foo can't use both CocoaPods and SwiftPM."

Although, I have been investigating a different solution: Switching to the Vision API for iOS as well (we use that on MacOS)
That would enable us to support SPM for iOS, although requires a bit of work

@Xa69
Copy link
Author

Xa69 commented Sep 6, 2024

@navaronbracke so for now shall we abandon this PR till Vision API changes are in?
We can also switch to the common Darwin folder along side and merge both changes for MacOS and iOS

@navaronbracke
Copy link
Collaborator

@Xa69 We can keep it as a draft for now. I'll focus on getting the Vision API changes in first, and then we can pick this up again.

I'll put it high on my priority list, as I currently don't have any big features planned for mobile_scanner.

@Xa69
Copy link
Author

Xa69 commented Sep 6, 2024

Alright let me know when we can pick this up again and I will verify/add in changes if needed. @navaronbracke

@navaronbracke
Copy link
Collaborator

navaronbracke commented Oct 24, 2024

@Xa69 We dropped support for MLKit in our latest beta, version 7.0.0-beta.1.

We merged both the iOS & MacOS codebases, so both now live under the Sources folder, and there is a Package.swift that lists both iOS & MacOS. (the latter will be available in the next beta, as we forgot to include iOS for the first beta there)

I'm not sure if the image_picker issue is still present, but you might be able to test the current master branch with SPM now. If that works properly then this PR is no longer needed.

@juliansteenbakker
Copy link
Owner

Hi all, is there any more work that can be done here? Or is this PR obsolete?

@navaronbracke
Copy link
Collaborator

navaronbracke commented Nov 13, 2024

@juliansteenbakker I think that this is already working, with the merge of iOS & MacOS.
We just need to test it, using the steps in https://docs.flutter.dev/packages-and-plugins/swift-package-manager/for-plugin-authors#how-to-turn-on-swift-package-manager

We should keep it as available, but disabled on stable until it is turned on in stable as well.

Edit: Updated the link

@loic-sharma
Copy link

We should keep it as available, but disabled on stable until it is turned on in stable as well.

Hey there, the Flutter team recommends you add SwiftPM support to your plugins today, if possible!

Flutter's stable release does support SwiftPM, the feature is just off by default. If you add SwiftPM support to your plugin, it will still work as expected on the stable branch.

Tons of other plugins have already landed SwiftPM support, like Firebase's cloud_firestore (firebase/flutterfire#13329), or plus plugins like share_plus (fluttercommunity/plus_plugins#3169), or sqflite (tekartik/sqflite@68b17c9).

We recently updated pub.dev's package scoring to check for Swift Package Manager compatibility. See the Platform support section in mobile_scanner's Scores tab.

If there's anything that's blocking you from adding Swift Package Manager support, please let us know!

@navaronbracke
Copy link
Collaborator

navaronbracke commented Nov 13, 2024

@loic-sharma Ah, that is convenient! We added SPM support in our current beta version. We already had it for MacOS for a while. And with the new beta version of our plugin we merged iOS & MacOS using the darwin folder strategy, which enabled providing SPM for iOS, too.

I'll test the current beta with Flutter's stable release, just to be sure. But otherwise this migration is complete. Thanks for reaching out!

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.

4 participants