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

Update to use LibGit2Sharp v0.24 #1248

Merged
merged 10 commits into from Oct 11, 2017

Conversation

Projects
None yet
5 participants
@jcansdale
Contributor

jcansdale commented Oct 5, 2017

This PR updates GHfVS to use the LibGit2Sharp v0.24 (the latest stable version).

This is the last release before a moving to .NET Core compatible library.

It will be the last supported release with the prior architecture; as a result, this release is primarily bugfixes and does not include major new APIs.

https://github.com/libgit2/libgit2sharp/releases/tag/v0.24

There are a number of point #pragma warning disable 0618 statements, which allow us to use a few deprecated methods.

Unfortunately some of the suggested replacement methods expect a Repository object rather than an IRepository. This makes mocking little more awkward. Rather than refactor a bunch of our code and tests, I opted to simply disable and document the warnings.

The impetus to update was due the the following issue:

There have been reports (see #1244) of the following error on the GitHub pane:
Failed to mmap. No data written: Not enough storage is available to process this command.

It looks like this might be related to the version of LibGit2 we're using. See comment by @ethomson here:
https://stackoverflow.com/a/38918860/121348
libgit2/libgit2#3690

Fixes #1244

@jcansdale

This comment has been minimized.

Contributor

jcansdale commented Oct 5, 2017

In this build, I've changed from using LibGit2Sharp 0.22 to 0.23.1 and from LibGit2Sharp.NativeBinaries.1.0.129 to 1.0.164.

GitHub.VisualStudio-2.3.3.42-Debug.zip (newer version below)

I'm going to ask the user if this fixes the issue they were seeing.

@ethomson

This comment has been minimized.

ethomson commented Oct 5, 2017

👍 Let me know if it doesn't work and we can debug further.

@Cliff-Miller

This comment has been minimized.

Cliff-Miller commented Oct 5, 2017

@jcansdale

This comment has been minimized.

Contributor

jcansdale commented Oct 10, 2017

@ethomson Thanks for your offer of help debugging. 😄

One user has tried a test build that updates from LibGit2Sharp 0.22 to 0.23.1 (and NativeBinaries.1.0.129 to 1.0.164). This initial report is encouraging. I'm awaiting feedback from another user.

jcansdale added some commits Oct 5, 2017

Upgrade from LibGit2Sharp 0.22 to 0.23.1
Also from LibGit2Sharp.NativeBinaries.1.0.129 to 1.0.164.
Allow obsolete LibGit2Sharp methods to be called
Hack to allow methods that are obsolete in LibGit2Sharp 0.23.1 to be called.
#pragma warning disable 0612, 0618

This is a temparary workaround.
Update to LibGit2Sharp v0.24
This is the latest stable release.
Depends on LibGit2Sharp.NativeBinaries v1.0.185.

@jcansdale jcansdale changed the title from [wip] Fix for "Failed to mmap. No data written" to Update to use LibGit2Sharp v0.24 Oct 10, 2017

@jcansdale jcansdale requested a review from shana Oct 10, 2017

@shana

Looks good to me! This is going to require thorough testing of all our git usage.

@shana

shana approved these changes Oct 11, 2017

@jcansdale jcansdale merged commit a480116 into master Oct 11, 2017

5 checks passed

GitHub CLA @jcansdale has accepted the GitHub Contributor License Agreement.
Details
VisualStudio Build #8271704 succeeded in 97s
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
jenkins/build_log Jenkins Build Log
Details
@jcansdale

This comment has been minimized.

Contributor

jcansdale commented Oct 11, 2017

Here's a build that includes this PR.
https://ghfvs-installer.github.com/releases/2.3.4.44/GitHub.VisualStudio.vsix

@meaghanlewis

This comment has been minimized.

Contributor

meaghanlewis commented Oct 11, 2017

This looks good to me @jcansdale 👍

Focused testing around fetching, pulling, pushing and checking out branches.

Name = DisplayName = branch.FriendlyName;
Repository = branch.IsRemote ? new LocalRepositoryModel(branch.Remote.Url) : repo;
#pragma warning disable 0618 // TODO: Replace `Branch.Remote` with `Repository.Network.Remotes[branch.RemoteName]`.
var remoteUrl = branch.Remote.Url;

This comment has been minimized.

@jcansdale

jcansdale Oct 18, 2017

Contributor

A bug was introduced here. Remote will be null if IsRemote is false.

jcansdale added a commit that referenced this pull request Oct 18, 2017

Fix null ref when no remote branch
This issue was introduced in #1248.

@jcansdale jcansdale deleted the wip/fix-for-mmap branch Feb 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment