[google_sign_in_ios] Adds Swift Package Manager support #7356

loic-sharma · 2024-08-09T19:31:22Z

Adds Swift Package Manager support to google_sign_in_ios.

Pre-launch Checklist

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

loic-sharma · 2024-08-09T19:38:38Z

packages/google_sign_in/google_sign_in_ios/example/ios/Podfile

@@ -26,17 +26,14 @@ end
 require File.expand_path(File.join('packages', 'flutter_tools', 'bin', 'podhelper'), flutter_root)

 # Suppress warnings from transitive dependencies that cause analysis to fail.
-pod 'AppAuth', :inhibit_warnings => true
-pod 'GTMAppAuth', :inhibit_warnings => true
+inhibit_all_warnings!


The previous lines inhibited warnings but also added AppAuth and GTMAppAuth as CocoaPod dependencies:

pod 'AppAuth', :inhibit_warnings => true pod 'GTMAppAuth', :inhibit_warnings => true

If the SwiftPM feature is enabled, this causes duplicate modules and the build fails.

loic-sharma · 2024-08-09T19:41:54Z

packages/google_sign_in/google_sign_in_ios/darwin/google_sign_in_ios/Package.swift

+  name: "google_sign_in_ios",
+  platforms: [
+    .iOS("12.0"),
+    .macOS("10.15")


This is bumped up to macOS 10.15. Required to depend on GoogleSignIn (here)

We already require 10.15 (which is good, because otherwise this would be very hard to roll out in a non-disruptive way).

loic-sharma · 2024-08-09T19:43:18Z

packages/google_sign_in/google_sign_in_ios/darwin/google_sign_in_ios/Package.swift

+  ],
+  dependencies: [
+    // AppAuth and GTMSessionFetcher are GoogleSignIn transitive dependencies.
+    // Depend on versions which define modules.


This mirrors:

packages/packages/google_sign_in/google_sign_in_ios/darwin/google_sign_in_ios.podspec

Lines 20 to 24 in 275e985

# AppAuth and GTMSessionFetcher are GoogleSignIn transitive dependencies.

# Depend on versions which defines modules.

s.dependency 'AppAuth', '>= 1.7.6'

s.dependency 'GTMSessionFetcher', '>= 3.4.0'

s.dependency 'GoogleSignIn', '~> 7.1'

Do we really need this? Isn't not defining modules something that would only apply to CocoaPods, not SPM?

Good question, let me fiddle with this

stuartmorgan · 2024-10-08T19:37:50Z

From triage: still blocked on upstream issues.

loic-sharma · 2024-11-26T18:46:14Z

AppAuth-iOS 1.7.6 was released with the fix, this PR should be unblocked now: openid/AppAuth-iOS#871

loic-sharma · 2024-11-27T23:01:42Z

This is failing xcode-analyze and native-test because our transitive dependency AppAuth has warnings, and we treat warnings as errors. This isn't a problem when using CocoaPods as we've configured it to ignore these warnings. Unfortunately, SwiftPM doesn't let suppress warnings in our dependencies.

Our options are to either allow packages to opt-out of warnings as errors, or, fix the warnings in the upstream dependency: openid/AppAuth-iOS#888

stuartmorgan · 2024-12-02T19:20:16Z

I'm okay with temporarily turning off warnings-as-errors for this package, but we should have a tracking issue with context for turning it back on.

Ideally we should also reference an SPM feature request for the ability to do what we used to do with Cocoapods. Having the ability to use different warning settings for code you control and code you don't is a fairly common feature in build systems.

Today, we treat Xcode warnings as errors on all iOS and macOS plugins. However, the google_sign_in_ios plugin has dependencies with warnings. We're unable to suppress these warnings when using Swift Package Manager. As a workaround, we'll need to disable Xcode warnings for the google_sign_in_ios plugin. This will be done in a subsequent pull request (see #7356). This change introduces `--xcode-warnings-exceptions` to the `native-test` command. When running Xcode test, `GCC_TREAT_WARNINGS_AS_ERRORS` will be provided only if the plugin isn't on the exception list. Additionally, plugins that are the exception list are excluded from the `xcode-analyze` command. Part of flutter/flutter#146904

loic-sharma · 2025-02-04T23:47:47Z

packages/google_sign_in/google_sign_in_ios/darwin/google_sign_in_ios.podspec


  # AppAuth and GTMSessionFetcher are GoogleSignIn transitive dependencies.
  # Depend on versions which defines modules.
-  s.dependency 'AppAuth', '>= 1.7.4'
+  s.dependency 'AppAuth', '>= 1.7.6'


This fixes some but not all AppAuth-iOS warnings

stuartmorgan · 2025-02-05T15:25:09Z

Ideally we should also reference an SPM feature request for the ability to do what we used to do with Cocoapods. Having the ability to use different warning settings for code you control and code you don't is a fairly common feature in build systems.

Did we find/file an issue for this that we can reference in the comments?

vashworth · 2025-02-06T15:17:14Z

Ideally we should also reference an SPM feature request for the ability to do what we used to do with Cocoapods. Having the ability to use different warning settings for code you control and code you don't is a fairly common feature in build systems.

Did we find/file an issue for this that we can reference in the comments?

Looks like you can potentially use an unsafeFlag, however, "use of unsafe flags makes the products containing this target ineligible for use by other packages". I wonder if we could use it conditionally only during testing. @loic-sharma might be worth trying out. Perhaps using an environment variable?

https://stackoverflow.com/a/64842804

stuartmorgan · 2025-02-06T15:25:16Z

It's not clear to me that that SO answer address our (and the questioner's) use case; it's showing suppressing warnings on a target being defined as part of the package, not on a dependency.

Either way though, "I want warnings in my code, but not in code I don't control" is a pretty standard thing to want, and a use case we have a demonstrated need for, so we should request SPM fully (not only via unsafe flags) support it.

stuartmorgan · 2025-02-06T15:28:45Z

To clarify, I don't consider this as blocking landing; if we do have a reference we can add, great, if not we can land without it but I think it's important that we (in parallel) provide feedback on ways that SPM doesn't work for us.

loic-sharma · 2025-02-06T18:35:19Z

I did try various configurations of unsafeFlags, cSettings, swiftSettings to suppress dependency warnings. These worked locally on my machine. However, I could not get these suppressions to work in our CI. I was unable to reproduce locally, even if I used the same version of Xcode as our CI... It's not clear to me what's going wrong on our CI.

vashworth

LGTM

github-actions bot added p: google_sign_in platform-ios platform-macos labels Aug 9, 2024

loic-sharma force-pushed the spm_google_sign_in branch from b359e38 to b455e98 Compare August 9, 2024 19:35

loic-sharma commented Aug 9, 2024

View reviewed changes

loic-sharma force-pushed the spm_google_sign_in branch 7 times, most recently from f36566e to eb4c0d5 Compare August 9, 2024 22:34

loic-sharma force-pushed the spm_google_sign_in branch 3 times, most recently from a11e7df to 687528d Compare November 26, 2024 20:45

loic-sharma force-pushed the spm_google_sign_in branch 8 times, most recently from a25afdb to 8964482 Compare January 29, 2025 00:54

loic-sharma mentioned this pull request Jan 29, 2025

[tool] Add --xcode-warnings-exceptions flag #8524

Merged

11 tasks

loic-sharma force-pushed the spm_google_sign_in branch 2 times, most recently from 7eb402a to 77e6de9 Compare January 30, 2025 23:12

[google_sign_in_ios] Adds Swift Package Manager support

3cbc736

loic-sharma force-pushed the spm_google_sign_in branch from 77e6de9 to 3cbc736 Compare January 31, 2025 23:14

loic-sharma commented Feb 4, 2025

View reviewed changes

loic-sharma requested a review from vashworth February 4, 2025 23:49

loic-sharma marked this pull request as ready for review February 4, 2025 23:49

loic-sharma force-pushed the spm_google_sign_in branch from 3d9dff3 to 9455e71 Compare February 6, 2025 21:33

Remove transitive dependency hoisting

626a80a

loic-sharma force-pushed the spm_google_sign_in branch from 9455e71 to 626a80a Compare February 6, 2025 22:41

vashworth approved these changes Feb 7, 2025

View reviewed changes

loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 12, 2025

auto-submit bot merged commit fd53793 into flutter:main Feb 12, 2025
77 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[google_sign_in_ios] Adds Swift Package Manager support #7356

[google_sign_in_ios] Adds Swift Package Manager support #7356

loic-sharma commented Aug 9, 2024 •

edited

Loading

loic-sharma Aug 9, 2024 •

edited

Loading

loic-sharma Aug 9, 2024 •

edited

Loading

stuartmorgan Aug 10, 2024

loic-sharma Aug 9, 2024 •

edited

Loading

stuartmorgan Feb 5, 2025

loic-sharma Feb 6, 2025

stuartmorgan commented Oct 8, 2024

loic-sharma commented Nov 26, 2024

loic-sharma commented Nov 27, 2024

stuartmorgan commented Dec 2, 2024 •

edited

Loading

loic-sharma Feb 4, 2025

stuartmorgan commented Feb 5, 2025

vashworth commented Feb 6, 2025 •

edited

Loading

stuartmorgan commented Feb 6, 2025

stuartmorgan commented Feb 6, 2025 •

edited

Loading

loic-sharma commented Feb 6, 2025

vashworth left a comment

	# AppAuth and GTMSessionFetcher are GoogleSignIn transitive dependencies.
	# Depend on versions which defines modules.
	s.dependency 'AppAuth', '>= 1.7.6'
	s.dependency 'GTMSessionFetcher', '>= 3.4.0'
	s.dependency 'GoogleSignIn', '~> 7.1'

[google_sign_in_ios] Adds Swift Package Manager support #7356

[google_sign_in_ios] Adds Swift Package Manager support #7356

Conversation

loic-sharma commented Aug 9, 2024 • edited Loading

Pre-launch Checklist

loic-sharma Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

loic-sharma Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

stuartmorgan Aug 10, 2024

Choose a reason for hiding this comment

loic-sharma Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

stuartmorgan Feb 5, 2025

Choose a reason for hiding this comment

loic-sharma Feb 6, 2025

Choose a reason for hiding this comment

stuartmorgan commented Oct 8, 2024

loic-sharma commented Nov 26, 2024

loic-sharma commented Nov 27, 2024

stuartmorgan commented Dec 2, 2024 • edited Loading

loic-sharma Feb 4, 2025

Choose a reason for hiding this comment

stuartmorgan commented Feb 5, 2025

vashworth commented Feb 6, 2025 • edited Loading

stuartmorgan commented Feb 6, 2025

stuartmorgan commented Feb 6, 2025 • edited Loading

loic-sharma commented Feb 6, 2025

vashworth left a comment

Choose a reason for hiding this comment

loic-sharma commented Aug 9, 2024 •

edited

Loading

loic-sharma Aug 9, 2024 •

edited

Loading

loic-sharma Aug 9, 2024 •

edited

Loading

loic-sharma Aug 9, 2024 •

edited

Loading

stuartmorgan commented Dec 2, 2024 •

edited

Loading

vashworth commented Feb 6, 2025 •

edited

Loading

stuartmorgan commented Feb 6, 2025 •

edited

Loading