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

Update to use LibGit2Sharp v0.24 #1248

Merged
merged 10 commits into from Oct 11, 2017
Merged

Update to use LibGit2Sharp v0.24 #1248

merged 10 commits into from Oct 11, 2017

Conversation

@jcansdale
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Member

ethomson commented Oct 5, 2017

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

@Cliff-Miller
Copy link

Cliff-Miller commented Oct 5, 2017

@jcansdale jcansdale force-pushed the wip/fix-for-mmap branch from a22fd26 to d354ed4 Oct 10, 2017
@jcansdale
Copy link
Collaborator Author

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 5 commits Oct 5, 2017
Also from LibGit2Sharp.NativeBinaries.1.0.129 to 1.0.164.
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.
This is the latest stable release.
Depends on LibGit2Sharp.NativeBinaries v1.0.185.
@jcansdale jcansdale force-pushed the wip/fix-for-mmap branch from 21c82e4 to d620d23 Oct 10, 2017
@jcansdale jcansdale changed the title [wip] Fix for "Failed to mmap. No data written" Update to use LibGit2Sharp v0.24 Oct 10, 2017
@jcansdale jcansdale requested a review from shana Oct 10, 2017
Copy link
Collaborator

shana left a comment

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
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
Copy link
Collaborator Author

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
Copy link
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 Author Collaborator

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
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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.