8000 [SPIKE] Stop TeamFoundation MEF exceptions when installing by jcansdale · Pull Request #970 · github/VisualStudio · GitHub
[go: up one dir, main page]

Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

[SPIKE] Stop TeamFoundation MEF exceptions when installing #970

Closed
wants to merge 23 commits into from

Conversation

jcansdale
Copy link
Collaborator

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 May 2, 2017 15:31
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 16:45
@shana
Copy link
Contributor
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)
Copy link
Contributor

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?

Copy link
Collaborator Author

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>
Copy link
Contributor

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?

Copy link
Collaborator Author

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.

Copy link
Contributor

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
Copy link
Contributor

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.

shana
shana previously requested changes May 3, 2017

public void Dispose()
{
AppDomain.CurrentDomain.AssemblyResolve -= CurrentDomain_AssemblyResolve;
Copy link
Contributor

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.

@jcansdale
Copy link
Collaborator Author

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

@shana
Copy link
Contributor
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. 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.

@grokys
Copy link
Contributor
grokys commented Dec 11, 2017

@jcansdale what is the status of this PR?

@jcansdale
Copy link
Collaborator Author

@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

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 August 23, 2018 13:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lots of "Catalog construction errors" in Microsoft.VisualStudio.Default.err
3 participants
0