libgit2 / objective-git

Objective-C bindings to libgit2
MIT License
1.16k stars 280 forks source link

Xcode 7.3 Support #567

Closed phatblat closed 8 years ago

phatblat commented 8 years ago

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.

:warning: 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 facebook/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.

phatblat commented 8 years ago

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 commented 8 years ago

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

phatblat commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

Nice work. 2 cents:

phatblat commented 8 years ago

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 commented 8 years ago

Closing/reopening PR to trigger build.

pietbrauer commented 8 years ago

Travis smartypants :+1:

phatblat commented 8 years ago

The build is finally :green_heart:

pietbrauer commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago
❌  /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 commented 8 years ago

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

pietbrauer commented 8 years ago

Sweet 👌