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

Xcode 7.3 Support #567

Merged
merged 12 commits into from
Apr 11, 2016
Merged

Xcode 7.3 Support #567

merged 12 commits into from
Apr 11, 2016

Conversation

phatblat
Copy link
Member

Quick & Nimble

Update Quick & Nimble for Swift 2.2 compatibility. Quick (0.9.2) and Nimble (4.0) contain fixes (Quick/Quick#504 and Quick/Nimble#269) for Swift 2.2.

⚠️ Requires Xcode 7.3. While the framework targets should still build with any version of Xcode 7.0+, the Tests target require 7.3 because these test frameworks won't build with older versions of Swift.

xctool

Due to issues with xctool (such as facebookarchive/xctool#666) with Xcode upgrades, it has been dropped as a build tool.

xcpretty

The cibuild script has been completely overhauled, making it much simpler and more reliable. Instead of xctool, it now uses straight xcodebuild commands with output piped through xcpretty.

When running on Travis, xcpretty-travis-formatter is used to format the the output.

@pietbrauer pietbrauer self-assigned this Mar 24, 2016
@phatblat
Copy link
Member Author

Looks like we've got a valid test failure

  1) -[GTSignatureSpec should_keep_the_git_signature_alive_even_if_the_object_goes_out_of_scope] (ObjectiveGit-MacTests.xctest)
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Test crashed while running.

The git_signature is NULL on the last line of this snippet causing initWithGitSignature: to blow up.

it(@"should keep the git_signature alive even if the object goes out of scope", ^{
    const git_signature *git_signature = NULL;

    {
        GTSignature *testSignature __attribute__((objc_precise_lifetime)) = [[GTSignature alloc] initWithName:name email:email time:time];
        git_signature = testSignature.git_signature;
    }

    GTSignature *testSignature = [[GTSignature alloc] initWithGitSignature:git_signature];

This is different in Xcode 7.3. It works fine in 7.2.1.

@pietbrauer
Copy link
Member

@phatblat I have a patch, will post it in 2 minutes.

@phatblat
Copy link
Member Author

xctool 0.2.8 died in the same spot running the iOS tests as we had trouble with it testing Quick on Xcode 7.3. However, the message is different.

[Info] Collecting info for testables... (1234 ms)
2016-03-24 05:18:41.651 xctool[29278:35046] *** Assertion failure in cpu_type_t CpuTypeForTestBundleAtPath(NSString *__strong)(), /tmp/xctool20160208-1139-sj4ed3/xctool-0.2.8/Common/XCToolUtil.m:914
2016-03-24 05:18:41.654 xctool[29278:35046] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Bundle's executable code doesn't support nor i386, nor x86_64 CPU types. Bundle path: /Users/travis/Library/Developer/Xcode/DerivedData/ObjectiveGitFramework-dggdbcqvjtimfddtjpjrzodjcbls/Build/Products/Debug-iphoneos/ObjectiveGit-iOSTests.xctest, supported cpu types: (

Updating xctool to HEAD to see if that gets past this.

@pietbrauer
Copy link
Member

I released https://github.com/libgit2/objective-git/releases/tag/0.12.0 as carthage build objective-git --no-skip-current is working. This removes a bit of pressure from this PullRequest.

@phatblat
Copy link
Member Author

phatblat commented Apr 2, 2016

The crashing test - GTSignatureSpec should_keep_the_git_signature_alive_even_if_the_object_goes_out_of_scope - was due to __attribute__((objc_precise_lifetime)) not behaving the same in Xcode 7.3 and Apple LLVM version 7.3.0 (clang-703.0.29). The git_signature was NULL when the 2nd GTSignature was trying to initialize. Removing the attribute causes the test to pass and not crash.

After that, GTOIDSpec should_keep_the_git_oid_alive_even_if_the_object_goes_out_of_scope was failing (no crash). Similarly, removing __attribute__((objc_precise_lifetime)) caused that test to pass as well.

I don't know if I fully understand __attribute__((objc_returns_inner_pointer)), but it sounds like it tells ARC to not release the containing (GT*) object when one of these "interior" pointers is still in scope. Adding __attribute__((objc_precise_lifetime)) sounds like it disables this behavior.

I've removed these two instances of __attribute__((objc_precise_lifetime)) for now.

@phatblat
Copy link
Member Author

phatblat commented Apr 3, 2016

Something is going wrong in cibuild. I've tested xctool with these changes and it runs fine and the tests pass, but when executed from the cibuild script it's throwing this assertion:

2016-04-03 11:18:03.349 xctool[83039:241514] *** Assertion failure in -[SimulatorInfo systemRootForSimulatedSdk], /tmp/xctool20160323-7546-1p54dmr/xctool/xctool/SimulatorWrapper/SimulatorInfo.m:280
2016-04-03 11:18:03.349 xctool[83039:241514] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Platform 'iphoneos' is not yet supported.'

It looks as if the sdkflags are never getting set. I'm guessing the xctool.awk script is returning 0. I'm going to try simplifying the script to work around this.

@pietbrauer
Copy link
Member

Nice work. 2 cents:

  • Rebasing would be nice
  • I was debating why we still use xctool and not change to xcodebuild and xcpretty which comes standard with Travis.

@phatblat
Copy link
Member Author

phatblat commented Apr 4, 2016

Sure, I can clean this up and rebase it.

As for xcpretty, I'm game. I ran into some trouble with it and Xcode 7.0 last fall but with all the trouble we've had with xctool, I think it's worth looking into again.

@phatblat phatblat changed the title Update Quick & Nimble for Swift 2.2 compatibility Xcode 7.3 Support Apr 5, 2016
- Quick 0.9.1+
- Nimble 3.2.0+

These resolve many deprecation warnings with Swift 2.2 in Xcode 7.3
- Replaced xctool with xcodebuild + xcpretty
- Removed unused cruft and awk scripts
Also enabled fast_finish for builds
@phatblat
Copy link
Member Author

phatblat commented Apr 5, 2016

Closing/reopening PR to trigger build.

@phatblat phatblat closed this Apr 5, 2016
@phatblat phatblat reopened this Apr 5, 2016
@@ -1,14 +1,17 @@
osx_image: xcode7
language: objective-c
matrix:
fast_finish: true:
Copy link
Member

Choose a reason for hiding this comment

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

I think this is why Travis is not starting the build true: should be true

Copy link
Member Author

Choose a reason for hiding this comment

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

oops

@pietbrauer
Copy link
Member

Travis smartypants 👍

echo "*** Building and testing $SCHEME..."
echo

xcodebuild -workspace "$XCWORKSPACE" \
Copy link
Member

Choose a reason for hiding this comment

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

From the xcpretty docs:

$ set -o pipefail && xcodebuild [flags] | xcpretty
#
# OR
#
$ xcodebuild [flags] | xcpretty && exit ${PIPESTATUS[0]}

@phatblat
Copy link
Member Author

phatblat commented Apr 6, 2016

The build is finally 💚

@pietbrauer
Copy link
Member

Looks good but waiting for the merge since Nimble 4.0.0 was just released and I suspect a Quick release with Xcode 7.3 support too.

@phatblat
Copy link
Member Author

phatblat commented Apr 6, 2016

Fine by me. I'd rather be using released versions.

Quick may take a couple days because I just submitted Quick/Nimble#278 to unblock Quick/Quick#507

@phatblat
Copy link
Member Author

phatblat commented Apr 8, 2016

❌  /Users/travis/build/libgit2/objective-git/ObjectiveGitTests/SwiftSpec.swift:11:8: module file's minimum deployment target is ios9.3 v9.3: /Users/travis/Library/Developer/Xcode/DerivedData/ObjectiveGitFramework-dggdbcqvjtimfddtjpjrzodjcbls/Build/Products/Debug-iphonesimulator/Quick.framework/Modules/Quick.swiftmodule/i386.swiftmodule
import Quick
       ^

It's counter-intuitive, but I think that build failure basically means Xcode is confused because the deployment target isn't 8.0.

@phatblat
Copy link
Member Author

Quick and Nimble have been updated. This PR is ready to be merged! 🚀

@pietbrauer pietbrauer merged commit 71907ef into libgit2:master Apr 11, 2016
@pietbrauer
Copy link
Member

Sweet 👌

@phatblat phatblat deleted the ben/quick-nimble branch April 11, 2016 14:50
@pietbrauer pietbrauer mentioned this pull request Apr 15, 2016
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.

2 participants