8000 Update to use LibGit2Sharp v0.24 by jcansdale · Pull Request #1248 · github/VisualStudio · GitHub
[go: up one dir, main page]

Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

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 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
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 via email

@jcansdale
Copy link
Collaborator Author

@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.

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 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 October 10, 2017 14:28
shana
shana previously approved these changes Oct 10, 2017
Copy link
Contributor
@shana shana left a comment

Choose a reason for hiding this comment

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

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

@jcansdale jcansdale merged commit a480116 into master Oct 11, 2017
@jcansdale
Copy link
Collaborator Author

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

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;
Copy link
Collaborator Author
@jcansdale jcansdale Oct 18, 2017

Choose a reason for hiding this comment

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

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 February 7, 2018 09:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errors opening a pull request: "Failed to nmap..."
5 participants
0