aboutyou / dart_packages

Dart and Flutter plugins maintained and used by @ABOUTYOU
222 stars 151 forks source link

[sign_in_with_apple] 9 issues when building Flutter app in new Xcode 16.0 #437

Closed appcapsergen closed 1 month ago

appcapsergen commented 2 months ago

I have just installed the newly released macOS 15, and with that Xcode 16.0.

Upon trying to (re)build my code, I have noticed that even in the latest version of your sign_in_with_apple package, these 9 issues occur (in full):

Screenshot 2024-09-17 at 7 47 38 PM

This urgently requires your attention, since anybody who upgrades their MacBook is now unable to run or build their Flutter app if they use this package.

0x1306e6d commented 1 month ago

As a workaround, I downgraded Swift Language Version to 5 and it works now for me.

image
appcapsergen commented 1 month ago

As a workaround, I downgraded Swift Language Version to 5 and it works now for me.

@0x1306e6d Thank you for your response, but your workaround does not work for me. Not sure if I'm doing something wrong, what were your steps to solve this exactly?

I will also wait for a response from the sign_in_with_apple team to resolve this for Swift 6.

tp commented 1 month ago

@appcapsergen I have not upgraded macOS yet, for fear of build problems in my actively developed projects.

But I'll see that I get the new Xcode installed side-by-side and then we should see the same "Swift 6" errors as well. Will then check how we can (conditionally) support both.

But as I am currently traveling, I will probably not get to it before the middle of next week.

appcapsergen commented 1 month ago

Thank you for the response and transparency @tp! I will hopefully see the progress in this issue.

0x1306e6d commented 1 month ago

Thank you for your response, but your workaround does not work for me. Not sure if I'm doing something wrong, what were your steps to solve this exactly?

I specified CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES to YES to address Firebase issue (https://github.com/firebase/flutterfire/issues/13323). Even so, I don't think it is related with this issue.

I will also wait for a response from the sign_in_with_apple team to resolve this for Swift 6.

+1

gwhizofss commented 1 month ago

Hi All, it looks like the podspec files for sign_in_with_apple have versions like 0.0.1. SHould those agree with the latest release 6.1.2?

tp commented 1 month ago

Hi All, it looks like the podspec files for sign_in_with_apple have versions like 0.0.1. SHould those agree with the latest release 6.1.2?

Those do not matter, as dependencies are not published to Cocoapods, and thus only the pub.dev versions are visible to the consumer (without digging into the files). So we don't need to bump those podspecs with every release.

appcapsergen commented 1 month ago

I specified CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES to YES to address Firebase issue (firebase/flutterfire#13323). Even so, I don't think it is related with this issue.

Then I don't think the work-around works for everyone (aka not for me)

gwhizofss commented 1 month ago

Hi All, it looks like the podspec files for sign_in_with_apple have versions like 0.0.1. SHould those agree with the latest release 6.1.2?

Those do not matter, as dependencies are not published to Cocoapods, and thus only the pub.dev versions are visible to the consumer (without digging into the files). So we don't need to bump those podspecs with every release.

It's strange, when I leave it at 0.0.1 then the output of the build shows it is loading version 0.0.1. I'm on MacOS Sonoma. Should I open a new issue instead? BTW is the plugin supported when building on Macs with the M1 chip?

tp commented 1 month ago

Should I open a new issue instead?

No, I don't think we need to put any effort into this version. Even if Pod logs 0.0.1, we know from the pubspec.yaml already what it actually is.

So unless it would now be a common pattern for iOS packages to bump the Pod spec file in unison with the pubspec, I would not do that.

We'll be working on SPM support next anyway, as Pods are on the way out (though we will still support them for a long time going forward).

BTW is the plugin supported when building on Macs with the M1 chip?

Yes, works fine for me :)

tp commented 1 month ago

I can confirm that the example breaks once I switch to Swift 6 language mode. But before that it compiles just fine with Xcode 16.

CleanShot 2024-09-25 at 11 59 20@2x

Will see that we make the adaptations in a way that ideally compiles with both versions.

gwhizofss commented 1 month ago

Should I open a new issue instead?

No, I don't think we need to put any effort into this version. Even if Pod logs 0.0.1, we know from the pubspec.yaml already what it actually is.

So unless it would now be a common pattern for iOS packages to bump the Pod spec file in unison with the pubspec, I would not do that.

We'll be working on SPM support next anyway, as Pods are on the way out (though we will still support them for a long time going forward).

BTW is the plugin supported when building on Macs with the M1 chip?

Yes, works fine for me :)

Can I suggest updating the documentation/comments that it is not for x86 only, but can work on M* with Rosetta2?

tp commented 1 month ago

Can I suggest updating the documentation/comments that it is not for x86 only, but can work on M* with Rosetta2?

Sorry, I don't understand what you're referring to. This plugin can compile natively for Apple Silicon (when building Mac apps) and does not need Rosetta.

Edit: Ah, I found the line in ios/sign_in_with_apple.podspec you must be talking about. To be honest, I have never read this auto-generated code, and I don't think whatever is written in there makes any difference in practice, as it runs perfectly fine on the simulator on Apple Silicon. But feel free to change it in a PR if you think this would confuse anyone.

Also I would appreciate if we could keep this issue focused on just the Xcode 16 compilation issues.

tp commented 1 month ago

A preview release is now available at https://pub.dev/packages/sign_in_with_apple/versions/6.1.3-dev1

I would highly appreciate if someone with the original problem would confirm that this now fixes the compilation of their app.

I have successfully tested this locally with Xcode 16 in both Swift 5 and 6 language modes (plus the CI checks we had for older versions in place already).

appcapsergen commented 1 month ago

@tp I can run my app again with v6.1.3-dev1!

appcapsergen commented 1 month ago

@tp actually, false positive 😅 I forgot to use Swift 6 in my project. Without selecting this for the Pod's, my app works again, but as soon as I specify Swift 6, I get the following Swift Compiler Error's:

Swift Compiler Error (Xcode): Call to main actor-isolated initializer 'init(_:)' in a synchronous nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleAvailablePlugin.swift:39:35

Swift Compiler Error (Xcode): Call to main actor-isolated instance method 'performRequests(requests:)' in a synchronous nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleAvailablePlugin.swift:44:29

Swift Compiler Error (Xcode): Call to main actor-isolated static method 'parseRequests(rawRequests:)' in a synchronous nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleAvailablePlugin.swift:45:65

Swift Compiler Error (Xcode): Reference to var 'FlutterMethodNotImplemented' is not concurrency-safe because it involves shared mutable state
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleAvailablePlugin.swift:100:19

Swift Compiler Error (Xcode): Main actor-isolated property 'systemName' can not be referenced from a nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleError.swift:31:47

Swift Compiler Error (Xcode): Main actor-isolated class property 'current' can not be referenced from a nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleError.swift:31:39

Swift Compiler Error (Xcode): Main actor-isolated property 'systemVersion' can not be referenced from a nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleError.swift:31:78

Swift Compiler Error (Xcode): Main actor-isolated class property 'current' can not be referenced from a nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleError.swift:31:70
tp commented 1 month ago

@tp I can run my app again with v6.1.3-dev1!

Awesome. Thanks for checking and your patience on this issue.

Noted for next year that we should start testing 1-2 weeks before the release, as this seems quite avoidable.

Published as 6.1.3 now: https://pub.dev/packages/sign_in_with_apple/versions/6.1.3

appcapsergen commented 1 month ago

@tp what about the errors I showed with Swift 6?

tp commented 1 month ago

I forgot to use Swift 6 in my project.

Weird. I also tested that (though I did not add a CI test to rewrite the project to also use Swift 6 yet), and locally I can run the example with Swift 6 language mode just fine.

CleanShot 2024-09-30 at 11 48 20@2x

Would be interesting to see what's different in your config (if anything) so I can address that as well.

tp commented 1 month ago

I could imagine (from my cursory knowledge of the Swift 6 changes) that we would need to mark our plug-in class or at least some methods as @MainActor.

But I haven't found any general information on this regarding Flutter, and also current examples on the latest Flutter stable (3.24.3) still generate code similar to this package's.

What Flutter version are you running @appcapsergen ?

HenriBeck commented 1 month ago

@appcapsergen this sounds like: https://github.com/flutter/flutter/issues/150388#issuecomment-2174576564

Did you already enable the strict concurrency setting?

HenriBeck commented 1 month ago

Nevermind, the Flutter issue read a bit like as if this was also a setting for Swift 6 that can be controlled.

But I haven't found any general information on this regarding Flutter, and also current examples on the latest Flutter stable (3.24.3) still generate code similar to this package's.

Since that is the case, I think we need to wait a little here to get some guidance from the Flutter Team on how to write plugins using Swift 6.

tp commented 1 month ago

