Skip to content

Update libgit2 to 0.24.0 #564

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

Merged
merged 1 commit into from
Mar 24, 2016
Merged

Update libgit2 to 0.24.0 #564

merged 1 commit into from
Mar 24, 2016

Conversation

pietbrauer
Copy link
Member

This updates libgit2 to the latest release v0.24.0:

I had to disable GIT_OPT_ENABLE_STRICT_OBJECT_CREATION in these tests.

  • [GTRemotePushSpec pushing__to_remote__can_push_one_commit]
  • [GTRemoteSpec network_operations____GTRepository_fetchRemote_withOptions_error_progress____brings_in_new_commits]
  • [GTRepositoryCommitting can_create_commits]
  • [GTRepositoryPullSpec pull__from_remote__fails_to_merge_when_there_is_a_conflict]
  • [GTTreeBuilderSpec GTTreeBuilder_building__should_write_new_blobs_when_the_tree_is_written]

Now I am unsure if I should stick to permanently disabling this or just silence it for the tests.

@pietbrauer
Copy link
Member Author

This is where it was added: libgit2/libgit2#3633

I am unsure why our additions to the tree are considered invalid. The OID is generated using +[GTOID OIDByHashingData:type:error:], file mode is valid is set to GTFileModeBlob and the file name is valid too.

Maybe @ethomson has an idea?

@pietbrauer pietbrauer force-pushed the piet/update-libgit-0.24.0 branch from cfe733b to 31354b1 Compare March 23, 2016 15:43
@pietbrauer pietbrauer changed the title [WIP] Update libgit2 to 0.24.0 Update libgit2 to 0.24.0 Mar 23, 2016
@pietbrauer
Copy link
Member Author

Ready for review, once this is merged I will do a new release.

@ethomson
Copy link
Member

Well, for [GTTreeBuilderSpec GTTreeBuilder_building__should_write_new_blobs_when_the_tree_is_written], for example, the process is:

  1. Hash the blob (in [GTTreeBuilder addEntryWithData])
  2. Add to the pending data list
  3. Add the tree entry with the blob's ID / filename / mode, by calling git_treebuilder_insert

You're not actually writing the blob, though, until [GTTreeBuilder writeTree], which will run [GTTreeBuilder writePendingData] and actually flush the pending list to blobs.

When you call git_treebuilder_insert (in step 3), it's going to check that a blob exists in the database. It doesn't yet. So I expect this failure.

I don't see why Objective C isn't simply adding the blob to the ODB in [GTTreeBuilder addEntryWithData]...? You're spending a lot of memory and CPU to avoid doing so... Unless there's a really good reason to avoid it, I would make [GTTreeBuilder addEntryWithData] add the given data to the ODB, then call addEntryWithOID with the new OID, and get rid of the pending data list entirely.

(Note, please correct my notation if I'm putting brackets in the wrong place, I'm very much a noob when it comes to Objective C.)

I only looked at this one test - it's possible that the other tests have the same root cause (or not).

I was hoping to actually step through these with a debugger, but I have failed to do so as of yet. :(

Do you have or can you point me to a quick getting started for this, for somebody who is not an Objective C developer and maybe doesn't understand how frameworks work? The README suggested that as long as I was running Xcode 7 that I would be set, so I ran script/bootstrap, but my ObjectiveGit-MacTests build fails to include the frameworks. (SwiftSpec.swift: No such module 'Nimble').

I also tried building them with carthage and dragging the resultant frameworks in with no avail.

@pietbrauer
Copy link
Member Author

Oh, I think I have to reread this when I am more awake 😆

You should be able to run the tests if all the submodules are there (including Nimble). Did you maybe open the .xcodeproj instead of the .xcworkspace?

@ethomson
Copy link
Member

Yes, that's exactly what I did. Thanks for the tip. :)

@pietbrauer pietbrauer force-pushed the piet/update-libgit-0.24.0 branch from 31354b1 to 4f39f5f Compare March 24, 2016 02:54
@pietbrauer
Copy link
Member Author

Reworked this to include the changes made in #566 so the tests pass now. Once this is merged I will release 0.12.0 and we can all update to Xcode 7.3 😁

/cc @joshaber @phatblat

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

Looks good to me.

@phatblat phatblat merged commit f06bbdd into master Mar 24, 2016
@phatblat
Copy link
Member

At least Nimble will need an update to 3.2 for basic Swift 2.2/Xcode 7.3 compatibility. However, there are a ton of deprecation warnings that I've only just fixed tonight in Quick and Nimble. I'll submit a PR to update these to the commits in master since they haven't been included in a release yet.

@phatblat phatblat deleted the piet/update-libgit-0.24.0 branch March 24, 2016 04:37
@pietbrauer
Copy link
Member Author

Cool, but this is not blocking the release right?

@phatblat
Copy link
Member

No. It's just blocking running the tests on Xcode 7.3. I had Travis failures from building and testing Quick due to the spurious warning output. Apparently if the build writes more than 4MB of logs it's considered a failure.

@pietbrauer
Copy link
Member Author

Fun fact: it is blocking the release as Carthage builds Nimble & Quick. 😆

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.

3 participants