-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[shared_preferences_foundation] Add Swift runtime search paths for Objective-C apps #6952
Conversation
| 97C147011CF9000F007C117D /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 97C146FF1CF9000F007C117D /* LaunchScreen.storyboard */; }; | ||
| 9AF3A5CAB88B27F2BC36D686 /* Pods_RunnerTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = D4C443E7E16FB2AD978AC1D6 /* Pods_RunnerTests.framework */; }; | ||
| B81650923B266CE1F32B75E4 /* Pods_Runner.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = F3702C135032CE4599D8327B /* Pods_Runner.framework */; }; | ||
| F7E47F772970EBA900FA52F3 /* AppDelegate.m in Sources */ = {isa = PBXBuildFile; fileRef = 7AFFD8EE1D35381100E5BB4D /* AppDelegate.m */; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of reverting the project file back to before #6940 I instead surgically modified the project file to do the minimum to convert it back to an Objective-C app using static library versions of the plugin.
| 'LIBRARY_SEARCH_PATHS' => '$(TOOLCHAIN_DIR)/usr/lib/swift/$(PLATFORM_NAME)/ $(SDKROOT)/usr/lib/swift', | ||
| 'LD_RUNPATH_SEARCH_PATHS' => '/usr/lib/swift', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the fix.
| @implementation SharedPreferencesTests | ||
|
|
||
| - (void)testPlugin { | ||
| SharedPreferencesPlugin *plugin = [[SharedPreferencesPlugin alloc] init]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is pretty lame, I didn't re-write the better Swift tests back to Objective-C. Will do that after this lands, to unblock the tree ASAP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have to abandon the Swift tests? I would have thought we could have Swift tests even though the app is Objective-C.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that does the automagical thing with the Swift runtimes when running an XCTest since Xcode does all kinds of voodoo to get the tests to see non-public APIs? Relying on this from the example app seems fragile though, it's just luck that we caught it in the storekit example app. I'll try to think about a better way to holistically test this problem...
stuartmorgan-g
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM to fix the regression; one question about the test follow-up.
|
(Thanks for cleaning this up, and sorry I ignored the warning sign of the CI issue when I switched the app-facing package.) |
|
Merging. @stuartmorgan to publish tomorrow, which should green out the tree. |
shared_preferences_foundationis using Swift as of #6940. The Objective-C projects do not automatically set up the search paths to find the Swift runtimes. Hook that up in theshared_preferences_foundationpodspec.See explanation at flutter/flutter#16049 (comment) and the first time this was done in plugins: #6369
Convert the example project back to Objective-C to catch future versions of this issue.
Fixes flutter/flutter#118418
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.