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

Commit 6c1e811

Browse files
authored
Merge pull request #1996 from github/fixes/1995-too-many-binding-paths
Ensure only one binding path for extension
2 parents 5f502af + e782108 commit 6c1e811

File tree

5 files changed

+183
-2
lines changed

5 files changed

+183
-2
lines changed

src/GitHub.VisualStudio/GitHub.VisualStudio.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,7 @@
345345
<Compile Include="Commands\GoToSolutionOrPullRequestFileCommand.cs" />
346346
<Compile Include="GitContextPackage.cs" />
347347
<Compile Include="GitHubPanePackage.cs" />
348+
<Compile Include="Helpers\BindingPathHelper.cs" />
348349
<Compile Include="IServiceProviderPackage.cs" />
349350
<Compile Include="Helpers\ActiveDocumentSnapshot.cs" />
350351
<Compile Include="Commands\AddConnectionCommand.cs" />

src/GitHub.VisualStudio/GitHubPackage.cs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using GitHub.Logging;
1212
using GitHub.Services;
1313
using GitHub.Settings;
14+
using GitHub.VisualStudio.Helpers;
1415
using GitHub.VisualStudio.Commands;
1516
using GitHub.Services.Vssdk.Commands;
1617
using GitHub.ViewModels.GitHubPane;
@@ -166,18 +167,39 @@ public sealed class ServiceProviderPackage : AsyncPackage, IServiceProviderPacka
166167
public const string ServiceProviderPackageId = "D5CE1488-DEDE-426D-9E5B-BFCCFBE33E53";
167168
static readonly ILogger log = LogManager.ForContext<ServiceProviderPackage>();
168169

169-
protected override Task InitializeAsync(CancellationToken cancellationToken, IProgress<ServiceProgressData> progress)
170+
protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress<ServiceProgressData> progress)
170171
{
172+
await CheckBindingPathsAsync();
173+
171174
AddService(typeof(IGitHubServiceProvider), CreateService, true);
172175
AddService(typeof(IVSGitExt), CreateService, true);
173176
AddService(typeof(IUsageTracker), CreateService, true);
174177
AddService(typeof(IUsageService), CreateService, true);
175178
AddService(typeof(ILoginManager), CreateService, true);
176179
AddService(typeof(IGitHubToolWindowManager), CreateService, true);
177180
AddService(typeof(IPackageSettings), CreateService, true);
178-
return Task.CompletedTask;
179181
}
180182

