[video_player_avfoundation] Adds Swift Package Manager compatibility by vashworth · Pull Request #6675 · flutter/packages

vashworth · 2024-05-06T15:51:02Z

Makes video_player_avfoundation available as a Swift Package to Flutter. Also, remains compatible with CocoaPods.

Redo of #6634.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

vashworth · 2024-05-06T15:51:48Z

...avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/messages.g.m

 // See also: https://pub.dev/packages/pigeon

-#import "messages.g.h"
+#import "./include/video_player_avfoundation/messages.g.h"


I changed this manually. This will get overwritten by pigeon until flutter/flutter#147587 is fixed.

vashworth · 2024-05-06T15:52:08Z

packages/video_player/video_player_avfoundation/example/macos/Runner/AppDelegate.swift

 import FlutterMacOS

-@NSApplicationMain
+@main


@NSApplicationMain was deprecated, tool migrated it: flutter/flutter#146848

vashworth · 2024-05-06T15:52:24Z

packages/video_player/video_player_avfoundation/example/ios/Runner.xcodeproj/project.pbxproj

 /* Begin PBXBuildFile section */
 		1498D2341E8E89220040F4C2 /* GeneratedPluginRegistrant.m in Sources */ = {isa = PBXBuildFile; fileRef = 1498D2331E8E89220040F4C2 /* GeneratedPluginRegistrant.m */; };
 		3B3967161E833CAA004F5970 /* AppFrameworkInfo.plist in Resources */ = {isa = PBXBuildFile; fileRef = 3B3967151E833CAA004F5970 /* AppFrameworkInfo.plist */; };
+		78CF8D742BC5CEA80051231B /* OCMock in Frameworks */ = {isa = PBXBuildFile; productRef = 78CF8D732BC5CEA80051231B /* OCMock */; };


Note: pbxproj file does not contain Swift Package Manager migration changes so that it will continue to build with stable. Migration will happen on the fly.

vashworth · 2024-05-06T15:53:48Z

packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Package.swift

