Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
97726c4
Ensure only one binding path for extension
jcansdale Oct 18, 2018
4eb1ff8
Merge branch 'master' into fixes/1995-too-many-binding-paths
jcansdale Oct 18, 2018
9196e0c
Use StringComparison.OrdinalIgnoreCase
jcansdale Oct 19, 2018
c7b96f9
Move BindingPathUtilities to GitHub.Exports
jcansdale Oct 19, 2018
7b497f3
Make BindingPathUtilities static
jcansdale Oct 19, 2018
e323dde
Add tests for BindingPathUtilities
jcansdale Oct 19, 2018
abf2772
Add xmldoc comments for BindingPathUtilities
jcansdale Oct 19, 2018
fed313c
Only RationalizeBindingPaths on DEBUG builds
jcansdale Oct 19, 2018
0c20c5a
Extract FindRedundantBindingPaths method
jcansdale Oct 19, 2018
3ec98ff
Add simple dialog based UI
jcansdale Oct 19, 2018
40ee391
Detect wrong binding path and offer info
jcansdale Oct 19, 2018
2212a1f
Remove redundant code
jcansdale Oct 19, 2018
0f29425
Use settings manager instead of reflection
jcansdale Oct 22, 2018
221f19c
Check for DEBUG and catch in separate method
jcansdale Oct 22, 2018
7b312d3
Add xmldoc comments for BindingPathHelper
jcansdale Oct 22, 2018
7cb1dea
Always call FindBindingPaths on UI thread
jcansdale Oct 22, 2018
7d5631b
Open issue with how to fix
jcansdale Oct 22, 2018
b6e86e3
Merge branch 'master' into fixes/1995-too-many-binding-paths
jcansdale Oct 22, 2018
0cb3401
Fix CA errors on CheckBindingPathsAsync
jcansdale Oct 23, 2018
8bc8745
Consolidate BindingPath(Utilities|Helper)
jcansdale Oct 23, 2018
5ceaefc
Add GlobalSuppressions for unit tests
jcansdale Oct 23, 2018
e782108
Fix CA errors
jcansdale Oct 24, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/GitHub.VisualStudio/GitHub.VisualStudio.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@
<Compile Include="Commands\GoToSolutionOrPullRequestFileCommand.cs" />
<Compile Include="GitContextPackage.cs" />
<Compile Include="GitHubPanePackage.cs" />
<Compile Include="Helpers\BindingPathHelper.cs" />
<Compile Include="IServiceProviderPackage.cs" />
<Compile Include="Helpers\ActiveDocumentSnapshot.cs" />
<Compile Include="Commands\AddConnectionCommand.cs" />
Expand Down
26 changes: 24 additions & 2 deletions src/GitHub.VisualStudio/GitHubPackage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using GitHub.Logging;
using GitHub.Services;
using GitHub.Settings;
using GitHub.VisualStudio.Helpers;
using GitHub.VisualStudio.Commands;
using GitHub.Services.Vssdk.Commands;
using GitHub.ViewModels.GitHubPane;
Expand Down Expand Up @@ -166,18 +167,39 @@ public sealed class ServiceProviderPackage : AsyncPackage, IServiceProviderPacka
public const string ServiceProviderPackageId = "D5CE1488-DEDE-426D-9E5B-BFCCFBE33E53";
static readonly ILogger log = LogManager.ForContext<ServiceProviderPackage>();

protected override Task InitializeAsync(CancellationToken cancellationToken, IProgress<ServiceProgressData> progress)
protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress<ServiceProgressData> progress)
{
await CheckBindingPathsAsync();

AddService(typeof(IGitHubServiceProvider), CreateService, true);
AddService(typeof(IVSGitExt), CreateService, true);
AddService(typeof(IUsageTracker), CreateService, true);
AddService(typeof(IUsageService), CreateService, true);
AddService(typeof(ILoginManager), CreateService, true);
AddService(typeof(IGitHubToolWindowManager), CreateService, true);
AddService(typeof(IPackageSettings), CreateService, true);
return Task.CompletedTask;
}

#if DEBUG
async Task CheckBindingPathsAsync()
{
try
{
// When running in the Exp instance, ensure there is only one active binding path.
// This is necessary when the regular (AllUsers) extension is also installed.
// See: https://github.com/github/VisualStudio/issues/2006
await JoinableTaskFactory.SwitchToMainThreadAsync();
BindingPathHelper.CheckBindingPaths(GetType().Assembly, this);
}
catch (Exception e)
{
log.Error(e, nameof(CheckBindingPathsAsync));
}
}
#else
Task CheckBindingPathsAsync() => Task.CompletedTask;
#endif

public async Task<IGitHubPaneViewModel> ShowGitHubPane()
{
var pane = ShowToolWindow(new Guid(GitHubPane.GitHubPaneGuid));
Expand Down
110 changes: 110 additions & 0 deletions src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
using System;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Diagnostics;
using System.Globalization;
using System.Collections.Generic;
using GitHub.Logging;
using Microsoft.VisualStudio.Shell;
using Microsoft.VisualStudio.Shell.Interop;
using Microsoft.VisualStudio.Shell.Settings;
using Microsoft.VisualStudio.Settings;
using Serilog;

namespace GitHub.VisualStudio.Helpers
{
/// <summary>
/// This a workaround for extensions that define a ProvideBindingPath attribute and
/// install for AllUsers.
/// </summary>
/// <remarks>
/// Extensions that are installed for AllUsers, will also be installed for all
/// instances of Visual Studio - including the experimental (Exp) instance which
/// is used in development. This isn't a problem so long as all features that
/// exist in the AllUsers extension, also exist in the extension that is being
/// developed.
///
/// When an extension uses the ProvideBindingPath attribute, the binding path for
/// the AllUsers extension gets installed at the same time as the one in development.
/// This doesn't matter when an assembly is strong named and is loaded using its
/// full name (including version number). When an assembly is loaded using its
/// simple name, assemblies from the AllUsers extension can end up loaded at the
/// same time as the extension being developed. This can happen when an assembly
/// is loaded from XAML or an .imagemanifest.
///
/// This is a workaround for that issue. The <see cref="FindRedundantBindingPaths(List{string}, string)" />
/// method will check to see if a reference assembly could be loaded from an alternative
/// binding path. It will return any alternative paths that is finds.
/// See https://github.com/github/VisualStudio/issues/1995
/// </remarks>
public static class BindingPathHelper
{
static readonly ILogger log = LogManager.ForContext(typeof(BindingPathHelper));

internal static void CheckBindingPaths(Assembly assembly, IServiceProvider serviceProvider)
{
log.Information("Looking for assembly on wrong binding path");

ThreadHelper.CheckAccess();
var bindingPaths = FindBindingPaths(serviceProvider);
var bindingPath = FindRedundantBindingPaths(bindingPaths, assembly.Location)
.FirstOrDefault();
if (bindingPath == null)
{
log.Information("No incorrect binding path found");
return;
}

// Log what has been detected
log.Warning("Found assembly on wrong binding path {BindingPath}", bindingPath);

var message = string.Format(CultureInfo.CurrentCulture, @"Found assembly on wrong binding path:
{0}

Would you like to learn more about this issue?", bindingPath);
var action = VsShellUtilities.ShowMessageBox(serviceProvider, message, "GitHub for Visual Studio", OLEMSGICON.OLEMSGICON_WARNING,
OLEMSGBUTTON.OLEMSGBUTTON_YESNO, OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST);
if (action == 6) // Yes = 6, No = 7
{
Process.Start("https://github.com/github/VisualStudio/issues/2006");
}
}

/// <summary>
/// Find any alternative binding path that might have been installed by an AllUsers extension.
/// </summary>
/// <param name="bindingPaths">A list of binding paths to search</param>
/// <param name="assemblyLocation">A reference assembly that has been loaded from the correct path.</param>
/// <returns>A list of redundant binding paths.</returns>
public static IList<string> FindRedundantBindingPaths(IEnumerable<string> bindingPaths, string assemblyLocation)
{
var fileName = Path.GetFileName(assemblyLocation);
return bindingPaths
.Select(p => (path: p, file: Path.Combine(p, fileName)))
.Where(pf => File.Exists(pf.file))
.Where(pf => !pf.file.Equals(assemblyLocation, StringComparison.OrdinalIgnoreCase))
.Select(pf => pf.path)
.ToList();
}

/// <summary>
/// Find Visual Studio's list of binding paths.
/// </summary>
/// <returns>A list of binding paths.</returns>
public static IEnumerable<string> FindBindingPaths(IServiceProvider serviceProvider)
{
const string bindingPaths = "BindingPaths";
var manager = new ShellSettingsManager(serviceProvider);
var store = manager.GetReadOnlySettingsStore(SettingsScope.Configuration);
foreach (var guid in store.GetSubCollectionNames(bindingPaths))
{
var guidPath = Path.Combine(bindingPaths, guid);
foreach (var path in store.GetPropertyNames(guidPath))
{
yield return path;
}
}
}
}
}
10 changes: 10 additions & 0 deletions test/GitHub.VisualStudio.UnitTests/GlobalSuppressions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

// This file is used by Code Analysis to maintain SuppressMessage
// attributes that are applied to this project.
// Project-level suppressions either have no target or are given
// a specific target and scoped to a namespace, type, member, etc.

[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1034:Nested types should not be visible",
Justification = "It's okay for nested unit test types to be visible")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1707:Identifiers should not contain underscores",
Justification = "It's okay for unit test names to contain underscores")]
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
using System.IO;
using System.Collections.Generic;
using GitHub.VisualStudio.Helpers;
using NUnit.Framework;

public static class BindingPathHelperTests
{
public class TheFindRedundantBindingPathsMethod
{
[TestCase]
public void Redundant_Binding_Paths_Contains_Alternative_Path()
{
var alternativeLocation = GetType().Assembly.Location;
var fileName = Path.GetFileName(alternativeLocation);
var alternativeDir = Path.GetDirectoryName(alternativeLocation);
var assemblyDir = @"c:\target";
var assemblyLocation = Path.Combine(assemblyDir, fileName);
var bindingPaths = new List<string> { alternativeDir, assemblyDir };

var paths = BindingPathHelper.FindRedundantBindingPaths(bindingPaths, assemblyLocation);

Assert.That(paths, Contains.Item(alternativeDir));
Assert.That(paths, Does.Not.Contain(assemblyDir));
}

[TestCase]
public void No_Redundant_Binding_Paths()
{
var assemblyLocation = GetType().Assembly.Location;
var assemblyDir = Path.GetDirectoryName(assemblyLocation);
var bindingPaths = new List<string> { assemblyDir };

var paths = BindingPathHelper.FindRedundantBindingPaths(bindingPaths, assemblyLocation);

Assert.That(paths, Does.Not.Contain(assemblyDir));
}
}
}