8000 Move from LibGit2Sharp v0.24.0 to v0.23.1 by jcansdale · Pull Request #1316 · 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.

Move from LibGit2Sharp v0.24.0 to v0.23.1 #1316

Merged
merged 2 commits into from
Nov 17, 2017
Merged

Move from LibGit2Sharp v0.24.0 to v0.23.1 #1316

merged 2 commits into from
Nov 17, 2017

Conversation

jcansdale
Copy link
Collaborator
@jcansdale jcansdale commented Nov 14, 2017 •
edited
Loading

We upgraded from LibGit2Sharp v0.22 to v0.24 in PR #1248. This upgrade was in order to fix #1244 (the Failed to mmap issue).

Unfortunately, LibGit2Sharp v2.4.0 sometimes throws an AccessViolationException when RetrieveStatus is called (see #1315). This appears to still be an issue in v0.25.0-preview-0033.

Moving to LibGit2Sharp v2.3.1 because it appears to be more stable than v0.24 and fixes both #1244 and #1315.

NOTE: I've left in some of the dependentAssembly and NuGetPackageImportStamp junk that NuGet likes to add. Hopefully this will make the next time we update NuGet packages cleaner. We don't package the App.config files, so this shouldn't make any difference to the extension.

Workaround for: #1306

@jcansdale jcansdale requested a review from grokys November 14, 2017 12:16
@jcansdale jcansdale mentioned this pull request Nov 15, 2017 < 10000 /a>
Closed
shana added a commit that referenced this pull request Nov 15, 2017
@shana
Move from LibGit2Sharp v2.4.0 to v2.3.1
a934c06 
LibGit2Sharp v2.4.0 sometimes throws AccessViolationException on RetrieveStatus.

Cherry-picked from caaa967 (PR #1316)
@shana shana mentioned this pull request Nov 15, 2017
Merged
shana added a commit that referenced this pull request Nov 15, 2017
@shana
Move from LibGit2Sharp v2.4.0 to v2.3.1
f043bb0 
LibGit2Sharp v2.4.0 sometimes throws AccessViolationException on RetrieveStatus.

Cherry-picked from caaa967 (PR #1316)
shana added a commit that referenced this pull request Nov 15, 2017
@shana
Move from LibGit2Sharp v2.4.0 to v2.3.1
98203ba 
LibGit2Sharp v2.4.0 sometimes throws AccessViolationException on RetrieveStatus.

Cherry-picked from caaa967 (PR #1316)
shana
shana suggested changes Nov 15, 2017
View reviewed changes
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.

There's a bunch of uneeded changes that VS probably automatically did to the app.config and csproj files, which makes this reeeeally hard to cherry-pick properly. These changes should probably go in so that VS doesn't keep doing them, but they should be done in a separate cosmetic PR in a commit marked as such so that they're isolated and don't pollute the history.

src/DesignTimeStyleHelper/App.config Outdated
<dependentAssembly>
<assemblyIdentity name="Microsoft.VisualStudio.Validation" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
<bindingRedirect oldVersion="0.0.0.0-14.0.0.0" newVersion="14.0.0.0" />
</dependentAssembly>
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes shouldn't be in this PR

src/GitHub.App/GitHub.App.csproj Outdated
@@ -16,6 +16,8 @@
<CodeAnalysisRuleSet>..\common\GitHubVS.ruleset</CodeAnalysisRuleSet>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<CodeAnalysisIgnoreGeneratedCode>true</CodeAnalysisIgnoreGeneratedCode>
<NuGetPackageImportStamp>
</NuGetPackageImportStamp>
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes shouldn't be here

src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj Outdated
@@ -15,6 +15,8 @@
<CodeAnalysisRuleSet>..\common\GitHubVS.ruleset</CodeAnalysisRuleSet>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<CodeAnalysisIgnoreGeneratedCode>true</CodeAnalysisIgnoreGeneratedCode>
<NuGetPackageImportStamp>
</NuGetPackageImportStamp>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj Outdated
<None Include="packages.config" />
<None Include="packages.config">
<SubType>Designer</SubType>
</None>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

src/GitHub.Exports/GitHub.Exports.csproj Outdated
@@ -15,6 +15,8 @@
<CodeAnalysisRuleSet>..\common\GitHubVS.ruleset</CodeAnalysisRuleSet>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<CodeAnalysisIgnoreGeneratedCode>true</CodeAnalysisIgnoreGeneratedCode>
<NuGetPackageImportStamp>
</NuGetPackageImportStamp>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj Outdated
@@ -17,6 +17,8 @@
<CodeAnalysisRuleSet>..\common\GitHubVS.ruleset</CodeAnalysisRuleSet>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<CodeAnalysisIgnoreGeneratedCode>true</CodeAnalysisIgnoreGeneratedCode>
<NuGetPackageImportStamp>
</NuGetPackageImportStamp>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj Outdated
@@ -17,6 +17,8 @@
<CodeAnalysisRuleSet>..\common\GitHubVS.ruleset</CodeAnalysisRuleSet>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<CodeAnalysisIgnoreGeneratedCode>true</CodeAnalysisIgnoreGeneratedCode>
<NuGetPackageImportStamp>
</NuGetPackageImportStamp>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

src/GitHub.VisualStudio/GitHub.VisualStudio.csproj Outdated
@@ -12,6 +12,8 @@
<VsixType>v3</VsixType>
<IsProductComponent>false</IsProductComponent>
<ExtensionInstallationFolder>GitHub\GitHub</ExtensionInstallationFolder>
<NuGetPackageImportStamp>
</NuGetPackageImportStamp>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

src/GitHub.VisualStudio/app.config Outdated
<dependentAssembly>
<assemblyIdentity name="Microsoft.VisualStudio.Validation" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
<bindingRedirect oldVersion="0.0.0.0-14.0.0.0" newVersion="14.0.0.0" />
</dependentAssembly>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

test/UnitTests/app.config Outdated
<dependentAssembly>
<assemblyIdentity name="Microsoft.VisualStudio.Validation" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
<bindingRedirect oldVersion="0.0.0.0-14.0.0.0" newVersion="14.0.0.0" />
</dependentAssembly>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

shana added a commit that referenced this pull request Nov 15, 2017
@shana
Merge pull request #1321 from backport/2.3.4/1316-libgit2sharp-2.3.1
b4b5da3 
LibGit2Sharp v2.4.0 sometimes throws AccessViolationException on RetrieveStatus.

Cherry-picked from caaa967 (PR #1316)
@jcansdale
Move from LibGit2Sharp v2.4.0 to v2.3.1
23721f6 
LibGit2Sharp v2.4.0 sometimes throws AccessViolationException on RetrieveStatus.
@jcansdale jcansdale force-pushed the fixes/1306-LibGit2Sharp-v2.3.1 branch from caaa967 to 23721f6 Compare November 16, 2017 16:56
@shana
Copy link
Contributor
shana commented Nov 16, 2017

@jcansdale

NOT 8000 E: I've left in some of the dependentAssembly and NuGetPackageImportStamp junk that NuGet likes to add. Hopefully this will make the next time we update NuGet packages cleaner. We don't package the App.config files, so this shouldn't make any difference to the extension.

Yeah, existing things are totally ok. Not having the changes to junk in the commit for the fix is mainly for two things:

  • The new things added and changed that made an hour backport operation into almost five hours, what with all the fighting the branch merges due to the junk lines changed being so close to the actual important changes.
  • If someone does a blame on a csproj and sees a junk line changed but the commit message says "fix for libgit2", they can't tell whether that line was actually required for the fix or if it's just junk. If the commit message for stuff like this clearly says "clean up", then they know it's changes they don't need to look at, which helps a lot when backtracking through history.

@jcansdale
Copy link
Collaborator Author

@shana I've split the stuff that NuGet wanted to add into a separate "cosmetic" PR: #1325.

In the past I've tried to keep the diff nice and clean. In this case I wasn't sure what to do, because it feels like I've been fighting NuGet a lot recently. Hopefully this cosmetic PR will make future changes less of a pain.

@shana shana merged commit 85c169f into master Nov 17, 2017
@jcansdale jcansdale changed the title Move from LibGit2Sharp v2.4.0 to v2.3.1 Move from LibGit2Sharp v0.24.0 to v0.23.1 Nov 17, 2017
@shana shana mentioned this pull request Nov 17, 2017
Closed
@jcansdale jcansdale mentioned this pull request Nov 20, 2017
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@shana shana shana approved these changes

@grokys grokys Awaiting requested review from grokys

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Errors opening a pull request: "Failed to nmap..."
2 participants
@jcansdale @shana
0