-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[SPIKE] Stop TeamFoundation MEF exceptions when installing #970
Conversation
The TeamExplorerSections seem to work. The TeamExplorerNavigationItems aren't working.
Fix to make NavigationItems visible again (for some reason CreationPolicy.Shared on the factory stopped them from showing).
It's okay to use public consts in attributes because this doesn't force the containing type to load.
Leaving the `Export` attributes doesn't cause any harm and not touching lots of source files is less clutter for code review.
Probe BindingPath for TeamFoundation assemblies. These paths are set here: AppDomain.CurrentDomain.DomainManager.BindingPaths
Added a workaround related to #923 that gives our resolver a chance to run first. Be more specific about which assemblies might be resolved (include PublicKeyToken). They must start with "Microsoft.TeamFoundation." and end with ", PublicKeyToken=b03f5f7f11d50a3a".
Made namespaces consistent with source file locations.
If the |
object progress = null) | ||
public async Task Clone(string cloneUrl, string clonePath, bool recurseSubmodules, object progress = null) | ||
{ | ||
#if (!TEAMEXPLORER14) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're only using one project now, do we need these ifdefs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, we can get rid of all #if
and consolidate to a single project. I just wanted a commit where it worked but hardly touched the GitHub.TeamFoundation.*
projects.
I'll now strip out the unnecessary stuff. 😄
<Reference Include="Microsoft.Expression.Interactions, Version=4.5.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL"> | ||
<HintPath>..\..\packages\Expression.Blend.Sdk.WPF.1.0.1\lib\net45\Microsoft.Expression.Interactions.dll</HintPath> | ||
<Private>True</Private> | ||
</Reference> | ||
<Reference Include="Microsoft.TeamFoundation.Controls, Version=14.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL"> | ||
<SpecificVersion>False</SpecificVersion> | ||
<HintPath>..\..\lib\15.0\Microsoft.TeamFoundation.Controls.dll</HintPath> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TE doesn't ship backwards compatible DLLs, so how does referencing version 15 and calling it 14 work, exactly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't. I'm not sure what's happening there. We'll only be referencing the 15.0
version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried this in VS2015? I'm really curious how this will work given that the sections and navigation items implement Team Explorer interfaces that are not ABI compatible (because DLL versions)
{ | ||
[Export] | ||
[PartCreationPolicy(CreationPolicy.Shared)] | ||
public sealed class TeamFoundationResolver : IDisposable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and the other new MEF exports should be exporting interfaces and not the actual types, for easier testing.
public void Dispose() | ||
{ | ||
AppDomain.CurrentDomain.AssemblyResolve -= CurrentDomain_AssemblyResolve; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should follow the Dispose pattern we have everywhere else.
Change to use `TeamFoundationResolver` import attribute with `IResolver` to signal that the import is special.
@shana I've addressed all of the issues you highlighted. I also did some refactoring/separating of concerns. Could you take another look? 😄 |
@jcansdale Looks like a good time to revisit this PR? One thing that's important to ensure is that this works properly on a machine that only has Visual Studio 2015 installed. 6D40 Machines with both will cause VS2015 to correctly locate assemblies that are only shipped with 2017 and might mask problems with a VS2015-only installation. |
@jcansdale what is the status of this PR? |
@grokys I think this is worth revisiting some time, but it certainly isn't ready to merge. The situation with the two tests of TeamFoundation assemblies completely messes up the MEF log. I wonder if the next version of VS will introduce a 3rd set? I'm going to mark this as |
There was a simpler fix for this issue merged in #1705 for VS 2017. Fixing for VS 2015 proved to be extremely difficult or maybe impossible. |
This PR stops MEF exceptions being thrown as the extension installs. After installing the
Microsoft.VisualStudio.Default.err
file should be completely empty. This makes it a lot easier to stop MEF issues when working on other aspects of the extension.All
GitHub.TeamFoundation
related discovery has been moved to theGitHub.VisualStudio
assembly. MEF attributes have been left on theGitHub.TeamFoundation.*
assemblies, but are no longer used by Visual Studio for discovery. I wanted to touch these projects as little as possible as part of this PR.The
GitHub.TeamFoundation.15
is now used inside VS 2015 and 2017. The enhanced VS 2017 features are enabled in code rather than using#if
directives. TheGitHub.TeamFoundation.14
project is no longer necessary.Enabling support for VS 2015 and 2017 in the same assembly is done using an assembly resolver. The resolver is installed deterministically and doesn't race to be installed on time (which was an issue when we used an assembly resolver for the extension as a whole).
Fixes #954.