183+
#if DEBUG
184+
async Task CheckBindingPathsAsync()
185+
{
186+
try
187+
{
188+
// When running in the Exp instance, ensure there is only one active binding path.
189+
// This is necessary when the regular (AllUsers) extension is also installed.
190+
// See: https://github.com/github/VisualStudio/issues/2006
191+
await JoinableTaskFactory.SwitchToMainThreadAsync();
192+
BindingPathHelper.CheckBindingPaths(GetType().Assembly, this);
193+
}
194+
catch (Exception e)
195+
{
196+
log.Error(e, nameof(CheckBindingPathsAsync));
197+
}
198+
}
199+
#else
200+
Task CheckBindingPathsAsync() => Task.CompletedTask;
201+
#endif
202+
181203
public async Task<IGitHubPaneViewModel> ShowGitHubPane()
182204
{
183205
var pane = ShowToolWindow(new Guid(GitHubPane.GitHubPaneGuid));
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
using System;
2+
using System.IO;
3+
using System.Linq;
4+
using System.Reflection;
5+
using System.Diagnostics;
6+
using System.Globalization;
7+
using System.Collections.Generic;
8+
using GitHub.Logging;
9+
using Microsoft.VisualStudio.Shell;
10+
using Microsoft.VisualStudio.Shell.Interop;
11+
using Microsoft.VisualStudio.Shell.Settings;
12+
using Microsoft.VisualStudio.Settings;
13+
using Serilog;
14+
15+
namespace GitHub.VisualStudio.Helpers
16+
{
17+
/// <summary>
18+
/// This a workaround for extensions that define a ProvideBindingPath attribute and
19+
/// install for AllUsers.
20+
/// </summary>
21+
/// <remarks>
22+
/// Extensions that are installed for AllUsers, will also be installed for all
23+
/// instances of Visual Studio - including the experimental (Exp) instance which
24+
/// is used in development. This isn't a problem so long as all features that
25+
/// exist in the AllUsers extension, also exist in the extension that is being
26+
/// developed.
27+
///
28+
/// When an extension uses the ProvideBindingPath attribute, the binding path for
29+
/// the AllUsers extension gets installed at the same time as the one in development.
30+
/// This doesn't matter when an assembly is strong named and is loaded using its
31+
/// full name (including version number). When an assembly is loaded using its
32+
/// simple name, assemblies from the AllUsers extension can end up loaded at the
33+
/// same time as the extension being developed. This can happen when an assembly
34+
/// is loaded from XAML or an .imagemanifest.
35+
///
36+
/// This is a workaround for that issue. The <see cref="FindRedundantBindingPaths(List{string}, string)" />
37+
/// method will check to see if a reference assembly could be loaded from an alternative
38+
/// binding path. It will return any alternative paths that is finds.
39+
/// See https://github.com/github/VisualStudio/issues/1995
40+
/// </remarks>
41+
public static class BindingPathHelper
42+
{
43+
static readonly ILogger log = LogManager.ForContext(typeof(BindingPathHelper));
44+
45+
internal static void CheckBindingPaths(Assembly assembly, IServiceProvider serviceProvider)
46+
{
47+
log.Information("Looking for assembly on wrong binding path");
48+
49+
ThreadHelper.CheckAccess();
50+
var bindingPaths = FindBindingPaths(serviceProvider);
51+
var bindingPath = FindRedundantBindingPaths(bindingPaths, assembly.Location)
52+
.FirstOrDefault();
53+
if (bindingPath == null)
54+
{
55+
log.Information("No incorrect binding path found");
56+
return;
57+
}
58+
59+
// Log what has been detected
60+
log.Warning("Found assembly on wrong binding path {BindingPath}", bindingPath);
61+
62+
var message = string.Format(CultureInfo.CurrentCulture, @"Found assembly on wrong binding path:
63+
{0}
64+
65+
Would you like to learn more about this issue?", bindingPath);
66+
var action = VsShellUtilities.ShowMessageBox(serviceProvider, message, "GitHub for Visual Studio", OLEMSGICON.OLEMSGICON_WARNING,
67+
OLEMSGBUTTON.OLEMSGBUTTON_YESNO, OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST);
68+
if (action == 6) // Yes = 6, No = 7
69+
{
70+
Process.Start("https://github.com/github/VisualStudio/issues/2006");
71+
}
72+
}
73+
74+
/// <summary>
75+
/// Find any alternative binding path that might have been installed by an AllUsers extension.
76+
/// </summary>
77+
/// <param name="bindingPaths">A list of binding paths to search</param>
78+
/// <param name="assemblyLocation">A reference assembly that has been loaded from the correct path.</param>
79+
/// <returns>A list of redundant binding paths.</returns>
80+
public static IList<string> FindRedundantBindingPaths(IEnumerable<string> bindingPaths, string assemblyLocation)
81+
{
82+
var fileName = Path.GetFileName(assemblyLocation);
83+
return bindingPaths
84+
.Select(p => (path: p, file: Path.Combine(p, fileName)))
85+
.Where(pf => File.Exists(pf.file))
86+
.Where(pf => !pf.file.Equals(assemblyLocation, StringComparison.OrdinalIgnoreCase))
87+
.Select(pf => pf.path)
88+
.ToList();
89+
}
90+
91+
/// <summary>
92+
/// Find Visual Studio's list of binding paths.
93+
/// </summary>
94+
/// <returns>A list of binding paths.</returns>
95+
public static IEnumerable<string> FindBindingPaths(IServiceProvider serviceProvider)
96+
{
97+
const string bindingPaths = "BindingPaths";
98+
var manager = new ShellSettingsManager(serviceProvider);
99+
var store = manager.GetReadOnlySettingsStore(SettingsScope.Configuration);
100+
foreach (var guid in store.GetSubCollectionNames(bindingPaths))
101+
{
102+
var guidPath = Path.Combine(bindingPaths, guid);
103+
foreach (var path in store.GetPropertyNames(guidPath))
104+
{
105+
yield return path;
106+
}
107+
}
108+
}
109+
}
110+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+

2+
// This file is used by Code Analysis to maintain SuppressMessage
3+
// attributes that are applied to this project.
4+
// Project-level suppressions either have no target or are given
5+
// a specific target and scoped to a namespace, type, member, etc.
6+
7+
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1034:Nested types should not be visible",
8+
Justification = "It's okay for nested unit test types to be visible")]
9+
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1707:Identifiers should not contain underscores",
10+
Justification = "It's okay for unit test names to contain underscores")]
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
using System.IO;
2+
using System.Collections.Generic;
3+
using GitHub.VisualStudio.Helpers;
4+
using NUnit.Framework;
5+
6+
public static class BindingPathHelperTests
7+
{
8+
public class TheFindRedundantBindingPathsMethod
9+
{
10+
[TestCase]
11+
public void Redundant_Binding_Paths_Contains_Alternative_Path()
12+
{
13+
var alternativeLocation = GetType().Assembly.Location;
14+
var fileName = Path.GetFileName(alternativeLocation);
15+
var alternativeDir = Path.GetDirectoryName(alternativeLocation);
16+
var assemblyDir = @"c:\target";
17+
var assemblyLocation = Path.Combine(assemblyDir, fileName);
18+
var bindingPaths = new List<string> { alternativeDir, assemblyDir };
19+
20+
var paths = BindingPathHelper.FindRedundantBindingPaths(bindingPaths, assemblyLocation);
21+
22+
Assert.That(paths, Contains.Item(alternativeDir));
23+
Assert.That(paths, Does.Not.Contain(assemblyDir));
24+
}
25+
26+
[TestCase]
27+
public void No_Redundant_Binding_Paths()
28+
{
29+
var assemblyLocation = GetType().Assembly.Location;
30+
var assemblyDir = Path.GetDirectoryName(assemblyLocation);
31+
var bindingPaths = new List<string> { assemblyDir };
32+
33+
var paths = BindingPathHelper.FindRedundantBindingPaths(bindingPaths, assemblyLocation);
34+
35+
Assert.That(paths, Does.Not.Contain(assemblyDir));
36+
}
37+
}
38+
}

0 commit comments

Comments
 (0)