Skip to content

Conversation

@jairbubbles
Copy link
Contributor

@jairbubbles jairbubbles commented Sep 5, 2021

@jairbubbles
Copy link
Contributor Author

Hey @bording @A-Ovchinnikov-mx, I started to look at integrating the new versions but many tests are failing 😅

Some tests like LibGit2Sharp.Tests.RebaseFixture.CanContinueRebase do generate a System.AccessViolationException which is a bit worrying (cc @ethomson)

I started to look at the is_valid_name depreciation but because of libgit2/libgit2#6032 we cannot switch yet to the new version.

@jairbubbles
Copy link
Contributor Author

jairbubbles commented Sep 5, 2021

Ok, thanks to libgit2/git2go#777 I found the culprit for the memory corruption. A new callback create_commit_cb was added (libgit2/libgit2#6016) but it's not at the end of the struct.

EDIT: same problem with the introduction of git_remote_ready_cb (see libgit2/libgit2#6012)
EDIT2: The introcution of writemidx also corrupted the layout (see libgit2/libgit2#5405)

@codecov-commenter
Copy link

codecov-commenter commented Sep 5, 2021

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.80%. Comparing base (6329bea) to head (491d248).
⚠️ Report is 166 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1907   +/-   ##
=======================================
  Coverage   84.80%   84.80%           
=======================================
  Files         231      231           
  Lines        9120     9120           
=======================================
  Hits         7734     7734           
  Misses       1386     1386           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bording
Copy link
Member

bording commented Sep 16, 2021

At some point I guess we'll need to turn those padding fields into actual things, but for now I'll just go ahead and merge this.

@bording bording merged commit 5055fbd into libgit2:master Sep 16, 2021
@bording
Copy link
Member

bording commented Sep 16, 2021

Before I push up a new preview package that uses this, I need to rip out all of the managed HTTPS implementation stuff added in #1618 and go back to using the native OpenSSL stuff. I should have some time this weekend to take a look at that.

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