Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix The deployment target used to find the right simulator to compile the binaries #73

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AliSoftware
Copy link

Context

  • cocoapods: 1.6.0
  • cocoapods-binary: master

The issue

Currently, when I try to use cocoapods-binary on a project which uses platform :ios, '11.0' at the top of its Podfile, pod install fails during the prebuild of binary pods because Fourflusher finds the older simulator matching iOS 8.0, and then xcodebuild (Xcode 10.2 on our CircleCI) fails with:

xcodebuild: error: Unable to find a destination matching the provided destination specifier:
		{ id:85D2D947-6AFF-469A-9C95-FFBFFB069294 }

	Available destinations for the "AFNetworking" scheme:
		{ platform:iOS Simulator, id:C0DA41D1-EA6C-40A6-AF82-E2D38C2A4503, OS:11.4, name:iPad (5th generation) }
_[...snip...]_
		{ platform:iOS Simulator, id:3D5639A8-8F01-44E3-A3DE-1B4B2C5B3E58, OS:12.2, name:iPhone Xʀ }

Where that id 85D2D947-6AFF-469A-9C95-FFBFFB069294 corresponds to a simulator with the iOS 10.3 runtime when I list xcrun simctl on said CI.

The reason

This is due to the fact that the code uses the PodTarget.platform.deployment_target value to determine the minimum deployment target to pass to Fourflusher::SimControl.new.destination(:oldest, platform, deployment_target)

As a matter of fact, target.platform.deployment_target corresponds to the Pod's minimum deployment target as defined in its .podspec (or more precisely, the max between '8.0' and the value in the podspec, as seen here in CocoaPods' source).

But what is finally used as the IPHONEOS_DEPLOYMENT_TARGET Build Setting for the Pod targets generated by CocoaPods in the Pods.xcodeproj in the end, is the platform.deployment_target of the Aggregate Targets – as seen here in CocoaPods source – and not the PodTarget (whose platform reflects the one in their spec, not the one for integration). So this is the minimum version we need to use to build the pods

The current behavior means that if we have any pod which have a s.platforms.ios = '8.0' in their podspec, cocoapods-binary will ask FourFlusher for a simulator with iOS 8.0 minimum, find for example a 10.3 simulator, but then will fail to compile the Pod target due to its IPHONEOS_DEPLOYMENT_TARGET=11.0 set on the Pod target – because we used platform :ios, '11.0' at the top of our Podfile

The fix

We need to rely on the platform.deployment_target of the aggregate target (which will then be used by CocoaPods to set the IPHONEOS_DEPLOYMENT_TARGET build setting in each pod's .xcconfig file) to determine the right simulator to use for compilation and especially the minimum iOS version that this simulator needs to support.

My solution is to use the maximum value of all the aggregate targets that include the Pod being compiled. That way, if Pod A is integrated in our Podfile in both in a target specifying platform :ios, '10.0' and another target specifying platform :ios, '11.0', then we'd need to compile pod A using an iOS 11 simulator.

Alternative

We could also instead just use the latest iOS simulator available. After all, what's important for compilation is the SDK version. So if we're using Xcode 10.2 to compile, we should use the latest simulator, one running with iOS SDK 12, because what we care about is the iOS SDK used to compile.

Sadly, after a quick glance at Fourflusher, it seems there's no filter :newest to get the most recent simulator, only :oldest.

@rogerluan
Copy link

@leavez any plans to incorporate this PR? It'd help close #101 and #97

@OliverPearmain-Triller
Copy link

OliverPearmain-Triller commented Nov 16, 2020

Certain members of our team were having silent pod install failures (ala #97) and this PR has proved to resolve the issue. @leavez please get this merged 🙏 🙏 🙏 so we don't have to rely on the fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants