ios: Add workaround for package:firebase_messaging when building with Xcode 16 #958

rajveermalviya · 2024-09-19T21:18:23Z

Without this workaround iOS build currently fails with the following error:

Lexical or Preprocessor Issue (Xcode): Include of non-modular header inside framework module 'firebase_messaging FLTFirebaseMessagingPlugin':
'/Users/rajveer/Projects/zulip-flutter/ios/Pods/Headers/Public/Firebase/Firebase.h'
/Users/rajveer/.pub-cache/hosted/pub.dev/firebase_messaging-15.0.4/ios/Classes/FLTFirebaseMessagingPlugin.h:11:8

Upstream issue and the workaround comment:
firebase/flutterfire#13323
firebase/flutterfire#13323 (comment)

Docs: https://developer.apple.com/documentation/xcode/build-settings-reference#Allow-Non-modular-Includes-In-Framework-Modules

The documentation states that enabling this setting may result in issues later on, which is why this workaround should only be temporary until firebase_messaging is patched with the correct fix.

rajveermalviya · 2024-09-19T21:25:59Z

Chat thread:
https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Can't.20compile.20for.20iOS.20on.20macOS.2015.2E0.20.28latest.29.20and.20Xcode.2016.2E0/near/1944045

chrisbobbe · 2024-09-19T23:15:43Z

LGTM, and I've tested on Xcode Version 15.4 (15F31d) and it works there too.

An alternative that should also work, and didn't break the build in my testing on Xcode 15, is to add a line

CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES = YES

in ios/Flutter/Debug.xcconfig and ios/Flutter/Release.xcconfig, and leave project.pbxproj untouched. This might be nicer, because the project.pbxproj is hard to read, and there's a lot in there. Putting this setting in an uncrowded file, with a helpful comment saying it's a temporary workaround for a specific issue, seems helpful.

The Xcode GUI lets you see how the final value for this setting is resolved.

Here's with the PR as-is, with the additions in project.pbxproj:

And with my proposal:

In my proposal, the prevailing "YES" value comes from the config-file column, instead of the target ("Runner") column.

