[path_provider] path provider linux implementation in dart #2589
Conversation
|
Here is the dart implementation of path_provider for Linux we discussed. I'm having trouble figuring out how to get the example app to run. It seems that currently there is a lack of support for automatically registering pure dart plugins for platforms other than web. Can you confirm or point me in the right direction? Currently the Makefile expects there to be a C++ header file by the name of path_provider_linux.h. |
|
I took a look at the place where the plugin registry is built: My proposal would be to:
By adding this language parameter this we could eventually allow using ffi (through dart parameter in pubspec), C# for windows, etc... |
Yes, unfortunately that's the case; a couple of us were looking at that a bit at the end of last week and discovered that the planned solution hasn't actually been implemented yet.
We shouldn't need that; the goal is to implement the platform interface, bypassing platform messages entirely. That's one of the things the new platform interface abstraction was designed to address.
The existing plan for allowing Dart registration on any platform is described here. There's no plan to add something as generic as "language" for all plugin platforms as the potential for confusion is high. The complexity is not in generating the Dart plugin registrar, but in finding a way to invoke it in a way that doesn't have problematic edge cases. It's something that will need more investigation and discussion in the coming weeks.
In general, we want to be extremely thoughtful about when/how we add native language options for plugins; allowing lots of choice at that layer is not necessarily a feature. |
|
@stuartmorgan Thanks for pointing me to that document. I haven't forgotten this plugin. Is there an issue I can follow to know when the flutter_tools support of dart plugins for non-web clients is available? |
|
flutter/flutter#52267 is tracking dartPluginClass plumbing. However, based on recent discussions I think while we're waiting for that we can do manual registration in the front-end plugin. So we would have path_provider endorse path_provider_linux, and then for |
|
@TimWhiting I'm going to be working on setting up the testing infrastructure for Linux (which is a blocker for landing Linux plugins) next week; if you're interested in updating this PR we can use it to continue blazing the trail of landing an FFI-based Linux plugin. I think it's pretty close, the main things are to add the registration to the front-end package (to work around the lack of support for automatic registration), and removing the |
|
Okay, I'll look into it tomorrow, thanks! |
1b872e5 to
63e41ef
Compare
The podspec stub is still required for iOS. I believe some kind of Android stub is still required on stable as well for the moment. |
Okay, I'll revert that last commit |
|
On the testing infrastructure front, the |
packages/path_provider/path_provider_linux/ios/Classes/PathProviderLinuxPlugin.h
Outdated
Show resolved
Hide resolved
packages/path_provider/path_provider_linux/example/linux/Makefile
Outdated
Show resolved
Hide resolved
|
The lint errors look like there is a tighter set of requirements being enforced on new/changed packages. https://github.com/flutter/plugins/blob/master/packages/google_maps_flutter/google_maps_flutter/ios/google_maps_flutter.podspec looks like one that's recent enough to address the things flagged by the linter. |
I'm not sure what's going on there; my local 3.10 test didn't have that issue. I'll need to try to figure out how to get verbose build output enabled for a bot run (which will be tricky since the tools that drive the build are from a published package). |
|
|
Fix landed; if you merge in master, I think there's a good chance this will finally be green across the board! 🤞 |
|
It looks like a lot of plugins are now removing the no-op android folders. Should I do the same here? |
Yes, I hadn't followed which release that bug was waiting on, but apparently it was 1.17. No need to land 9E88 the Android no-op if it's no longer needed. |
|
@stuartmorgan |
|
I was planning on landing and publishing it this morning. I was just doing a final once-over and noticed that |
… as well as the notification that path_provider 1.0.0 is coming, with instructions on how to set dependencies to allow for an upgrade to 1.0.0
|
@stuartmorgan |
|
The readme is somewhat misleading since there is a step other than adding it to your pubspec, but since we should be doing the follow-up to endorse it shortly I'll land it as-is. |
|
https://pub.dev/packages/path_provider_linux 🎉 Congratulations on getting the first Linux flutter/plugins plugin, and the first pure-Dart desktop plugin, landed and published 🙂 Thanks again for bearing with all the difficulties of blazing this trail. You should be able to do the endorsement PR now, which I expect to go much more smoothly. |
|
@TimWhiting In unit test(s) Am i clear in my understanding? I then have to mock method channel sometimes. See https://stackoverflow.com/questions/56158676/why-applicationsdocumentsdirectory-return-null-for-unit-test |
|
@apgapg You are correct that Or see the discussion on this pull request about |
|
@TimWhiting for first question: But when i run the same command in my CI/CD on github action where the test run on ubuntu, they all passed. So the flutter test in my case is behaving differently on different platforms. For second para: |
|
@apgapg i'm not sure you are testing what you think you are testing.. but you probably want to just mock plugins in unit tests anyway, mocking path_provider method channel is actually an example on the flutter test page: https://flutter.dev/docs/cookbook/persistence/reading-writing-files#testing |
|
@apgapg The macos plugin uses method channels so it needs to be mocked in the same way as your link you wrote about earlier. (Unless you mock the What is it that you are trying to test? Tests for Also make sure you remove any manual registration of the Linux implementation of PathProviderPlatform, if you were doing that. |
|
@TimWhiting @hpoul Yes its my ignorance about testing code. I didn't mock path_provider and just messing around with nothing. Thanks both of you for clarifying and helping me to learn it in more better way. UPDATE 1:
|
Description
This PR contributes a Linux implementation of the path provider plugin. It is a work in progress, since it seems that pure dart implementations aren't automatically detected / registered. (Flutter generates a makefile that expects a C++ class / header file).
Related Issues
flutter/flutter#41716 would be resolved by this PR
google/flutter-desktop-embedding PR #674 contains discussion related / leading to this implementation
flutter/packages PR #117 contains work leading up to this
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?