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

Bump libgit2. #446

Merged
merged 9 commits into from
Mar 6, 2015
Merged

Conversation

robrix
Copy link
Contributor

@robrix robrix commented Mar 6, 2015

Bumps libgit2 to fix a crash enumerating status items (see below).

libgit2 has had some breaking API changes lately. I’ve followed these, and thus this PR also introduces breaking API changes.

Backtrace:

Exception Type:        EXC_ARITHMETIC (SIGFPE)
Exception Codes:       EXC_I386_DIV (divide by zero)

Thread 6 Crashed:: Dispatch queue: com.github.GitHub.GHGitConnection
0   org.libgit2.ObjectiveGit        0x00000001067d5228 hashsig_heap_compare + 376
1   org.libgit2.ObjectiveGit        0x00000001067d504d git_hashsig_compare + 45
2   org.libgit2.ObjectiveGit        0x00000001067c69fb git_diff_find_similar__calc_similarity + 43
3   org.libgit2.ObjectiveGit        0x00000001067c8a92 similarity_measure + 1026
4   org.libgit2.ObjectiveGit        0x00000001067c6f34 git_diff_find_similar + 1284
5   org.libgit2.ObjectiveGit        0x000000010684591a git_status_list_new + 1370
6   org.libgit2.ObjectiveGit        0x0000000106880ba9 -[GTRepository(Status) enumerateFileStatusWithOptions:error:usingBlock:] + 569
7   com.github.GitHub               0x0000000105e03cdd __45-[GHGitConnection statusItemsWithRepository:]_block_invoke_2 + 267
8   org.reactivecocoa.ReactiveCocoa 0x00000001064b9a7c -[RACScheduler performAsCurrentScheduler:] + 375
9   libdispatch.dylib               0x00007fff834a6323 _dispatch_call_block_and_release + 12
10  libdispatch.dylib               0x00007fff834a1c13 _dispatch_client_callout + 8
11  libdispatch.dylib               0x00007fff834a5365 _dispatch_queue_drain + 1100
12  libdispatch.dylib               0x00007fff834a6ecc _dispatch_queue_invoke + 202
13  libdispatch.dylib               0x00007fff834a46b7 _dispatch_root_queue_drain + 463
14  libdispatch.dylib               0x00007fff834b2fe4 _dispatch_worker_thread3 + 91
15  libsystem_pthread.dylib         0x00007fff8563a637 _pthread_wqthread + 729
16  libsystem_pthread.dylib         0x00007fff8563840d start_wqthread + 13

@@ -1 +1 @@
Subproject commit d5712ed2b33a18a4f9417d112bda7813c0570caa
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The full list of changes to libgit2.

@jspahrsummers jspahrsummers self-assigned this Mar 6, 2015
@jspahrsummers
Copy link
Contributor

👍 These changes seem reasonable. The test failures look legit, though.

// fixme: This is a workaround for an issue where `git_filter_list_apply_to_file`
// will not resolve relative paths against the worktree. It should be reverted when
// libgit2 has been updated to resolve that.
NSString *absolutePath = relativePath.absolutePath ? relativePath : [repository.fileURL URLByAppendingPathComponent:relativePath].path;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The relevant libgit2 issue is libgit2/libgit2#2960.

@robrix
Copy link
Contributor Author

robrix commented Mar 6, 2015

Many thanks to @ethomson for tracking down libgit2/libgit2#2960!


return [NSString stringWithUTF8String:string];
NSString *string = [[NSString alloc] initWithBytes:buffer.ptr length:buffer.size encoding:NSUTF8StringEncoding];
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we have an NSString category for this. (If not, maybe we should.)

@jspahrsummers
Copy link
Contributor

1️⃣


return [NSString stringWithUTF8String:string];
return [[NSString alloc] initWithData:[NSData git_dataWithBuffer:&buffer] encoding:NSUTF8StringEncoding];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Collapsed context. We have an NSData method for this, but no NSString one. I feel adding this to the NSString category wouldn’t contribute any significant value, so I’ve just used the NSData one.

Copy link
Contributor

Choose a reason for hiding this comment

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

👌

@robrix
Copy link
Contributor Author

robrix commented Mar 6, 2015

2️⃣

jspahrsummers added a commit that referenced this pull request Mar 6, 2015
@jspahrsummers jspahrsummers merged commit 13c32cb into master Mar 6, 2015
@jspahrsummers jspahrsummers deleted the bump-libgit2-to-fix-status-item-crash branch March 6, 2015 20:01
@jspahrsummers
Copy link
Contributor

@jspahrsummers jspahrsummers removed their assignment May 22, 2015
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