From 97726c479c7b357f97f17c7058b65124dea2cf71 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Thu, 18 Oct 2018 11:41:58 +0100 Subject: [PATCH 01/20] Ensure only one binding path for extension Check that only one binding path is active and remove and extra binding paths that would cause issues. --- .../BindingPathUtilities.cs | 54 +++++++++++++++++++ .../GitHub.VisualStudio.csproj | 1 + src/GitHub.VisualStudio/GitHubPackage.cs | 3 ++ 3 files changed, 58 insertions(+) create mode 100644 src/GitHub.VisualStudio/BindingPathUtilities.cs diff --git a/src/GitHub.VisualStudio/BindingPathUtilities.cs b/src/GitHub.VisualStudio/BindingPathUtilities.cs new file mode 100644 index 0000000000..ae7059821d --- /dev/null +++ b/src/GitHub.VisualStudio/BindingPathUtilities.cs @@ -0,0 +1,54 @@ +using System; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Collections.Generic; +using GitHub.Logging; +using Serilog; + +namespace GitHub.VisualStudio +{ + public class BindingPathUtilities + { + static readonly ILogger log = LogManager.ForContext(); + + public static void RationalizeBindingPaths(string assemblyLocation, List bindingPaths = null) + { + bindingPaths = bindingPaths ?? FindBindingPaths(); + + var fileName = Path.GetFileName(assemblyLocation); + bindingPaths + .Select(p => new { path = p, file = Path.Combine(p, fileName) }) + .Where(pf => File.Exists(pf.file)) + .Where(pf => !pf.file.Equals(assemblyLocation, StringComparison.OrdinalIgnoreCase)) + .ToList() + .ForEach(pf => + { + var loaded = IsAssemblyLoaded(pf.file); + if (loaded) + { + log.Error("Assembly has already been loaded from {Location}", pf.file); + } + + log.Warning("Removing duplicate binding path {BindingPath}", pf.path); + bindingPaths.Remove(pf.path); + }); + } + + public static bool IsAssemblyLoaded(string assemblyLocation) + { + var prefix = Path.GetFileNameWithoutExtension(assemblyLocation) + ","; + return AppDomain.CurrentDomain.GetAssemblies() + .Where(a => a.FullName.StartsWith(prefix)) + .Where(a => a.Location.Equals(assemblyLocation, StringComparison.OrdinalIgnoreCase)) + .Any(); + } + + public static List FindBindingPaths() + { + var manager = AppDomain.CurrentDomain.DomainManager; + var property = manager.GetType().GetProperty("BindingPaths", BindingFlags.NonPublic | BindingFlags.Instance); + return (List)property?.GetValue(manager); + } + } +} diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index a355aad787..9dec86dea4 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -330,6 +330,7 @@ Properties\SolutionInfo.cs + diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 2f368e96c0..411f559481 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using System.Threading; using System.Threading.Tasks; using System.ComponentModel.Design; @@ -168,6 +169,8 @@ public sealed class ServiceProviderPackage : AsyncPackage, IServiceProviderPacka protected override Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { + BindingPathUtilities.RationalizeBindingPaths(GetType().Assembly.Location); + AddService(typeof(IGitHubServiceProvider), CreateService, true); AddService(typeof(IVSGitExt), CreateService, true); AddService(typeof(IUsageTracker), CreateService, true); From 9196e0cbe4094ea084ffff49d3dce41ccc182e3c Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 19 Oct 2018 09:21:49 +0100 Subject: [PATCH 02/20] Use StringComparison.OrdinalIgnoreCase Fix CA error. --- src/GitHub.VisualStudio/BindingPathUtilities.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GitHub.VisualStudio/BindingPathUtilities.cs b/src/GitHub.VisualStudio/BindingPathUtilities.cs index ae7059821d..5c0d27abd1 100644 --- a/src/GitHub.VisualStudio/BindingPathUtilities.cs +++ b/src/GitHub.VisualStudio/BindingPathUtilities.cs @@ -39,7 +39,7 @@ public static bool IsAssemblyLoaded(string assemblyLocation) { var prefix = Path.GetFileNameWithoutExtension(assemblyLocation) + ","; return AppDomain.CurrentDomain.GetAssemblies() - .Where(a => a.FullName.StartsWith(prefix)) + .Where(a => a.FullName.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) .Where(a => a.Location.Equals(assemblyLocation, StringComparison.OrdinalIgnoreCase)) .Any(); } From c7b96f9cedea1f1a75ea0da84ec24cdc94c0e9a8 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 19 Oct 2018 10:19:19 +0100 Subject: [PATCH 03/20] Move BindingPathUtilities to GitHub.Exports This is a helper class and not fundamental to GitHub for Visual Studio. --- .../Helpers}/BindingPathUtilities.cs | 2 +- src/GitHub.VisualStudio/GitHub.VisualStudio.csproj | 1 - src/GitHub.VisualStudio/GitHubPackage.cs | 1 + 3 files changed, 2 insertions(+), 2 deletions(-) rename src/{GitHub.VisualStudio => GitHub.Exports/Helpers}/BindingPathUtilities.cs (98%) diff --git a/src/GitHub.VisualStudio/BindingPathUtilities.cs b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs similarity index 98% rename from src/GitHub.VisualStudio/BindingPathUtilities.cs rename to src/GitHub.Exports/Helpers/BindingPathUtilities.cs index 5c0d27abd1..cfdced0347 100644 --- a/src/GitHub.VisualStudio/BindingPathUtilities.cs +++ b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs @@ -6,7 +6,7 @@ using GitHub.Logging; using Serilog; -namespace GitHub.VisualStudio +namespace GitHub.Helpers { public class BindingPathUtilities { diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index 9dec86dea4..a355aad787 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -330,7 +330,6 @@ Properties\SolutionInfo.cs - diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 411f559481..81396deee1 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -9,6 +9,7 @@ using GitHub.Commands; using GitHub.Info; using GitHub.Exports; +using GitHub.Helpers; using GitHub.Logging; using GitHub.Services; using GitHub.Settings; From 7b497f31590aefe9c78fc0253da50fcd161a879e Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 19 Oct 2018 10:20:06 +0100 Subject: [PATCH 04/20] Make BindingPathUtilities static --- src/GitHub.Exports/Helpers/BindingPathUtilities.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs index cfdced0347..5a08454b8a 100644 --- a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs +++ b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs @@ -8,9 +8,9 @@ namespace GitHub.Helpers { - public class BindingPathUtilities + public static class BindingPathUtilities { - static readonly ILogger log = LogManager.ForContext(); + static readonly ILogger log = LogManager.ForContext(typeof(BindingPathUtilities)); public static void RationalizeBindingPaths(string assemblyLocation, List bindingPaths = null) { From e323dde267dd8117a5363c23eb3bfa1a5da4fa9a Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 19 Oct 2018 11:38:48 +0100 Subject: [PATCH 05/20] Add tests for BindingPathUtilities Make RationalizeBindingPaths return false when an assembly has already been loaded from an alternative location. Make FindBindingPaths return empty when not running inside Visual Studio. --- .../Helpers/BindingPathUtilities.cs | 11 +- .../Helpers/BindingPathUtilitiesTests.cs | 100 ++++++++++++++++++ 2 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs diff --git a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs index 5a08454b8a..bf80511d65 100644 --- a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs +++ b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs @@ -12,10 +12,11 @@ public static class BindingPathUtilities { static readonly ILogger log = LogManager.ForContext(typeof(BindingPathUtilities)); - public static void RationalizeBindingPaths(string assemblyLocation, List bindingPaths = null) + public static bool RationalizeBindingPaths(string assemblyLocation, List bindingPaths = null) { bindingPaths = bindingPaths ?? FindBindingPaths(); + var isAlreadyLoaded = false; var fileName = Path.GetFileName(assemblyLocation); bindingPaths .Select(p => new { path = p, file = Path.Combine(p, fileName) }) @@ -27,12 +28,15 @@ public static void RationalizeBindingPaths(string assemblyLocation, List var loaded = IsAssemblyLoaded(pf.file); if (loaded) { + isAlreadyLoaded = true; log.Error("Assembly has already been loaded from {Location}", pf.file); } log.Warning("Removing duplicate binding path {BindingPath}", pf.path); bindingPaths.Remove(pf.path); }); + + return !isAlreadyLoaded; } public static bool IsAssemblyLoaded(string assemblyLocation) @@ -47,8 +51,9 @@ public static bool IsAssemblyLoaded(string assemblyLocation) public static List FindBindingPaths() { var manager = AppDomain.CurrentDomain.DomainManager; - var property = manager.GetType().GetProperty("BindingPaths", BindingFlags.NonPublic | BindingFlags.Instance); - return (List)property?.GetValue(manager); + var property = manager?.GetType().GetProperty("BindingPaths", BindingFlags.NonPublic | BindingFlags.Instance); + var bindingPaths = property?.GetValue(manager) as List; + return bindingPaths ?? new List(0); } } } diff --git a/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs b/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs new file mode 100644 index 0000000000..3b64cbd6c4 --- /dev/null +++ b/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs @@ -0,0 +1,100 @@ +using System; +using System.IO; +using System.Collections.Generic; +using GitHub.Helpers; +using NUnit.Framework; +using NSubstitute; +using Serilog; + +public static class BindingPathUtilitiesTests +{ + public class TheRationalizeBindingPathsMethod + { + [TestCase] + public void Alternative_Path_Is_Removed_From_BinsingPaths() + { + 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 { alternativeDir, assemblyDir }; + + BindingPathUtilities.RationalizeBindingPaths(assemblyLocation, bindingPaths); + + Assert.That(bindingPaths, Contains.Item(assemblyDir)); + Assert.That(bindingPaths, Does.Not.Contain(alternativeDir)); + } + + [TestCase] + public void Return_False_When_Assembly_Already_Loaded_From_Alternative_Location() + { + 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 { alternativeDir, assemblyDir }; + + var success = BindingPathUtilities.RationalizeBindingPaths(assemblyLocation, bindingPaths); + + Assert.That(success, Is.False); + } + + [TestCase] + public void Return_True_When_Assembly_Not_Already_Loaded_From_Alternative_Location() + { + var assemblyLocation = GetType().Assembly.Location; + var assemblyDir = Path.GetDirectoryName(assemblyLocation); + var bindingPaths = new List { assemblyDir }; + + var success = BindingPathUtilities.RationalizeBindingPaths(assemblyLocation, bindingPaths); + + Assert.That(success, Is.True); + } + } + + public class TheFindBindingPathsMethod + { + [TestCase] + public void Return_Empty_When_Not_In_Visual_Studio() + { + var bindingPaths = BindingPathUtilities.FindBindingPaths(); + + Assert.IsEmpty(bindingPaths); + } + } + + public class TheIsAssemblyLoadedMethod + { + [TestCase] + public void Check_Executing_Assumbly_Is_Loaded() + { + var location = GetType().Assembly.Location; + + var isLoaded = BindingPathUtilities.IsAssemblyLoaded(location); + + Assert.That(isLoaded, Is.True); + } + + [TestCase] + public void Check_Executing_Assumbly_Is_Loaded_When_Case_Differs() + { + var location = GetType().Assembly.Location.ToUpperInvariant(); + + var isLoaded = BindingPathUtilities.IsAssemblyLoaded(location); + + Assert.That(isLoaded, Is.True); + } + + [Test] + public void Check_Assembly_At_Unknown_Location_Not_Loaded() + { + var location = @"c:\unknown.dll"; + + var isLoaded = BindingPathUtilities.IsAssemblyLoaded(location); + + Assert.That(isLoaded, Is.False); + } + } +} From abf27729dab6c8da01d8b4475f525bd27e832a91 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 19 Oct 2018 12:14:55 +0100 Subject: [PATCH 06/20] Add xmldoc comments for BindingPathUtilities --- .../Helpers/BindingPathUtilities.cs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs index bf80511d65..b4acdc9426 100644 --- a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs +++ b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs @@ -8,10 +8,40 @@ namespace GitHub.Helpers { + /// + /// This a workaround for extensions that define a ProvideBindingPath attribute and + /// install for AllUsers. + /// + /// + /// 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 + /// method will check to see if a reference assembly could be loaded from an alternative + /// binding path. It will remove any alternative paths that is finds so that XAML or + /// .imagemanifest won't end up loading the wrong assembly. + /// public static class BindingPathUtilities { static readonly ILogger log = LogManager.ForContext(typeof(BindingPathUtilities)); + /// + /// Remove alternative binding path that might have been installed by an AllUsers extension. + /// + /// A reference assembly that has been loaded from the correct path. + /// A list of binding paths to rationalize (only used when testing) + /// True if assembly hasn't been loaded from an alternative path. public static bool RationalizeBindingPaths(string assemblyLocation, List bindingPaths = null) { bindingPaths = bindingPaths ?? FindBindingPaths(); @@ -39,6 +69,11 @@ public static bool RationalizeBindingPaths(string assemblyLocation, List return !isAlreadyLoaded; } + /// + /// Check to see if an assembly is already in memory. + /// + /// The assembly to look for. + /// True if assembly has already been loaded. public static bool IsAssemblyLoaded(string assemblyLocation) { var prefix = Path.GetFileNameWithoutExtension(assemblyLocation) + ","; @@ -48,6 +83,10 @@ public static bool IsAssemblyLoaded(string assemblyLocation) .Any(); } + /// + /// Use reflection to find Visual Studio's list of binding paths. + /// + /// A live list of binding paths or an empty list if not running in Visual Studio. public static List FindBindingPaths() { var manager = AppDomain.CurrentDomain.DomainManager; From fed313c10aff6f2680dd2cb01ab2b70ac2e89961 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 19 Oct 2018 12:53:34 +0100 Subject: [PATCH 07/20] Only RationalizeBindingPaths on DEBUG builds This doesn't need to run in production. --- src/GitHub.VisualStudio/GitHubPackage.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 81396deee1..3ed91ce401 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -1,5 +1,4 @@ using System; -using System.Linq; using System.Threading; using System.Threading.Tasks; using System.ComponentModel.Design; @@ -170,7 +169,9 @@ public sealed class ServiceProviderPackage : AsyncPackage, IServiceProviderPacka protected override Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { - BindingPathUtilities.RationalizeBindingPaths(GetType().Assembly.Location); + // Ensure there is only one active binding path for the extension. + // This is necessary when the regular (AllUsers) extension is installed. + RationalizeBindingPaths(); AddService(typeof(IGitHubServiceProvider), CreateService, true); AddService(typeof(IVSGitExt), CreateService, true); @@ -197,6 +198,13 @@ public async Task ShowGitHubPane() return await gitHubPane.GetViewModelAsync(); } + [System.Diagnostics.Conditional("DEBUG")] + void RationalizeBindingPaths() + { + log.Information("Rationalizing any duplicate binding paths"); + BindingPathUtilities.RationalizeBindingPaths(GetType().Assembly.Location); + } + static ToolWindowPane ShowToolWindow(Guid windowGuid) { IVsWindowFrame frame; From 0c20c5ae332f26e20a03d8f5bfe1780ce881c50f Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 19 Oct 2018 16:02:38 +0100 Subject: [PATCH 08/20] Extract FindRedundantBindingPaths method Separate the code that reads and the code that mutates. --- .../Helpers/BindingPathUtilities.cs | 50 ++++++++++--------- src/GitHub.VisualStudio/GitHubPackage.cs | 35 ++++++++++++- .../Helpers/BindingPathUtilitiesTests.cs | 24 ++------- 3 files changed, 65 insertions(+), 44 deletions(-) diff --git a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs index b4acdc9426..cee87bf8df 100644 --- a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs +++ b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs @@ -27,7 +27,7 @@ namespace GitHub.Helpers /// 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 + /// This is a workaround for that issue. The /// method will check to see if a reference assembly could be loaded from an alternative /// binding path. It will remove any alternative paths that is finds so that XAML or /// .imagemanifest won't end up loading the wrong assembly. @@ -39,34 +39,31 @@ public static class BindingPathUtilities /// /// Remove alternative binding path that might have been installed by an AllUsers extension. /// + /// A list of binding paths to rationalize /// A reference assembly that has been loaded from the correct path. - /// A list of binding paths to rationalize (only used when testing) - /// True if assembly hasn't been loaded from an alternative path. - public static bool RationalizeBindingPaths(string assemblyLocation, List bindingPaths = null) + /// True if binding path was removed. + public static bool RationalizeBindingPaths(List bindingPaths, string assemblyLocation) { - bindingPaths = bindingPaths ?? FindBindingPaths(); + var redundantBindingPaths = FindRedundantBindingPaths(bindingPaths, assemblyLocation); + RemoveRedundantBindingPaths(bindingPaths, assemblyLocation, redundantBindingPaths); + return redundantBindingPaths.Any(); + } - var isAlreadyLoaded = false; + /// + /// Find any alternative binding path that might have been installed by an AllUsers extension. + /// + /// A list of binding paths to search + /// A reference assembly that has been loaded from the correct path. + /// A list of redundant binding paths. + public static IList FindRedundantBindingPaths(List bindingPaths, string assemblyLocation) + { var fileName = Path.GetFileName(assemblyLocation); - bindingPaths - .Select(p => new { path = p, file = Path.Combine(p, fileName) }) + 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)) - .ToList() - .ForEach(pf => - { - var loaded = IsAssemblyLoaded(pf.file); - if (loaded) - { - isAlreadyLoaded = true; - log.Error("Assembly has already been loaded from {Location}", pf.file); - } - - log.Warning("Removing duplicate binding path {BindingPath}", pf.path); - bindingPaths.Remove(pf.path); - }); - - return !isAlreadyLoaded; + .Select(pf => pf.path) + .ToList(); } /// @@ -94,5 +91,12 @@ public static List FindBindingPaths() var bindingPaths = property?.GetValue(manager) as List; return bindingPaths ?? new List(0); } + + public static void RemoveRedundantBindingPaths(List bindingPaths, string assemblyLocation, + IList redundantBindingPaths) + { + redundantBindingPaths + .ForEach(p => bindingPaths.Remove(p)); + } } } diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 3ed91ce401..41183afaf5 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -23,6 +23,7 @@ using Microsoft.VisualStudio.Shell.Interop; using Serilog; using Task = System.Threading.Tasks.Task; +using System.IO; namespace GitHub.VisualStudio { @@ -201,8 +202,38 @@ public async Task ShowGitHubPane() [System.Diagnostics.Conditional("DEBUG")] void RationalizeBindingPaths() { - log.Information("Rationalizing any duplicate binding paths"); - BindingPathUtilities.RationalizeBindingPaths(GetType().Assembly.Location); + try + { + log.Information("Searching for duplicate binding paths"); + var assemblyLocation = GetType().Assembly.Location; + var bindingPaths = BindingPathUtilities.FindBindingPaths(); + var redundantPaths = BindingPathUtilities.FindRedundantBindingPaths(bindingPaths, assemblyLocation); + if (redundantPaths.Count == 0) + { + log.Information("No duplicate binding paths found"); + return; + } + + // Log what has been detected + foreach (var redundantPath in redundantPaths) + { + log.Warning("Found redundant binding path {BindingPath}", redundantPath); + + var redundantFile = Path.Combine(redundantPath, Path.GetFileName(assemblyLocation)); + var loaded = BindingPathUtilities.IsAssemblyLoaded(redundantFile); + if (loaded) + { + log.Error("Assembly has already been loaded from {Location}", redundantFile); + } + } + + // Try to fix + BindingPathUtilities.RemoveRedundantBindingPaths(bindingPaths, assemblyLocation, redundantPaths); + } + catch (Exception e) + { + log.Error(e, nameof(RationalizeBindingPaths)); + } } static ToolWindowPane ShowToolWindow(Guid windowGuid) diff --git a/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs b/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs index 3b64cbd6c4..ea97fdc97a 100644 --- a/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs +++ b/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs @@ -20,37 +20,23 @@ public void Alternative_Path_Is_Removed_From_BinsingPaths() var assemblyLocation = Path.Combine(assemblyDir, fileName); var bindingPaths = new List { alternativeDir, assemblyDir }; - BindingPathUtilities.RationalizeBindingPaths(assemblyLocation, bindingPaths); + var removed = BindingPathUtilities.RationalizeBindingPaths(bindingPaths, assemblyLocation); Assert.That(bindingPaths, Contains.Item(assemblyDir)); Assert.That(bindingPaths, Does.Not.Contain(alternativeDir)); + Assert.That(removed, Is.True); } [TestCase] - public void Return_False_When_Assembly_Already_Loaded_From_Alternative_Location() - { - 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 { alternativeDir, assemblyDir }; - - var success = BindingPathUtilities.RationalizeBindingPaths(assemblyLocation, bindingPaths); - - Assert.That(success, Is.False); - } - - [TestCase] - public void Return_True_When_Assembly_Not_Already_Loaded_From_Alternative_Location() + public void Return_False_When_No_BindingPath_Was_Removed() { var assemblyLocation = GetType().Assembly.Location; var assemblyDir = Path.GetDirectoryName(assemblyLocation); var bindingPaths = new List { assemblyDir }; - var success = BindingPathUtilities.RationalizeBindingPaths(assemblyLocation, bindingPaths); + var removed = BindingPathUtilities.RationalizeBindingPaths(bindingPaths, assemblyLocation); - Assert.That(success, Is.True); + Assert.That(removed, Is.False); } } From 3ec98ff1ed3af0c358ffd771feeae4f9205e7646 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 19 Oct 2018 19:39:58 +0100 Subject: [PATCH 09/20] Add simple dialog based UI --- .../GitHub.VisualStudio.csproj | 1 + src/GitHub.VisualStudio/GitHubPackage.cs | 53 +++----------- .../Helpers/BindingPathHelper.cs | 72 +++++++++++++++++++ 3 files changed, 82 insertions(+), 44 deletions(-) create mode 100644 src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index a355aad787..54088cfa9b 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -345,6 +345,7 @@ + diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 41183afaf5..b3ceb0a2d5 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -8,10 +8,10 @@ using GitHub.Commands; using GitHub.Info; using GitHub.Exports; -using GitHub.Helpers; 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; @@ -23,7 +23,6 @@ using Microsoft.VisualStudio.Shell.Interop; using Serilog; using Task = System.Threading.Tasks.Task; -using System.IO; namespace GitHub.VisualStudio { @@ -168,11 +167,15 @@ public sealed class ServiceProviderPackage : AsyncPackage, IServiceProviderPacka public const string ServiceProviderPackageId = "D5CE1488-DEDE-426D-9E5B-BFCCFBE33E53"; static readonly ILogger log = LogManager.ForContext(); - protected override Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) + protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { - // Ensure there is only one active binding path for the extension. - // This is necessary when the regular (AllUsers) extension is installed. - RationalizeBindingPaths(); +#if DEBUG + // 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/1995 + await BindingPathHelper.RationalizeBindingPathsAsync( + GetType().Assembly, JoinableTaskFactory, this); +#endif AddService(typeof(IGitHubServiceProvider), CreateService, true); AddService(typeof(IVSGitExt), CreateService, true); @@ -181,7 +184,6 @@ protected override Task InitializeAsync(CancellationToken cancellationToken, IPr AddService(typeof(ILoginManager), CreateService, true); AddService(typeof(IGitHubToolWindowManager), CreateService, true); AddService(typeof(IPackageSettings), CreateService, true); - return Task.CompletedTask; } public async Task ShowGitHubPane() @@ -199,43 +201,6 @@ public async Task ShowGitHubPane() return await gitHubPane.GetViewModelAsync(); } - [System.Diagnostics.Conditional("DEBUG")] - void RationalizeBindingPaths() - { - try - { - log.Information("Searching for duplicate binding paths"); - var assemblyLocation = GetType().Assembly.Location; - var bindingPaths = BindingPathUtilities.FindBindingPaths(); - var redundantPaths = BindingPathUtilities.FindRedundantBindingPaths(bindingPaths, assemblyLocation); - if (redundantPaths.Count == 0) - { - log.Information("No duplicate binding paths found"); - return; - } - - // Log what has been detected - foreach (var redundantPath in redundantPaths) - { - log.Warning("Found redundant binding path {BindingPath}", redundantPath); - - var redundantFile = Path.Combine(redundantPath, Path.GetFileName(assemblyLocation)); - var loaded = BindingPathUtilities.IsAssemblyLoaded(redundantFile); - if (loaded) - { - log.Error("Assembly has already been loaded from {Location}", redundantFile); - } - } - - // Try to fix - BindingPathUtilities.RemoveRedundantBindingPaths(bindingPaths, assemblyLocation, redundantPaths); - } - catch (Exception e) - { - log.Error(e, nameof(RationalizeBindingPaths)); - } - } - static ToolWindowPane ShowToolWindow(Guid windowGuid) { IVsWindowFrame frame; diff --git a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs new file mode 100644 index 0000000000..02353b38bf --- /dev/null +++ b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs @@ -0,0 +1,72 @@ +using System; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Reflection; +using GitHub.Helpers; +using GitHub.Logging; +using Microsoft.VisualStudio.Shell; +using Microsoft.VisualStudio.Shell.Interop; +using Microsoft.VisualStudio.Threading; +using Serilog; +using Task = System.Threading.Tasks.Task; + +namespace GitHub.VisualStudio.Helpers +{ + class BindingPathHelper + { + static readonly ILogger log = LogManager.ForContext(); + + internal async static Task RationalizeBindingPathsAsync( + Assembly assembly, + JoinableTaskFactory jtf, + IServiceProvider serviceProvider) + { + log.Information("Searching for duplicate binding paths"); + var assemblyLocation = assembly.Location; + var bindingPaths = BindingPathUtilities.FindBindingPaths(); + var redundantPaths = BindingPathUtilities.FindRedundantBindingPaths(bindingPaths, assemblyLocation); + if (redundantPaths.Count == 0) + { + log.Information("No duplicate binding paths found"); + return; + } + + // Log what has been detected + foreach (var redundantPath in redundantPaths) + { + log.Warning("Found redundant binding path {BindingPath}", redundantPath); + + var redundantFile = Path.Combine(redundantPath, Path.GetFileName(assemblyLocation)); + var loaded = BindingPathUtilities.IsAssemblyLoaded(redundantFile); + if (loaded) + { + log.Error("Assembly has already been loaded from {Location}", redundantFile); + } + } + + await jtf.SwitchToMainThreadAsync(); + var message = string.Format(@"Redundant binding path found at: +{0} + +Would like to: +[Abort] - Learn more about this issue +[Retry] - Attempt a fix (this might not work) +[Ignore] - Don't do anything +", redundantPaths.First()); + var action = VsShellUtilities.ShowMessageBox(serviceProvider, message, "Redundant Binding Path", OLEMSGICON.OLEMSGICON_WARNING, + OLEMSGBUTTON.OLEMSGBUTTON_ABORTRETRYIGNORE, OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_SECOND); + switch (action) + { + case 3: // Abort + Process.Start("https://github.com/github/VisualStudio/issues/1995"); + break; + case 4: // Retry - Try to fix + BindingPathUtilities.RemoveRedundantBindingPaths(bindingPaths, assemblyLocation, redundantPaths); + break; + case 5: // Ignore + break; + } + } + } +} From 40ee391e93735640af11ba93fb6ea7a8a06777d3 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Sat, 20 Oct 2018 00:44:09 +0100 Subject: [PATCH 10/20] Detect wrong binding path and offer info Simply detect wrong binding path and offer to show info rather than attempt to fix. --- .../Helpers/BindingPathUtilities.cs | 16 ++++++ .../Helpers/BindingPathHelper.cs | 49 +++++----------- .../Helpers/BindingPathUtilitiesTests.cs | 56 +++++++++++++++++-- 3 files changed, 82 insertions(+), 39 deletions(-) diff --git a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs index cee87bf8df..8d26110850 100644 --- a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs +++ b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs @@ -80,6 +80,22 @@ public static bool IsAssemblyLoaded(string assemblyLocation) .Any(); } + public static Assembly FindAssemblyWithDifferentCodeBase(Assembly assembly) + { + var simpleName = assembly.GetName().Name; + var resolvedAssembly = Assembly.Load(simpleName); + return resolvedAssembly != assembly ? resolvedAssembly : null; + } + + public static Assembly FindLoadedAssemblyWithSameName(Assembly assembly) + { + var prefix = assembly.GetName().Name + ','; + return AppDomain.CurrentDomain.GetAssemblies() + .Where(a => a.FullName.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + .Where(a => a != assembly) + .FirstOrDefault(); + } + /// /// Use reflection to find Visual Studio's list of binding paths. /// diff --git a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs index 02353b38bf..39c74f56c3 100644 --- a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs +++ b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs @@ -1,8 +1,7 @@ using System; -using System.Diagnostics; -using System.IO; using System.Linq; using System.Reflection; +using System.Diagnostics; using GitHub.Helpers; using GitHub.Logging; using Microsoft.VisualStudio.Shell; @@ -22,50 +21,30 @@ internal async static Task RationalizeBindingPathsAsync( JoinableTaskFactory jtf, IServiceProvider serviceProvider) { - log.Information("Searching for duplicate binding paths"); - var assemblyLocation = assembly.Location; + log.Information("Looking for assembly on wrong binding path"); + var bindingPaths = BindingPathUtilities.FindBindingPaths(); - var redundantPaths = BindingPathUtilities.FindRedundantBindingPaths(bindingPaths, assemblyLocation); - if (redundantPaths.Count == 0) + var bindingPath = BindingPathUtilities.FindRedundantBindingPaths(bindingPaths, assembly.Location) + .FirstOrDefault(); + if (bindingPath == null) { - log.Information("No duplicate binding paths found"); + log.Information("No incorrect binding path found"); return; } // Log what has been detected - foreach (var redundantPath in redundantPaths) - { - log.Warning("Found redundant binding path {BindingPath}", redundantPath); - - var redundantFile = Path.Combine(redundantPath, Path.GetFileName(assemblyLocation)); - var loaded = BindingPathUtilities.IsAssemblyLoaded(redundantFile); - if (loaded) - { - log.Error("Assembly has already been loaded from {Location}", redundantFile); - } - } + log.Warning("Found assembly on wrong binding path {BindingPath}", bindingPath); await jtf.SwitchToMainThreadAsync(); - var message = string.Format(@"Redundant binding path found at: + var message = string.Format(@"Found assembly on wrong binding path: {0} -Would like to: -[Abort] - Learn more about this issue -[Retry] - Attempt a fix (this might not work) -[Ignore] - Don't do anything -", redundantPaths.First()); - var action = VsShellUtilities.ShowMessageBox(serviceProvider, message, "Redundant Binding Path", OLEMSGICON.OLEMSGICON_WARNING, - OLEMSGBUTTON.OLEMSGBUTTON_ABORTRETRYIGNORE, OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_SECOND); - switch (action) +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 { - case 3: // Abort - Process.Start("https://github.com/github/VisualStudio/issues/1995"); - break; - case 4: // Retry - Try to fix - BindingPathUtilities.RemoveRedundantBindingPaths(bindingPaths, assemblyLocation, redundantPaths); - break; - case 5: // Ignore - break; + Process.Start("https://github.com/github/VisualStudio/issues/1995"); } } } diff --git a/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs b/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs index ea97fdc97a..d5a1ea4149 100644 --- a/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs +++ b/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs @@ -1,13 +1,61 @@ -using System; -using System.IO; +using System.IO; +using System.Reflection; using System.Collections.Generic; using GitHub.Helpers; using NUnit.Framework; -using NSubstitute; -using Serilog; public static class BindingPathUtilitiesTests { + public class TheFindAssemblyWithDifferentCodeBaseMethod + { + [Test] + public void Current_Assembly_Has_No_Twin() + { + var assembly = GetType().Assembly; + + var foundAssembly = BindingPathUtilities.FindAssemblyWithDifferentCodeBase(assembly); + + Assert.That(foundAssembly, Is.Null); + } + + [Test] + public void Assembly_Loaded_From_Bytes_Has_Twin() + { + var expectAssembly = GetType().Assembly; + var bytes = File.ReadAllBytes(expectAssembly.Location); + var assembly = Assembly.Load(bytes); + + var foundAssembly = BindingPathUtilities.FindAssemblyWithDifferentCodeBase(assembly); + + Assert.That(foundAssembly, Is.EqualTo(expectAssembly)); + } + } + + public class TheFindLoadedAssemblyWithSameName + { + [Test] + public void Mscorlib_Has_No_Twin() + { + var assembly = typeof(object).Assembly; + + var foundAssembly = BindingPathUtilities.FindLoadedAssemblyWithSameName(assembly); + + Assert.That(foundAssembly, Is.Null); + } + + [Test] + public void Assembly_Loaded_From_Bytes_Has_Twin() + { + var expectAssembly = typeof(BindingPathUtilities).Assembly; + var bytes = File.ReadAllBytes(expectAssembly.Location); + var assembly = Assembly.Load(bytes); + + var foundAssembly = BindingPathUtilities.FindLoadedAssemblyWithSameName(assembly); + + Assert.That(foundAssembly, Is.EqualTo(expectAssembly)); + } + } + public class TheRationalizeBindingPathsMethod { [TestCase] From 2212a1f7450573a68da448c2413c63bd898a616e Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Sat, 20 Oct 2018 00:56:41 +0100 Subject: [PATCH 11/20] Remove redundant code --- .../Helpers/BindingPathUtilities.cs | 59 +---------- .../Helpers/BindingPathUtilitiesTests.cs | 99 ++----------------- 2 files changed, 9 insertions(+), 149 deletions(-) diff --git a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs index 8d26110850..5ef21a8c85 100644 --- a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs +++ b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs @@ -3,8 +3,6 @@ using System.Linq; using System.Reflection; using System.Collections.Generic; -using GitHub.Logging; -using Serilog; namespace GitHub.Helpers { @@ -27,28 +25,12 @@ namespace GitHub.Helpers /// 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 + /// This is a workaround for that issue. The /// method will check to see if a reference assembly could be loaded from an alternative - /// binding path. It will remove any alternative paths that is finds so that XAML or - /// .imagemanifest won't end up loading the wrong assembly. + /// binding path. It will return any alternative paths that is finds. /// public static class BindingPathUtilities { - static readonly ILogger log = LogManager.ForContext(typeof(BindingPathUtilities)); - - /// - /// Remove alternative binding path that might have been installed by an AllUsers extension. - /// - /// A list of binding paths to rationalize - /// A reference assembly that has been loaded from the correct path. - /// True if binding path was removed. - public static bool RationalizeBindingPaths(List bindingPaths, string assemblyLocation) - { - var redundantBindingPaths = FindRedundantBindingPaths(bindingPaths, assemblyLocation); - RemoveRedundantBindingPaths(bindingPaths, assemblyLocation, redundantBindingPaths); - return redundantBindingPaths.Any(); - } - /// /// Find any alternative binding path that might have been installed by an AllUsers extension. /// @@ -66,36 +48,6 @@ public static IList FindRedundantBindingPaths(List bindingPaths, .ToList(); } - /// - /// Check to see if an assembly is already in memory. - /// - /// The assembly to look for. - /// True if assembly has already been loaded. - public static bool IsAssemblyLoaded(string assemblyLocation) - { - var prefix = Path.GetFileNameWithoutExtension(assemblyLocation) + ","; - return AppDomain.CurrentDomain.GetAssemblies() - .Where(a => a.FullName.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) - .Where(a => a.Location.Equals(assemblyLocation, StringComparison.OrdinalIgnoreCase)) - .Any(); - } - - public static Assembly FindAssemblyWithDifferentCodeBase(Assembly assembly) - { - var simpleName = assembly.GetName().Name; - var resolvedAssembly = Assembly.Load(simpleName); - return resolvedAssembly != assembly ? resolvedAssembly : null; - } - - public static Assembly FindLoadedAssemblyWithSameName(Assembly assembly) - { - var prefix = assembly.GetName().Name + ','; - return AppDomain.CurrentDomain.GetAssemblies() - .Where(a => a.FullName.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) - .Where(a => a != assembly) - .FirstOrDefault(); - } - /// /// Use reflection to find Visual Studio's list of binding paths. /// @@ -107,12 +59,5 @@ public static List FindBindingPaths() var bindingPaths = property?.GetValue(manager) as List; return bindingPaths ?? new List(0); } - - public static void RemoveRedundantBindingPaths(List bindingPaths, string assemblyLocation, - IList redundantBindingPaths) - { - redundantBindingPaths - .ForEach(p => bindingPaths.Remove(p)); - } } } diff --git a/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs b/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs index d5a1ea4149..8d135878ce 100644 --- a/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs +++ b/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs @@ -1,65 +1,14 @@ using System.IO; -using System.Reflection; using System.Collections.Generic; using GitHub.Helpers; using NUnit.Framework; public static class BindingPathUtilitiesTests { - public class TheFindAssemblyWithDifferentCodeBaseMethod - { - [Test] - public void Current_Assembly_Has_No_Twin() - { - var assembly = GetType().Assembly; - - var foundAssembly = BindingPathUtilities.FindAssemblyWithDifferentCodeBase(assembly); - - Assert.That(foundAssembly, Is.Null); - } - - [Test] - public void Assembly_Loaded_From_Bytes_Has_Twin() - { - var expectAssembly = GetType().Assembly; - var bytes = File.ReadAllBytes(expectAssembly.Location); - var assembly = Assembly.Load(bytes); - - var foundAssembly = BindingPathUtilities.FindAssemblyWithDifferentCodeBase(assembly); - - Assert.That(foundAssembly, Is.EqualTo(expectAssembly)); - } - } - - public class TheFindLoadedAssemblyWithSameName - { - [Test] - public void Mscorlib_Has_No_Twin() - { - var assembly = typeof(object).Assembly; - - var foundAssembly = BindingPathUtilities.FindLoadedAssemblyWithSameName(assembly); - - Assert.That(foundAssembly, Is.Null); - } - - [Test] - public void Assembly_Loaded_From_Bytes_Has_Twin() - { - var expectAssembly = typeof(BindingPathUtilities).Assembly; - var bytes = File.ReadAllBytes(expectAssembly.Location); - var assembly = Assembly.Load(bytes); - - var foundAssembly = BindingPathUtilities.FindLoadedAssemblyWithSameName(assembly); - - Assert.That(foundAssembly, Is.EqualTo(expectAssembly)); - } - } - public class TheRationalizeBindingPathsMethod { [TestCase] - public void Alternative_Path_Is_Removed_From_BinsingPaths() + public void Redundant_Binding_Paths_Contains_Alternative_Path() { var alternativeLocation = GetType().Assembly.Location; var fileName = Path.GetFileName(alternativeLocation); @@ -68,23 +17,22 @@ public void Alternative_Path_Is_Removed_From_BinsingPaths() var assemblyLocation = Path.Combine(assemblyDir, fileName); var bindingPaths = new List { alternativeDir, assemblyDir }; - var removed = BindingPathUtilities.RationalizeBindingPaths(bindingPaths, assemblyLocation); + var paths = BindingPathUtilities.FindRedundantBindingPaths(bindingPaths, assemblyLocation); - Assert.That(bindingPaths, Contains.Item(assemblyDir)); - Assert.That(bindingPaths, Does.Not.Contain(alternativeDir)); - Assert.That(removed, Is.True); + Assert.That(paths, Contains.Item(alternativeDir)); + Assert.That(paths, Does.Not.Contain(assemblyDir)); } [TestCase] - public void Return_False_When_No_BindingPath_Was_Removed() + public void No_Redundant_Binding_Paths() { var assemblyLocation = GetType().Assembly.Location; var assemblyDir = Path.GetDirectoryName(assemblyLocation); var bindingPaths = new List { assemblyDir }; - var removed = BindingPathUtilities.RationalizeBindingPaths(bindingPaths, assemblyLocation); + var paths = BindingPathUtilities.FindRedundantBindingPaths(bindingPaths, assemblyLocation); - Assert.That(removed, Is.False); + Assert.That(paths, Does.Not.Contain(assemblyDir)); } } @@ -98,37 +46,4 @@ public void Return_Empty_When_Not_In_Visual_Studio() Assert.IsEmpty(bindingPaths); } } - - public class TheIsAssemblyLoadedMethod - { - [TestCase] - public void Check_Executing_Assumbly_Is_Loaded() - { - var location = GetType().Assembly.Location; - - var isLoaded = BindingPathUtilities.IsAssemblyLoaded(location); - - Assert.That(isLoaded, Is.True); - } - - [TestCase] - public void Check_Executing_Assumbly_Is_Loaded_When_Case_Differs() - { - var location = GetType().Assembly.Location.ToUpperInvariant(); - - var isLoaded = BindingPathUtilities.IsAssemblyLoaded(location); - - Assert.That(isLoaded, Is.True); - } - - [Test] - public void Check_Assembly_At_Unknown_Location_Not_Loaded() - { - var location = @"c:\unknown.dll"; - - var isLoaded = BindingPathUtilities.IsAssemblyLoaded(location); - - Assert.That(isLoaded, Is.False); - } - } } From 0f29425f4c6de904c4de13710dca98169d7c8037 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 22 Oct 2018 13:09:26 +0100 Subject: [PATCH 12/20] Use settings manager instead of reflection Use ShellSettingsManager instead of reflection to find BindingPaths. --- .../Helpers/BindingPathUtilities.cs | 26 ++++++++++++------- .../Helpers/BindingPathHelper.cs | 2 +- .../Helpers/BindingPathUtilitiesTests.cs | 11 -------- 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs index 5ef21a8c85..be0281dbd8 100644 --- a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs +++ b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs @@ -1,8 +1,9 @@ using System; using System.IO; using System.Linq; -using System.Reflection; using System.Collections.Generic; +using Microsoft.VisualStudio.Settings; +using Microsoft.VisualStudio.Shell.Settings; namespace GitHub.Helpers { @@ -37,7 +38,7 @@ public static class BindingPathUtilities /// A list of binding paths to search /// A reference assembly that has been loaded from the correct path. /// A list of redundant binding paths. - public static IList FindRedundantBindingPaths(List bindingPaths, string assemblyLocation) + public static IList FindRedundantBindingPaths(IEnumerable bindingPaths, string assemblyLocation) { var fileName = Path.GetFileName(assemblyLocation); return bindingPaths @@ -49,15 +50,22 @@ public static IList FindRedundantBindingPaths(List bindingPaths, } /// - /// Use reflection to find Visual Studio's list of binding paths. + /// Find Visual Studio's list of binding paths. /// - /// A live list of binding paths or an empty list if not running in Visual Studio. - public static List FindBindingPaths() + /// A list of binding paths. + public static IEnumerable FindBindingPaths(IServiceProvider serviceProvider) { - var manager = AppDomain.CurrentDomain.DomainManager; - var property = manager?.GetType().GetProperty("BindingPaths", BindingFlags.NonPublic | BindingFlags.Instance); - var bindingPaths = property?.GetValue(manager) as List; - return bindingPaths ?? new List(0); + 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; + } + } } } } diff --git a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs index 39c74f56c3..a77da76887 100644 --- a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs +++ b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs @@ -23,7 +23,7 @@ internal async static Task RationalizeBindingPathsAsync( { log.Information("Looking for assembly on wrong binding path"); - var bindingPaths = BindingPathUtilities.FindBindingPaths(); + var bindingPaths = BindingPathUtilities.FindBindingPaths(serviceProvider); var bindingPath = BindingPathUtilities.FindRedundantBindingPaths(bindingPaths, assembly.Location) .FirstOrDefault(); if (bindingPath == null) diff --git a/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs b/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs index 8d135878ce..2a5d5aba38 100644 --- a/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs +++ b/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs @@ -35,15 +35,4 @@ public void No_Redundant_Binding_Paths() Assert.That(paths, Does.Not.Contain(assemblyDir)); } } - - public class TheFindBindingPathsMethod - { - [TestCase] - public void Return_Empty_When_Not_In_Visual_Studio() - { - var bindingPaths = BindingPathUtilities.FindBindingPaths(); - - Assert.IsEmpty(bindingPaths); - } - } } From 221f19cd9ea6db685d786c3b9ab26ce132b4a3a8 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 22 Oct 2018 13:15:55 +0100 Subject: [PATCH 13/20] Check for DEBUG and catch in separate method CA was failing because InitializeAsync wasn't awaiting anything on a non-DEBUG build. --- src/GitHub.VisualStudio/GitHubPackage.cs | 28 +++++++++++++------ .../Helpers/BindingPathHelper.cs | 2 +- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index b3ceb0a2d5..2f7e177d21 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -169,14 +169,7 @@ public sealed class ServiceProviderPackage : AsyncPackage, IServiceProviderPacka protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { -#if DEBUG - // 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/1995 - await BindingPathHelper.RationalizeBindingPathsAsync( - GetType().Assembly, JoinableTaskFactory, this); -#endif - + await CheckBindingPathsAsync(); AddService(typeof(IGitHubServiceProvider), CreateService, true); AddService(typeof(IVSGitExt), CreateService, true); AddService(typeof(IUsageTracker), CreateService, true); @@ -186,6 +179,25 @@ await BindingPathHelper.RationalizeBindingPathsAsync( AddService(typeof(IPackageSettings), CreateService, true); } + async Task CheckBindingPathsAsync() + { +#if DEBUG + 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/1995 + await BindingPathHelper.CheckBindingPathsAsync(GetType().Assembly, JoinableTaskFactory, this); + } + catch (Exception e) + { + log.Error(e, nameof(CheckBindingPathsAsync)); + } +#else + return Task.CompletedTask; +#endif + } + public async Task ShowGitHubPane() { var pane = ShowToolWindow(new Guid(GitHubPane.GitHubPaneGuid)); diff --git a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs index a77da76887..ac1b70ddbe 100644 --- a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs +++ b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs @@ -16,7 +16,7 @@ class BindingPathHelper { static readonly ILogger log = LogManager.ForContext(); - internal async static Task RationalizeBindingPathsAsync( + internal async static Task CheckBindingPathsAsync( Assembly assembly, JoinableTaskFactory jtf, IServiceProvider serviceProvider) From 7b312d39335def45461ef57ae478997181defe1d Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 22 Oct 2018 13:18:13 +0100 Subject: [PATCH 14/20] Add xmldoc comments for BindingPathHelper --- src/GitHub.VisualStudio/GitHubPackage.cs | 1 + src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 2f7e177d21..a215f2677f 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -170,6 +170,7 @@ public sealed class ServiceProviderPackage : AsyncPackage, IServiceProviderPacka protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { await CheckBindingPathsAsync(); + AddService(typeof(IGitHubServiceProvider), CreateService, true); AddService(typeof(IVSGitExt), CreateService, true); AddService(typeof(IUsageTracker), CreateService, true); diff --git a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs index ac1b70ddbe..ed2150a577 100644 --- a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs +++ b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs @@ -12,6 +12,13 @@ namespace GitHub.VisualStudio.Helpers { + /// + // 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/1995 + /// class BindingPathHelper { static readonly ILogger log = LogManager.ForContext(); From 7cb1dea45ddbf9e1e140b27419831a572f76beed Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 22 Oct 2018 16:22:15 +0100 Subject: [PATCH 15/20] Always call FindBindingPaths on UI thread --- src/GitHub.VisualStudio/GitHubPackage.cs | 3 ++- src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs | 9 ++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index a215f2677f..c35d8496cc 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -188,7 +188,8 @@ async Task CheckBindingPathsAsync() // 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/1995 - await BindingPathHelper.CheckBindingPathsAsync(GetType().Assembly, JoinableTaskFactory, this); + await JoinableTaskFactory.SwitchToMainThreadAsync(); + BindingPathHelper.CheckBindingPaths(GetType().Assembly, this); } catch (Exception e) { diff --git a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs index ed2150a577..b5ab618121 100644 --- a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs +++ b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs @@ -6,9 +6,7 @@ using GitHub.Logging; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Shell.Interop; -using Microsoft.VisualStudio.Threading; using Serilog; -using Task = System.Threading.Tasks.Task; namespace GitHub.VisualStudio.Helpers { @@ -23,13 +21,11 @@ class BindingPathHelper { static readonly ILogger log = LogManager.ForContext(); - internal async static Task CheckBindingPathsAsync( - Assembly assembly, - JoinableTaskFactory jtf, - IServiceProvider serviceProvider) + internal static void CheckBindingPaths(Assembly assembly, IServiceProvider serviceProvider) { log.Information("Looking for assembly on wrong binding path"); + ThreadHelper.CheckAccess(); var bindingPaths = BindingPathUtilities.FindBindingPaths(serviceProvider); var bindingPath = BindingPathUtilities.FindRedundantBindingPaths(bindingPaths, assembly.Location) .FirstOrDefault(); @@ -42,7 +38,6 @@ internal async static Task CheckBindingPathsAsync( // Log what has been detected log.Warning("Found assembly on wrong binding path {BindingPath}", bindingPath); - await jtf.SwitchToMainThreadAsync(); var message = string.Format(@"Found assembly on wrong binding path: {0} From 7d5631b06135847377c4eae16d2e3ebf2e189395 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 22 Oct 2018 16:24:07 +0100 Subject: [PATCH 16/20] Open issue with how to fix https://github.com/github/VisualStudio/issues/2006 --- src/GitHub.VisualStudio/GitHubPackage.cs | 2 +- src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index c35d8496cc..e96bef064b 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -187,7 +187,7 @@ async Task CheckBindingPathsAsync() { // 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/1995 + // See: https://github.com/github/VisualStudio/issues/2006 await JoinableTaskFactory.SwitchToMainThreadAsync(); BindingPathHelper.CheckBindingPaths(GetType().Assembly, this); } diff --git a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs index b5ab618121..4fa30a8b2b 100644 --- a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs +++ b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs @@ -46,7 +46,7 @@ internal static void CheckBindingPaths(Assembly assembly, IServiceProvider servi OLEMSGBUTTON.OLEMSGBUTTON_YESNO, OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST); if (action == 6) // Yes = 6, No = 7 { - Process.Start("https://github.com/github/VisualStudio/issues/1995"); + Process.Start("https://github.com/github/VisualStudio/issues/2006"); } } } From 0cb34010d3c175f416a612e9166cf14c4445563d Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 23 Oct 2018 18:47:34 +0100 Subject: [PATCH 17/20] Fix CA errors on CheckBindingPathsAsync --- src/GitHub.VisualStudio/GitHubPackage.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index e96bef064b..741b9ccfe8 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -180,9 +180,9 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke AddService(typeof(IPackageSettings), CreateService, true); } +#if DEBUG async Task CheckBindingPathsAsync() { -#if DEBUG try { // When running in the Exp instance, ensure there is only one active binding path. @@ -195,10 +195,10 @@ async Task CheckBindingPathsAsync() { log.Error(e, nameof(CheckBindingPathsAsync)); } + } #else - return Task.CompletedTask; + Task CheckBindingPathsAsync() => Task.CompletedTask; #endif - } public async Task ShowGitHubPane() { From 8bc87453aed3a7be96bf3f0635baf5764126d7f4 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 23 Oct 2018 19:00:56 +0100 Subject: [PATCH 18/20] Consolidate BindingPath(Utilities|Helper) Merge BindingPathUtilities into BindingPathHelper. --- .../Helpers/BindingPathUtilities.cs | 71 ------------------- .../Helpers/BindingPathHelper.cs | 68 ++++++++++++++++-- .../Helpers/BindingPathHelperTests.cs} | 10 +-- 3 files changed, 67 insertions(+), 82 deletions(-) delete mode 100644 src/GitHub.Exports/Helpers/BindingPathUtilities.cs rename test/{GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs => GitHub.VisualStudio.UnitTests/Helpers/BindingPathHelperTests.cs} (76%) diff --git a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs b/src/GitHub.Exports/Helpers/BindingPathUtilities.cs deleted file mode 100644 index be0281dbd8..0000000000 --- a/src/GitHub.Exports/Helpers/BindingPathUtilities.cs +++ /dev/null @@ -1,71 +0,0 @@ -using System; -using System.IO; -using System.Linq; -using System.Collections.Generic; -using Microsoft.VisualStudio.Settings; -using Microsoft.VisualStudio.Shell.Settings; - -namespace GitHub.Helpers -{ - /// - /// This a workaround for extensions that define a ProvideBindingPath attribute and - /// install for AllUsers. - /// - /// - /// 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 - /// 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. - /// - public static class BindingPathUtilities - { - /// - /// Find any alternative binding path that might have been installed by an AllUsers extension. - /// - /// A list of binding paths to search - /// A reference assembly that has been loaded from the correct path. - /// A list of redundant binding paths. - public static IList FindRedundantBindingPaths(IEnumerable 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(); - } - - /// - /// Find Visual Studio's list of binding paths. - /// - /// A list of binding paths. - public static IEnumerable 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; - } - } - } - } -} diff --git a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs index 4fa30a8b2b..e83d59121f 100644 --- a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs +++ b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs @@ -1,23 +1,43 @@ using System; +using System.IO; using System.Linq; using System.Reflection; using System.Diagnostics; -using GitHub.Helpers; +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 { /// - // 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. + /// This a workaround for extensions that define a ProvideBindingPath attribute and + /// install for AllUsers. /// /// + /// 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 + /// 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 /// - class BindingPathHelper + public class BindingPathHelper { static readonly ILogger log = LogManager.ForContext(); @@ -26,8 +46,8 @@ internal static void CheckBindingPaths(Assembly assembly, IServiceProvider servi log.Information("Looking for assembly on wrong binding path"); ThreadHelper.CheckAccess(); - var bindingPaths = BindingPathUtilities.FindBindingPaths(serviceProvider); - var bindingPath = BindingPathUtilities.FindRedundantBindingPaths(bindingPaths, assembly.Location) + var bindingPaths = FindBindingPaths(serviceProvider); + var bindingPath = FindRedundantBindingPaths(bindingPaths, assembly.Location) .FirstOrDefault(); if (bindingPath == null) { @@ -49,5 +69,41 @@ internal static void CheckBindingPaths(Assembly assembly, IServiceProvider servi Process.Start("https://github.com/github/VisualStudio/issues/2006"); } } + + /// + /// Find any alternative binding path that might have been installed by an AllUsers extension. + /// + /// A list of binding paths to search + /// A reference assembly that has been loaded from the correct path. + /// A list of redundant binding paths. + public static IList FindRedundantBindingPaths(IEnumerable 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(); + } + + /// + /// Find Visual Studio's list of binding paths. + /// + /// A list of binding paths. + public static IEnumerable 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; + } + } + } } } diff --git a/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs b/test/GitHub.VisualStudio.UnitTests/Helpers/BindingPathHelperTests.cs similarity index 76% rename from test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs rename to test/GitHub.VisualStudio.UnitTests/Helpers/BindingPathHelperTests.cs index 2a5d5aba38..a650ff87a7 100644 --- a/test/GitHub.Exports.UnitTests/Helpers/BindingPathUtilitiesTests.cs +++ b/test/GitHub.VisualStudio.UnitTests/Helpers/BindingPathHelperTests.cs @@ -1,11 +1,11 @@ using System.IO; using System.Collections.Generic; -using GitHub.Helpers; +using GitHub.VisualStudio.Helpers; using NUnit.Framework; -public static class BindingPathUtilitiesTests +public static class BindingPathHelperTests { - public class TheRationalizeBindingPathsMethod + public class TheFindRedundantBindingPathsMethod { [TestCase] public void Redundant_Binding_Paths_Contains_Alternative_Path() @@ -17,7 +17,7 @@ public void Redundant_Binding_Paths_Contains_Alternative_Path() var assemblyLocation = Path.Combine(assemblyDir, fileName); var bindingPaths = new List { alternativeDir, assemblyDir }; - var paths = BindingPathUtilities.FindRedundantBindingPaths(bindingPaths, assemblyLocation); + var paths = BindingPathHelper.FindRedundantBindingPaths(bindingPaths, assemblyLocation); Assert.That(paths, Contains.Item(alternativeDir)); Assert.That(paths, Does.Not.Contain(assemblyDir)); @@ -30,7 +30,7 @@ public void No_Redundant_Binding_Paths() var assemblyDir = Path.GetDirectoryName(assemblyLocation); var bindingPaths = new List { assemblyDir }; - var paths = BindingPathUtilities.FindRedundantBindingPaths(bindingPaths, assemblyLocation); + var paths = BindingPathHelper.FindRedundantBindingPaths(bindingPaths, assemblyLocation); Assert.That(paths, Does.Not.Contain(assemblyDir)); } From 5ceaefcaf25e08fb543152fd0b24da778ceafcbc Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 23 Oct 2018 19:01:35 +0100 Subject: [PATCH 19/20] Add GlobalSuppressions for unit tests --- .../GlobalSuppressions.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 test/GitHub.VisualStudio.UnitTests/GlobalSuppressions.cs diff --git a/test/GitHub.VisualStudio.UnitTests/GlobalSuppressions.cs b/test/GitHub.VisualStudio.UnitTests/GlobalSuppressions.cs new file mode 100644 index 0000000000..42af364f88 --- /dev/null +++ b/test/GitHub.VisualStudio.UnitTests/GlobalSuppressions.cs @@ -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")] From e782108aa45b3beb37729d82ea43f580ccadd77d Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 24 Oct 2018 11:23:26 +0100 Subject: [PATCH 20/20] Fix CA errors Explicitly specify CultureInfo.CurrentCulture. Make class BindingPathHelper static. --- src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs index e83d59121f..da47e8c755 100644 --- a/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs +++ b/src/GitHub.VisualStudio/Helpers/BindingPathHelper.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Reflection; using System.Diagnostics; +using System.Globalization; using System.Collections.Generic; using GitHub.Logging; using Microsoft.VisualStudio.Shell; @@ -37,9 +38,9 @@ namespace GitHub.VisualStudio.Helpers /// binding path. It will return any alternative paths that is finds. /// See https://github.com/github/VisualStudio/issues/1995 /// - public class BindingPathHelper + public static class BindingPathHelper { - static readonly ILogger log = LogManager.ForContext(); + static readonly ILogger log = LogManager.ForContext(typeof(BindingPathHelper)); internal static void CheckBindingPaths(Assembly assembly, IServiceProvider serviceProvider) { @@ -58,7 +59,7 @@ internal static void CheckBindingPaths(Assembly assembly, IServiceProvider servi // Log what has been detected log.Warning("Found assembly on wrong binding path {BindingPath}", bindingPath); - var message = string.Format(@"Found assembly on wrong binding path: + var message = string.Format(CultureInfo.CurrentCulture, @"Found assembly on wrong binding path: {0} Would you like to learn more about this issue?", bindingPath);