Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Conversation

@pulyaevskiy
Copy link
Contributor

@ahmedb
Copy link
Contributor

ahmedb commented Sep 13, 2018

Just a curiosity question: Why is the deployment target being set so low? iOS 6.0 is very, very old and the user base right now is very low ( https://data.apteligent.com/ios/ ).

However, my major concern right now is that using such a low deployment target may prevent us from adopting modern APIs and development practices. Apple deprecates so many APIs between major releases that we may also be at risk of using something they are no longer allowing to be used in apps.

@pulyaevskiy
Copy link
Contributor Author

Short answer: I used what is already set in cloud_firestore plugin.

Long answer: I’m not too familiar with how pods resolve this thing. :)

It looks like without it xcode thinks it supports 5.0 and fails to compile. Note that per Firebase dev answer here https://stackoverflow.com/questions/31076786/error-while-compiling-firebase-based-project-in-objective-c 6.0 is the lowest supported by the sdks.

When I run pod update it prints in the end something like “setting deployment target to 8.0”, do this setting probably works more like minimum supported target?

I’m just thinking out loud though and someone with more experience should weigh in.

Nevertheless this fixes the issue at least.

@pulyaevskiy
Copy link
Contributor Author

@ahmedb

From this page in official docs: https://guides.cocoapods.org/syntax/podspec.html#deployment_target

The minimum deployment targets of the supported platforms.

So I guess it does work as >=.

@ahmedb
Copy link
Contributor

ahmedb commented Sep 13, 2018

Hi @pulyaevskiy,

Thank you for your notes. I hope someone else can chime in too.

From my personal experience (iOS dev since 2009, former iOS instructor, Cocoapods user since 2013), deployment_target is meant to set the deployment target for the iOS framework generated by the Pod. The deployment target setting in XCode sets the minimum system version the framework can run on, and limits the available APIs to only those available between that minimum setting and the latest version of iOS.

In other words, only allow compilation for APIs that are available on all iOS SDK versions from 5.0 up through 12.0. This excludes all APIs that were introduced after iOS 5.0, as well as all APIs that were deprecated since iOS 5.0 was initially released.

When I was looking at the Runner project Flutter generates, I noticed its deployment target is set to 8.0 . Although I personally think this is also too low, I think it would be a good experiment to see if setting the deployment target to 8.0 has the same result as your changes from setting the version to 6.0 .

Unless there is a bug, Cocoapods should print out setting deployment target to 6.0 if the podfile is configured correctly. There are two quick experiments I think might be good to try out to see what is happening.

  1. Manually specify the the platform is iOS to see if that sets the target correctly (The platform line for most podfiles look like this: platform :ios, '9.0')
  2. Set the deployment target to 11.0 to see if it that prints out setting deployment target to 11.0. It may be the case that Cocoapods does not support iOS 6.0 .

This may be of some help as well: CocoaPods/CocoaPods#7314 (comment)

One last comment, Apple sent shockwaves through the iOS community when they released iOS7 and then again when they released Swift with iOS8. iOS7 ushered in the modern iOS UI, and Swift greatly changed the development workflow for iOS (some may say the dust still hasn't settled from either yet.)

Thanks,
Ahmed

@pulyaevskiy
Copy link
Contributor Author

Thanks for thorough explanation! This is really helpful.

As a side note, It'd be ok with me to change it to as high a number as makes sense to people more experienced in iOS development.

But, I also think that changing it in just one Firebase plugin of many won't make a big difference. To address the "deployment target" issue we should probably update all of them at once.

I created this PR to address a different blocking issue - adding cloud_functions to an existing project completely breaks it, it just won't compile otherwise. I'd probably prefer to keep it small so that it can be merged sooner and unblock people's work.

That said, I do see value in your suggestions, however my expertise is simply not enough to tackle such a project. I can only encourage you submit a separate PR with updates to deployment targets across all Firebase plugins. This would at least facilitate discussion with the Flutter team and help to resolve it sooner.

@pulyaevskiy
Copy link
Contributor Author

Also a friendly ping to @kroikie for a chance to get this reviewed. :)