I wasn't sure which of Debug.xcconfig and Release.xcconfig would control the "Profile" configuration, or if neither of them would, and I found experimentally that it's Release.xcconfig. I'm not finding the place in the Xcode GUI that tells me that, but this looks like the spot in project.pbxproj:

		249021D4217E4FDB00AE95B9 /* Profile */ = {
			isa = XCBuildConfiguration;
			baseConfigurationReference = 7AFA3C8E1D35360C0083082E /* Release.xcconfig */;

gnprice · 2024-09-20T22:22:51Z

ios/Flutter/Debug.xcconfig and ios/Flutter/Release.xcconfig, and leave project.pbxproj untouched.

Neat, I like that option. @rajveermalviya would you try that and confirm that it works for you?

It looks like those files also accept comments, with //. (See Generated.xcconfig in the same directory for an example.) That'd be good for mentioning that it's a workaround which should be temporary, and linking to the chat thread and that upstream issue firebase/flutterfire#13323 .

rajveermalviya · 2024-09-23T16:24:49Z

Thanks for the reviews @chrisbobbe and @gnprice!

Yes, adding the flag to ios/Flutter/{Debug.xcconfig, Release.xcconfig} does result in a successfull build as well.
I've pushed a new revision which does that via Zulip.xcconfig (new file) and #include-ing that in both Debug.xcconfig and Release.xcconfig.

PIG208 · 2024-09-23T18:32:44Z

ios/Flutter/Zulip.xcconfig

+// This flag is added to workaround the iOS build failing
+// on Xcode 16, remove it when `package:firebase_messaging`
+// is patched with a fix to build successfully on Xcode 16.
+//


nit: extra empty line

PIG208 · 2024-09-23T18:36:13Z

I don't have an Xcode installation at the moment to test this. But the change looks clean to me.

rajveermalviya · 2024-09-23T21:00:14Z

Thanks for the review @PIG208, pushed a new revision.

Additionally, added references for Zulip.xcconfig so that it becomes visible in XCode (without them it's not).

gnprice

Thanks! I like this structure. Small comment tweaks below.

Then this is otherwise ready for merge, so please update the commit message to a final/mergeable version.

gnprice · 2024-09-24T01:08:11Z

ios/Flutter/Zulip.xcconfig

+// Configuration settings file format documentation can be found at:
+// https://help.apple.com/xcode/#/dev745c5c974


Helpful, thanks for including this link.

gnprice · 2024-09-24T01:11:12Z

ios/Flutter/Zulip.xcconfig

+// Configuration settings file format documentation can be found at:
+// https://help.apple.com/xcode/#/dev745c5c974
+
+// TODO(firebase/flutterfire#13323): remove this flag


I guess we've talked about having a tool to scan for TODO comments that point to closed issues, and when we do it could potentially understand this syntax for external issues. So it's fine to include it.

But let's also include an actual link. That will make it nice and easy for someone who happens across this a month from now or a year from now to quickly check on the state of the upstream issue, even if we haven't built that potential tool by then.

gnprice · 2024-09-24T01:16:55Z

ios/Flutter/Zulip.xcconfig

+
+// TODO(firebase/flutterfire#13323): remove this flag
+//
+// This flag is added to workaround the iOS build failing


nit: the verb is "work around", and "workaround" is its noun form:

Suggested change

// This flag is added to workaround the iOS build failing

// This flag is added to work around the iOS build failing

Just like other verb phrases made of a verb plus preposition, where they're run together to make a noun:
"to log in" / "this login"
"to set up" / "this setup"
"to clean up" / "this cleanup"

(In each case the two versions are pronounced differently, too — the stress moves from the end in the verb form to the beginning in the noun form. That probably contributes to making the difference salient for me.)

gnprice · 2024-09-24T01:19:27Z

ios/Flutter/Zulip.xcconfig

+// This flag is added to workaround the iOS build failing
+// on Xcode 16, remove it when `package:firebase_messaging`
+// is patched with a fix to build successfully on Xcode 16.


Suggested change

// This flag is added to workaround the iOS build failing

// on Xcode 16, remove it when `package:firebase_messaging`

// is patched with a fix to build successfully on Xcode 16.

// This flag is added to work around the iOS build failing

// on Xcode 16. Remove it when `package:firebase_messaging`

// is patched with a fix to build successfully on Xcode 16:

// https://github.com/firebase/flutterfire/issues/13323

rajveermalviya · 2024-09-24T12:10:27Z

Thanks for the review @gnprice! Pushed a new revision, PTAL.

… Xcode 16 Without this workaround iOS build currently fails with the following error: Lexical or Preprocessor Issue (Xcode): Include of non-modular header inside framework module 'firebase_messaging FLTFirebaseMessagingPlugin': '/Users/rajveer/Projects/zulip-flutter/ios/Pods/Headers/Public/Firebase/Firebase.h' /Users/rajveer/.pub-cache/hosted/pub.dev/firebase_messaging-15.0.4/ios/Classes/FLTFirebaseMessagingPlugin.h:11:8 Upstream issue and the workaround comment: firebase/flutterfire#13323 firebase/flutterfire#13323 (comment) Docs: https://developer.apple.com/documentation/xcode/build-settings-reference#Allow-Non-modular-Includes-In-Framework-Modules The documentation states that enabling this setting may result in issues later on, which is why this workaround should only be temporary until firebase_messaging is patched with the correct fix.

gnprice · 2024-09-24T18:30:22Z

Thanks! Looks good — merging.

This reverts part of commit a9b2016 (zulip#958), keeping the `Zulip.xcconfig` file and its references. The fix was released in version `3.6.0` of `package:firebase_core` and version `15.1.3` of `package:firebase_messaging`: firebase/flutterfire@d7d2d4b We have since then updated Firebase dependencies 3 times: zulip@24215f6 zulip@1abb0a0 zulip@3ba9985

rajveermalviya added maintainer review PR ready for review by Zulip maintainers integration review Added by maintainers when PR may be ready for integration labels Sep 19, 2024

rajveermalviya force-pushed the pr-fix-xcode-16 branch 2 times, most recently from 8aab93a to 7607989 Compare September 23, 2024 16:22

PIG208 reviewed Sep 23, 2024

View reviewed changes

rajveermalviya force-pushed the pr-fix-xcode-16 branch 2 times, most recently from b996f21 to 2394754 Compare September 23, 2024 20:57

rajveermalviya requested review from chrisbobbe and gnprice September 23, 2024 21:00

gnprice reviewed Sep 24, 2024

View reviewed changes

rajveermalviya force-pushed the pr-fix-xcode-16 branch from 2394754 to 7888db0 Compare September 24, 2024 12:08

rajveermalviya requested a review from gnprice September 24, 2024 12:10

gnprice force-pushed the pr-fix-xcode-16 branch from 7888db0 to a9b2016 Compare September 24, 2024 18:30

gnprice merged commit a9b2016 into zulip:main Sep 24, 2024
1 check passed

rajveermalviya deleted the pr-fix-xcode-16 branch September 27, 2024 15:30

gnprice mentioned this pull request Jan 30, 2025

notif: Use associated account as initial account; if opened from cold start #1240

Merged

gnprice mentioned this pull request Mar 20, 2025

Revert ios build workaround for Firebase packages #1388

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ios: Add workaround for package:firebase_messaging when building with Xcode 16 #958

ios: Add workaround for package:firebase_messaging when building with Xcode 16 #958

Uh oh!

rajveermalviya commented Sep 19, 2024

Uh oh!

rajveermalviya commented Sep 19, 2024

Uh oh!

chrisbobbe commented Sep 19, 2024 •

edited

Loading

Uh oh!

gnprice commented Sep 20, 2024

Uh oh!

rajveermalviya commented Sep 23, 2024

Uh oh!

PIG208 Sep 23, 2024

Uh oh!

PIG208 commented Sep 23, 2024

Uh oh!

rajveermalviya commented Sep 23, 2024

Uh oh!

gnprice left a comment

Uh oh!

gnprice Sep 24, 2024

Uh oh!

gnprice Sep 24, 2024

Uh oh!

gnprice Sep 24, 2024

Uh oh!

gnprice Sep 24, 2024

Uh oh!

rajveermalviya commented Sep 24, 2024

Uh oh!

gnprice commented Sep 24, 2024

Uh oh!

Uh oh!

Uh oh!

		// Configuration settings file format documentation can be found at:
		// https://help.apple.com/xcode/#/dev745c5c974

	// This flag is added to workaround the iOS build failing
	// This flag is added to work around the iOS build failing

-// This flag is added to workaround the iOS build failing
-// on Xcode 16, remove it when `package:firebase_messaging`
-// is patched with a fix to build successfully on Xcode 16.
+// This flag is added to work around the iOS build failing
+// on Xcode 16.  Remove it when `package:firebase_messaging`
+// is patched with a fix to build successfully on Xcode 16:
+//   https://github.com/firebase/flutterfire/issues/13323

ios: Add workaround for package:firebase_messaging when building with Xcode 16 #958

ios: Add workaround for package:firebase_messaging when building with Xcode 16 #958

Uh oh!

Conversation

rajveermalviya commented Sep 19, 2024

Uh oh!

rajveermalviya commented Sep 19, 2024

Uh oh!

chrisbobbe commented Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnprice commented Sep 20, 2024

Uh oh!

rajveermalviya commented Sep 23, 2024

Uh oh!

PIG208 Sep 23, 2024

Choose a reason for hiding this comment

Uh oh!

PIG208 commented Sep 23, 2024

Uh oh!

rajveermalviya commented Sep 23, 2024

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

gnprice Sep 24, 2024

Choose a reason for hiding this comment

Uh oh!

gnprice Sep 24, 2024

Choose a reason for hiding this comment

Uh oh!

gnprice Sep 24, 2024

Choose a reason for hiding this comment

Uh oh!

gnprice Sep 24, 2024

Choose a reason for hiding this comment

Uh oh!

rajveermalviya commented Sep 24, 2024

Uh oh!

gnprice commented Sep 24, 2024

Uh oh!

Uh oh!

Uh oh!

chrisbobbe commented Sep 19, 2024 •

edited

Loading