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

adds 64 bitness to calabash.framework #37

Merged
merged 14 commits into from
Mar 6, 2014

Conversation

jmoody
Copy link
Member

@jmoody jmoody commented Mar 5, 2014

Motivation

calabash/calabash-ios#313 - thanks @daudt

_IMPORTANT_ when merging you must also merge calabash/calabash-ios#336

The target iOS version was iOS 4 - a version that we dropped support for some time ago.

This pull request sets the minimum iOS version at 5.1.1 and builds the calabash.framework with an arm64 and x86_64 slice.

In order to upgrade the min iOS version and include 64-bit slices, I needed to implement a new strategy for building the framework.

I spent a fair amount of time exploring the next Xcode version and the consequences of bumping the min iOS version. All seems well. I was able to execute tests using that Xcode version against iOS 5.1.1, 6.1, and 7.0.* devices as well as on the test cloud. There were hiccups testing against the simulator, but those hiccups exist independent of the state of the calabash.framework.

Testing

Xcode 5.0.2

Briar

NO_LAUNCH=1 ==> Manually Launching ==> UIA not available
  • iPad 1 iOS 5.1.1
  • iPhone Simulator iOS 6.1
NO_LAUNCH=0 ==> Launching with Instruments ==> ==> UIA available
  • iPad 4 iOS 7.0.3
  • iPhone Simulator iOS 7
  • iPhone Simulator iOS 7 64-bit

XTC

Apple iPhone 4S (5.1.1)
Apple iPad 2 (6.1.3)
Apple iPad 4th Gen (7.0.4)
Apple iPad Mini Retina (7.0.4)
Apple iPhone 4S (6.1.3)
Apple iPhone 4S (7.0.4)
Apple iPhone 5 (7.0.4)

ruby version smoke testing

LPSimpleExample

@jmoody jmoody mentioned this pull request Mar 5, 2014
@@ -20,7 +20,7 @@
#define ISO_TIME_WITH_TIMEZONE_FORMAT ISO_TIME_FORMAT @"Z"
//printf formats.
#define ISO_TIMEZONE_UTC_FORMAT @"Z"
#define ISO_TIMEZONE_OFFSET_FORMAT @"%+.2d%.2d"
#define ISO_TIMEZONE_OFFSET_FORMAT @"%+.2ld%.2ld"
Copy link
Member

Choose a reason for hiding this comment

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

What is this change: @jmoody

Copy link
Member Author

Choose a reason for hiding this comment

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

@krukow

see my comment on the pull request.

In this case, I have to use %ld because I want the leading zeros.

@jmoody
Copy link
Member Author

jmoody commented Mar 5, 2014

@krukow

re: ObjC autoboxing + format strings

Short answer: this is part of the port from 32-bit to 32-bit + 64-bit

Long answer

Xcode 5.1 introduces and/or enforces more type checking for primitive types like NSUInteger and NSInteger. This is because in some environments NSInteger evals to 32-bit (int) and in other environments it will eval to 64-bit (long int).

# [NSIndexPath row] and [NSIndexPath section] are NSInteger
# the %d is for 32-bit integers
# so the following throws 2 warnings - NSInteger does not fit in %d
return [NSString stringWithFormat:@"%@%d,%d", fm, [ip row], [ip section]];

# option 1 - type cast to long and use %ld
return [NSString stringWithFormat:@"%@%ld,%ld", fm, (long)[ip row], (long)[ip section]];

# option 2 - use autoboxing to convert to NSNumber and use %@ at the format directive
return [NSString stringWithFormat:@"%@%@,%@", fm, @([ip row]), @([ip section])];

I prefer option 2 because:

  • it does not require sprinkling the code with (long) casts which will eventually become useless (when we are all on 64-bit hardware) and
  • it reduces the chance of errors - let the runtime figure out how to interpret NSNumber - I can't be bothered to remember %lu, %ld, %d.

This is what I have been doing my own code.

@krukow
Copy link
Member

krukow commented Mar 5, 2014

got it - just me not keeping up. Go for it.

jmoody added a commit that referenced this pull request Mar 6, 2014
@jmoody jmoody merged commit 78d8e29 into calabash:master Mar 6, 2014
@jmoody jmoody deleted the feat/0.9.x-min-target-5.1.1-issue-313 branch March 6, 2014 20:03
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