@ahmedb
Copy link
Contributor

ahmedb commented Sep 13, 2018

Thanks @pulyaevskiy . Sounds like a good plan.

@kmorkos
Copy link
Contributor

kmorkos commented Sep 14, 2018

Flutter only supports iOS 8.0 and above so there would be no point in setting the deployment target to anything below 8.0: https://flutter.io/ios-release/#review-xcode-project-settings

@Coeur
Copy link

Coeur commented Sep 18, 2018

@pulyaevskiy, CocoaPods together with a line of code in Swift anywhere (in your app or in a dependency) will mechanically require framework support (or the newer CocoaPods 1.5.x with Swift static lib support) and iOS 8+ support. So in order to have the older targets iOS 7 (or iOS 6) supported, one prerequisite would be to have a pure Objective-C app (including all your pods). That's not a popular audience; it's better to encourage Swift+CocoaPods compatibility and have everybody say goodbye to iOS 7 or older.

The next big upcoming goodbye will be 32 bits apps (both on iOS and macOS).

@pulyaevskiy pulyaevskiy changed the title Set iOS deployment target to 6.0, fixes compilation errors Set iOS deployment target to 8.0, fixes compilation errors Sep 18, 2018
@pulyaevskiy
Copy link
Contributor Author

Flutter only supports iOS 8.0 and above

Makes sense. I pushed an update which bumps it to 8.0.

@pulyaevskiy
Copy link
Contributor Author

pulyaevskiy commented Oct 2, 2018

Just pushed a fix for null pointer error when callable function fails with exception. This was causing my app to crash.

Also friendly reminder for @kroikie . Would be nice to get this merged and remove dependency override finally. :)

@beardo01
Copy link

beardo01 commented Oct 8, 2018

I've stumbled onto this error too. Could we please get this pull request merged @kroikie? Alternatively, @pulyaevskiy, how could I use this now?

Edit: Never mind @pulyaevskiy, I've worked it out. For others that may stumble into the same issue, I've forked the plugins and applied the fix from this pull request in it. You should be able to modify your pubspec.yaml to reference my forked repository for the plugin instead of using the hosted one.

To do so, add the following to your pubspec.yaml:

dependency_overrides:
  cloud_functions:
    git:
       url: https://github.com/beardo01/plugins.git
       ref: f80bb100b208cb07ea3a097c5c3f7cde393d33cb
       path: packages/cloud_functions

Edit: I have updated my forked repo to match the changes that @kroikie has made to master while still keeping the fixes described in this pull request.

Good luck!

@kroikie kroikie self-assigned this Oct 15, 2018
@pulyaevskiy
Copy link
Contributor Author

Just pushed an update to resolve conflicts with master. Anything else I can help with to get this in? @kroikie

@radvansky-tomas
Copy link

dispatch_queue_t callbackQueue;

This error is still present: flutter/flutter#21363

@beardo01
Copy link

dispatch_queue_t callbackQueue;

This error is still present: flutter/flutter#21363

Does using my above recommendation fix that for you?

@radvansky-tomas
Copy link

Yes if I manually go to cloud functions / firestore plugin podspec and change deployment target to 8.0 and reinstall pods it works.

ie. I still dont understand why this is not by default set to at least 8.0

@beardo01
Copy link

@radvansky-tomas Right, gotcha.

Yeah, as you'll see in the conversation about, this pull request sets the deployment target to 8.0, so once this pull request is accepted, it shouldn't be a problem.

spillbeir added a commit to spillbeir/plugins that referenced this pull request Nov 4, 2018
Copy link
Contributor

@kroikie kroikie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for resolving this!

@kroikie kroikie merged commit 5b91e5e into flutter:master Nov 9, 2018
@pulyaevskiy
Copy link
Contributor Author

Awesome! Thanks for merging this.

andreidiaconu pushed a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
* Set iOS deployment target to 6.0, fixes compilation errors

* Fix null pointer error when callable fails with exception
andreidiaconu added a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020
* Set iOS deployment target to 6.0, fixes compilation errors

* Fix null pointer error when callable fails with exception
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants