larpon / QtFirebase

An effort to bring Google's Firebase C++ API to Qt + QML
MIT License
284 stars 83 forks source link

Messaging - iOS Qt 5.12.0/5.12.1 support (QMAKE_IOS_DEPLOYMENT_TARGET = 11.0) #106

Closed Markus87 closed 5 years ago

Markus87 commented 5 years ago

It seems Qt 5.12.0/5.12.1 is built with min version iOS 11.0. If I build my app with QtFirebase/Messaging I get a linker warning that the Qt libs are iOS 11.0 but I work with iOS 10.0.

It is because of this line that changes my complete app to iOS version target 10.0. https://github.com/Larpon/QtFirebase/blob/d0104a5cf3387aac2968a6fa44d58c31d8230b22/qtfirebase_target.pri#L178 Does this mean that QtFirebase requires as minimum iOS 10.0? If so(and it will just work with iOS 11.0) I think this should be a check if QMAKE_IOS_DEPLOYMENT_TARGET < 10.0, if the target is smaller than 10.0 it should generate an error. Otherwise it should do nothing, QMAKE_IOS_DEPLOYMENT_TARGET should be set by the project file of the app that uses this library.

larpon commented 5 years ago

Hi @Markus87 - it's a remain from when Messaging was implemented - where we had to set this for things to work (for some reason I've forgotten). You're super welcome to provide a patch with said check! I believe it definitely should just output an error instead of breaking newer versions :slightly_smiling_face:

adolby commented 5 years ago

Yes, the iOS deployment target was set to 10 as Firebase Cloud Messaging required it to use certain messaging features at one time, which I think was related to the data messages.

I'm not sure if that's still the case or we overlooked it when implementing in the past as it looks like the FCM sample code now includes compatibility code for versions of iOS earlier than 10.

Since Qt 5.12 is setting iOS 11 as a hard minimum it could be conditionally decided as you proposed (this should be possible) or we could remove it entirely, as the Qt documentation indicates that it should be set by the end user:

Expressing supported iOS versions

larpon commented 5 years ago

Yes, I've patched it with a note. QMAKE_IOS_DEPLOYMENT_TARGET should be controlled by the user - not enforced by QtFirebase

Thanks guys!