Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPIKE] Stop TeamFoundation MEF exceptions when installing #970

Closed
wants to merge 23 commits into from

Conversation

@jcansdale
Copy link
Collaborator

@jcansdale jcansdale commented May 2, 2017

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 the GitHub.VisualStudio assembly. MEF attributes have been left on the GitHub.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. The GitHub.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.

jcansdale added 15 commits Apr 19, 2017
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.
@jcansdale jcansdale requested review from grokys and shana May 2, 2017
@shana
Copy link
Collaborator

@shana shana commented May 3, 2017

If the GitHub.TeamFoundation.14 project is no longer necessary, then we should probably have one GitHub.TeamFoundation project and not a GitHub.TeamFoundation.15 project, since the number in the name no longer matters?

object progress = null)
public async Task Clone(string cloneUrl, string clonePath, bool recurseSubmodules, object progress = null)
{
#if (!TEAMEXPLORER14)

This comment has been minimized.

@shana

shana May 3, 2017
Collaborator

If we're only using one project now, do we need these ifdefs?

This comment has been minimized.

@jcansdale

jcansdale May 3, 2017
Author Collaborator

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>

This comment has been minimized.

@shana

shana May 3, 2017
Collaborator

TE doesn't ship backwards compatible DLLs, so how does referencing version 15 and calling it 14 work, exactly?

This comment has been minimized.

@jcansdale

jcansdale May 3, 2017
Author Collaborator

It doesn't. I'm not sure what's happening there. We'll only be referencing the 15.0 version.

This comment has been minimized.

@shana

shana May 3, 2017
Collaborator

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

This comment has been minimized.

@shana

shana May 3, 2017
Collaborator

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;

This comment has been minimized.

@shana

shana May 3, 2017
Collaborator

This should follow the Dispose pattern we have everywhere else.

jcansdale added 8 commits May 3, 2017
Change to use `TeamFoundationResolver` import attribute with `IResolver` to signal that the import is special.
@jcansdale
Copy link
Collaborator Author

@jcansdale jcansdale commented May 4, 2017

@shana I've addressed all of the issues you highlighted. I also did some refactoring/separating of concerns. Could you take another look? 😄

@shana shana force-pushed the master branch from 49ba6cf to a82bce9 Aug 16, 2017
@shana shana dismissed their stale review Nov 17, 2017

no longer valid

@shana
Copy link
Collaborator

@shana shana commented Nov 17, 2017

@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. 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.

@grokys
Copy link
Contributor

@grokys grokys commented Dec 11, 2017

@jcansdale what is the status of this PR?

@jcansdale
Copy link
Collaborator Author

@jcansdale jcansdale commented Dec 12, 2017

@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 [SPIKE] the the time being, signifying that it's an experiment / proof of concept.

@jcansdale jcansdale changed the title Stop TeamFoundation MEF exceptions when installing [SPIKE] Stop TeamFoundation MEF exceptions when installing Dec 12, 2017
@jcansdale
Copy link
Collaborator Author

@jcansdale jcansdale commented Aug 23, 2018

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.

@jcansdale jcansdale closed this Aug 23, 2018
@jcansdale jcansdale deleted the jcansdale/unify-TeamFoundation branch Aug 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.