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

[TIMOB-24057] Adapt native CocoaPods build behavior (2_0_X) #97

Merged
merged 1 commit into from Nov 8, 2016

Conversation

janvennemann
Copy link
Contributor

JIRA: https://jira.appcelerator.org/browse/TIMOB-24057

Changes the CocoaPods build to run with the same build configuration und to use the same build output dir as our main iOS build.

@janvennemann janvennemann changed the title [TIMOB-24057] Adapt native CocoaPods build behavior [TIMOB-24057] Adapt native CocoaPods build behavior (2_0_X) Oct 24, 2016
runPodInstallIfRequired(basedir, function (err) {
if (err) { return callback(err); }
runCocoaPodsBuild(basedir, appDir, sdkType, sdkVersion, minSDKVersion, xcodesettings, function (err, libs, libDir) {
runCocoaPodsBuild(basedir, iosBuilder, function (err, libs, libDir) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I do understand that you rather want to pass the whole iosBuilder object instead some of it's values. But can you ensure this does not effect the performance? Since there are many more values in there, that's probably why Jeff implemented it like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Objects are passed by reference so this doesn't affect performance at all.

if (iosBuilder.simHandle) {
// Manually set the arch for simulator builds since we cannot use -destination
// because the Pods project has no scheme.
args.push('-arch', process.arch === 'x64' ? 'x86_64' : 'i386');
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you test this on both sim and device? We use the following to use archs in other places:

if (xcodeTargetOS === 'iphoneos') {
    // armv7 for all devices. Note that we also could use armv7s or arm64 here
    args.push('armv7-apple-ios' + iosMinVersion);
} else {
    var simArch = process.arch === 'i386' ? 'i386' : 'x86_64';
    args.push(simArch + '-apple-ios' + iosMinVersion);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your code snippet is for swiftc which uses different arch specifiers. The arch argument will only be set when we have a sim handle so it doesn't affect device builds (We do something similar here in our main iOS build)

function runCocoaPodsBuild (basedir, appDir, sdkType, sdkVersion, minSDKVersion, xcodesettings, callback) {
var spawn = require('child_process').spawn,
function runCocoaPodsBuild (basedir, iosBuilder, callback) {
var appDir = iosBuilder.xcodeAppDir,
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually call this object iOSBuilder in the iOS build process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to not capitalize any acronyms or abbreviations, but i'll change it for the sake of consistency 👍

@hansemannn
Copy link
Contributor

CR + FT passed, PR approved! 🚀

@hansemannn hansemannn merged commit 92a9ead into tidev:2_0_X Nov 8, 2016
@janvennemann janvennemann deleted the TIMOB-24057-2_0_X branch March 23, 2017 13:57
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.

None yet

2 participants