+        .headerSearchPath("include/video_player_avfoundation")
+      ]
+    ),
+    .target(


Comment from previous PR by @stuartmorgan:
Interesting, so SPM doesn't allow source filtering without making separate targets?

Correct, it doesn't allow you to conditionalize by platform very much, only target dependencies, CSettings, CXXSettings, SwiftSettings, and LinkerSettings.

Sources can only be nil or an array of strings

vashworth · 2024-05-06T15:54:46Z

...on/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m

-#import "messages.g.h"
+#import "./include/video_player_avfoundation/AVAssetTrackUtils.h"
+#import "./include/video_player_avfoundation/FVPDisplayLink.h"
+#import "./include/video_player_avfoundation/messages.g.h"


Comment from previous PR by @stuartmorgan:
Do we know why include paths aren't taking care of this?

So for most of the headers, include paths is sufficient. However, messages.g.h is a problem when there's multiple plugins that also have a messages.g.h. Seems like it can't resolve which to use. More explanation here: flutter/flutter#147587 (comment)

So I decided to change all of them to relative paths to keep it uniform

I'm a little surprised the compilation doesn't scope the search to just the module. Maybe it's the way we have to make all the headers public to make our unit tests work?

We could consider using a different naming convention for Obj-C out from Pigeon in our repo to give them unique names to avoid this, but relative paths are fine too.

vashworth · 2024-05-06T15:55:40Z

packages/video_player/video_player_avfoundation/pubspec.yaml

 repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player_avfoundation
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22
-version: 2.5.7
+version: 2.6.0


Comment from previous PR by @stuartmorgan:
Optional: I would probably make this a minor bump, since it's more of a new feature than a fix.

I changed it to a minor bump

loic-sharma

Re-LGTM

stuartmorgan-g

LGTM

…ibility (flutter/packages#6675)

flutter/packages@8de142d...6c4482a 2024-05-09 15619084+vashworth@users.noreply.github.com [image_picker_ios] Adds Swift Package Manager compatibility (flutter/packages#6696) 2024-05-09 goderbauer@google.com [flutter_lints] Rev to 4.0.0; prepare for publishing (flutter/packages#6695) 2024-05-09 10687576+bparrishMines@users.noreply.github.com [pointer_interceptor] Remove `implements` from app-facing package (flutter/packages#6699) 2024-05-08 15619084+vashworth@users.noreply.github.com Temporarily add empty header files to video_player_avfoundation so include directory is downloaded (flutter/packages#6694) 2024-05-08 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[image_picker_ios] Adds Swift Package Manager compatibility to image_picker_ios (#6617)" (flutter/packages#6693) 2024-05-08 15619084+vashworth@users.noreply.github.com [image_picker_ios] Adds Swift Package Manager compatibility to image_picker_ios (flutter/packages#6617) 2024-05-08 15619084+vashworth@users.noreply.github.com [video_player_avfoundation] Adds Swift Package Manager compatibility (flutter/packages#6675) 2024-05-08 737941+loic-sharma@users.noreply.github.com [ios_platform_images] Add Swift Package Manager support (flutter/packages#6684) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…lutter#6675) Makes `video_player_avfoundation` available as a Swift Package to Flutter. Also, remains compatible with CocoaPods. Fixes flutter/flutter#146921. Redo of flutter#6634.

Convert video_player_avfoundation into a Swift Package

b6022e3

github-actions bot added platform-ios platform-macos p: video_player labels May 6, 2024

vashworth commented May 6, 2024

View reviewed changes

vashworth marked this pull request as ready for review May 6, 2024 15:56

vashworth requested a review from hellohuanlin as a code owner May 6, 2024 15:56

vashworth requested review from stuartmorgan-g and removed request for hellohuanlin May 6, 2024 15:56

loic-sharma approved these changes May 6, 2024

View reviewed changes

stuartmorgan-g approved these changes May 6, 2024

View reviewed changes

vashworth added the autosubmit Merge PR when tree becomes green via auto submit App label May 8, 2024

auto-submit bot merged commit 09a373f into flutter:main May 8, 2024

engine-flutter-autoroll mentioned this pull request May 9, 2024

Roll Packages from 8de142d33b88 to 4e796783a265 (6 revisions) flutter/flutter#148059

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 9, 2024

09a373f [video_player_avfoundation] Adds Swift Package Manager compat…

436c263

…ibility (flutter/packages#6675)

engine-flutter-autoroll mentioned this pull request May 9, 2024

Roll Packages from 8de142d33b88 to 4799c4963496 (7 revisions) flutter/flutter#148068

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 9, 2024

09a373f [video_player_avfoundation] Adds Swift Package Manager compat…

2e357a1

…ibility (flutter/packages#6675)

engine-flutter-autoroll mentioned this pull request May 9, 2024

Roll Packages from 8de142d33b88 to 6c4482a64492 (8 revisions) flutter/flutter#148079

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 9, 2024

09a373f [video_player_avfoundation] Adds Swift Package Manager compat…

2c0f0bc

…ibility (flutter/packages#6675)

reidbaker mentioned this pull request Jul 2, 2024

i151188 update espresso dependencies v2 reidbaker/packages#6

Closed

11 tasks

FirentisTFW mentioned this pull request Dec 4, 2024

[video_player_avfoundation] Split iOS native code into multiple files #8171

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[video_player_avfoundation] Adds Swift Package Manager compatibility#6675

[video_player_avfoundation] Adds Swift Package Manager compatibility#6675
auto-submit[bot] merged 1 commit intoflutter:mainfrom
vashworth:video_player_avfoundation_swift_package_v2

vashworth commented May 6, 2024

Uh oh!

vashworth May 6, 2024

Uh oh!

vashworth May 6, 2024

Uh oh!

vashworth May 6, 2024

Uh oh!

vashworth May 6, 2024

Uh oh!

vashworth May 6, 2024

Uh oh!

vashworth May 6, 2024

Uh oh!

vashworth May 6, 2024

Uh oh!

stuartmorgan-g May 6, 2024

Uh oh!

vashworth May 6, 2024

Uh oh!

vashworth May 6, 2024

Uh oh!

loic-sharma left a comment

Uh oh!

stuartmorgan-g left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

vashworth commented May 6, 2024

Pre-launch Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants