Ensure only one binding path for extension

[jcansdale]

Check that only one binding path is active and warn when extra binding paths might cause issues.

What this PR does

• Find list of BindingPaths in Visual Studio configuration using ShellSettingsManager
• Check for duplicate binding paths that could load GitHub.VisualStudio.dll
• Show warning dialog when extra binding paths might cause issues
• Offer instructions how to resolve this issue (link here)
• Only execute on DEBUG builds (no need to run in production)

How to test

1. Install GitHub for Visual Studio from Marketplace
2. Load GitHub for Visual Studio solution
3. Launch and install extension
4. Click View > Other Windows > GitHub
5. Expect the following to appear in the log
   
6. Click Yes and follow instructions
7. Repeat from 2. and check that no dialog appears

xmldoc

VisualStudio/src/GitHub.Exports/Helpers/BindingPathUtilities.cs

Lines 10 to 32 in 7d5631b

	/// <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.
	/// </remarks>

Fixes #1995

[codecov]

Codecov Report

Merging #1996 into master will decrease coverage by 1.47%.
The diff coverage is 23.52%.

@@            Coverage Diff             @@
##           master    #1996      +/-   ##
==========================================
- Coverage    41.7%   40.22%   -1.48%     
==========================================
  Files         373      406      +33     
  Lines       16126    17377    +1251     
  Branches     2211     2393     +182     
==========================================
+ Hits         6725     6990     +265     
- Misses       8890     9859     +969     
- Partials      511      528      +17

Impacted Files	Coverage Δ
src/GitHub.VisualStudio/GitHubPackage.cs	0% <0%> (ø)
...c/GitHub.VisualStudio/Helpers/BindingPathHelper.cs	25% <25%> (ø)
...nlineReviews/Services/PullRequestSessionManager.cs	82.23% <0%> (-1.02%)	⬇️
...c/GitHub.InlineReviews/Tags/InlineCommentTagger.cs	79.64% <0%> (-0.89%)	⬇️
...nlineReviews/Services/PullRequestSessionService.cs	16.7% <0%> (-0.41%)	⬇️
...rc/GitHub.VisualStudio/Commands/OpenLinkCommand.cs	0% <0%> (ø)
...Hub.VisualStudio/Commands/ShowGitHubPaneCommand.cs	0% <0%> (ø)
...o/Commands/GoToSolutionOrPullRequestFileCommand.cs	0% <0%> (ø)
src/GitHub.VisualStudio/Services/UsageTracker.cs	92.53% <0%> (ø)
... and 28 more

[grokys]

This is using reflection to change private state of code we don't own, which sounds to me like a recipe for disaster down the line. Even though it's only doing it in debug builds, it's quite possibly something that will come back to bite us and cause hours of debugging.

@madskristensen @AArnott does this sound like something we should be doing?

[jcansdale]

👋 @sharwell @josetr,

I wondered if you had any thoughts on this or have any suggestions for a cleaner fix?

Do you know if it's possible to completely disable an AllUsers extension in the Exp instance without impacting the regular instance?

[AArnott]

Thanks for checking. I'm pretty sure we really wouldn't want this to ever ship to customers. As for your own debugging experience, I guess that's up to you as I can't speak to how unstable this would make your code.

@Michael-Eng FYI

[jcansdale]

@grokys here's a non-hacky version for you that doesn't use reflection. 😉

There is a new feature in Visual Studio 2017 that lets you disable AllUsers extensions in just the experimental instance. I'd recommend doing this for NCrunch and any other AllUsers extensions you might have installed (less symbols to load when debugging).

[grokys]

Much happier with this ;) Just one question but LGTM!

[grokys]

This class only seems to be used by BindingPathHelper - can these methods just be put there?

[jcansdale]

It was just painful to write units tests that target, GitHub.VisualStudio because of the compile time! Now that it's stable, I've merged into BindingPathHelper as suggested. 👍