Since that is the case, I think we need to wait a little here to get some guidance from the Flutter Team on how to write plugins using Swift 6.

Yeah, it's weird though that the SiwA example app with Swift 6 language mode + "complete" concurrency checks (via SWIFT_STRICT_CONCURRENCY = complete;) runs fine for me locally.

So @appcapsergen if you could get our example app to not run that'd already be great (just file a PR with the broken/non-working configuration, and the we could see that we add the necessary fixes to make it work again).

appcapsergen commented 1 month ago

@tp I'm sure I will be able to run the example app and even my app, because what I meant is the following;

In my app, in the Podfile, I specify a few build settings for all pods, including SWIFT_VERSION:

post_install do |installer|
  installer.pods_project.targets.each do |target|
    flutter_additional_ios_build_settings(target)
    target.build_configurations.each do |config|
      config.build_settings['IPHONEOS_DEPLOYMENT_TARGET'] = '14.0'
      config.build_settings['SWIFT_VERSION'] = '5.10' # <<<<<<<<--------- over here
      config.build_settings['CODE_SIGNING_ALLOWED'] = 'NO'
    end
  end
end

So what I meant is that if I set the Swift version for pods with config.build_settings['SWIFT_VERSION'] = '6.0', I do still get the following errors in the sign_in_with_apple package:

Swift Compiler Error (Xcode): Call to main actor-isolated initializer 'init(_:)' in a synchronous nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleAvailablePlugin.swift:39:35

Swift Compiler Error (Xcode): Call to main actor-isolated instance method 'performRequests(requests:)' in a synchronous nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleAvailablePlugin.swift:44:29

Swift Compiler Error (Xcode): Call to main actor-isolated static method 'parseRequests(rawRequests:)' in a synchronous nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleAvailablePlugin.swift:45:65

Swift Compiler Error (Xcode): Reference to var 'FlutterMethodNotImplemented' is not concurrency-safe because it involves shared mutable state
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleAvailablePlugin.swift:100:19

Swift Compiler Error (Xcode): Main actor-isolated property 'systemName' can not be referenced from a nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleError.swift:31:47

Swift Compiler Error (Xcode): Main actor-isolated class property 'current' can not be referenced from a nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleError.swift:31:39

Swift Compiler Error (Xcode): Main actor-isolated property 'systemVersion' can not be referenced from a nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleError.swift:31:78

Swift Compiler Error (Xcode): Main actor-isolated class property 'current' can not be referenced from a nonisolated context
/Users/sepekerimu/.pub-cache/hosted/pub.dev/sign_in_with_apple-6.1.3-dev1/ios/Classes/SignInWithAppleError.swift:31:70

So running my app is fixed, unless I specify the Swift version for the pods.

tp commented 1 month ago

I finally figured out how to create the original error. This had nothing to do with the app project's Swift version (e.g. our store apps or the SiwA example project), but rather the build settings on the this plugin's module:

CleanShot 2024-10-02 at 11 05 10@2x

The OP might've changed this by accident (or might there be an overwrite in some cases?), but since those files live outside the Git tree, it's hard to spot that change.

Anyway, I don't think we can switch that yet anyhow, as

a) that would break backwards compatibility for Xcode < 16 b) when turning it on, I get a bunch of errors that the Flutter base class we're implementing is incompatible with the usual @MainActor annotation I would add there

For b) there might be a more advanced workaround, but since this would then make this a breaking change and is overall not needed (from my experience a Swift 6 app works fine with this Swift 5 package – also at runtime) I would defer this until Flutter upgrades the Swift language version here. Then we can make a major release with requires Xcode 16 + the new Flutter version which would then use this.

tp commented 1 month ago

In my app, in the Podfile, I specify a few build settings for all pods, including SWIFT_VERSION

Why do you want to overwrite the Swift version for every dependency to something else that those package authors did not specify (or test)?

Is there any advantage I am missing?

I think you could change the Podfile to not apply that change for this plugin (and keep it for all others if you must), and the overall "Swift 6" app should run just fine then.

appcapsergen commented 1 month ago

@tp great timing, but yes indeed that was the (or one of the) problem(s).

I am honestly not sure why I've done this in my project as I did this ages ago. Considering to remove this line since it does appear to be unnecessary!

tp commented 1 month ago

Awesome, if this fixes this for you I would consider this solved for now as our latest release works with Xcode 14-16.

We will then update this topic again when there are new changes & guidance from Flutter itself.