8000 bindingRedirects can become out of sync with installed extension · Issue #1217 · 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.

bindingRedirects can become out of sync with installed extension #1217

Closed
jcansdale opened this issue Sep 5, 2017 · 10 comments · Fixed by #1220
Closed

bindingRedirects can become out of sync with installed extension #1217

jcansdale opened this issue Sep 5, 2017 · 10 comments · Fixed by #1220
Labels
needs investigation

Comments

@jcansdale
Copy link
Collaborator
jcansdale commented Sep 5, 2017 •
edited
Loading
  • GitHub Extension for Visual Studio version: v2.3.1.30 (iirc)
  • Visual Studio version: 2017 15.3

There have been a few instances where it looks like the devenv.exe.config file has become out of whack with the installed version of the extension. This has been a problem because we use ProvideCodeBase / ProvideBindingRedirection attributes to ensure that our assemblies can be resolved (without resorting to assembly resolve event hacks).

This was particularly noticeable when I changed to an earlier version of the extension. There was a bindingRedirect in the devenv.exe.config, that was looking for an assembly that no longer existed. The extension failed to work completely.

image

There have also been reports of the GitHub icon missing from the Start Page. This used to happen in the past when the assembly containing the icon couldn't be resolved by its fully qualified name. This might happen if the devenv.exe.config hasn't been updated and the assembly version on disk doesn't match the assembly range in the bindingRedirect.

Here's an example of one of the GitHub extension's entries in devenv.exe.config.

<dependentAssembly>
    <assemblyIdentity name="GitHub.UI" publicKeyToken="bc1bd09f2901c82e" culture="neutral"/>
    <bindingRedirect oldVersion="0.0.0.0-2.3.2.33" newVersion="2.3.2.33"/>
    <codeBase version="2.3.2.33" href="C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\Common7\IDE\Extensions\GitHub\GitHub\GitHub.UI.dll"/>
</dependentAssembly>

If this wan't updated and the version on disk changed to say, 2.3.3.44, Visual Studio wouldn't know where to find the GitHub.UI.dll assembly.

@tinaschrepfer @Michael-Eng,
Is this a known issue or a likely symptom of what has been reported in #1213?
@jcansdale jcansdale mentioned this issue Sep 5, 2017
Closed
@jcansdale
Copy link
Collaborator Author
jcansdale commented Sep 5, 2017 •
edited
Loading

Here is an example of the Start Page in Visual Studio 2017 15.3.3. This is immediately after Visual Studio Enterprise 2017 15.3.3 was installed.

image

Clicking on the GitHub link brings up the Team Explorer, but without any GitHub extension functionality.

Here is the devenv.exe.config file from C:\Users\jamie\AppData\Local\Microsoft\VisualStudio\15.0_886c2a72.
devenv.exe.config.txt.txt

It is missing all GitHub brindingRedirects.

@jcansdale jcansdale mentioned this issue Sep 7, 2017
Merged
@jcansdale
Copy link
Collaborator Author

It turns out the most common reason for the missing icon was our use of an unqualified assembly name (/GitHub.VisualStudio;component) in the GitHub.VisualStudio.imagemanifest file:

VisualStudio/src/GitHub.VisualStudio/GitHub.VisualStudio.imagemanifest

Line 6 in 7d6e8de

<String Name="Resources" Value="/GitHub.VisualStudio;component/Resources/icons" />

Prior to VS 15.3 this wasn't an issue because the GitHub.VisualStudio was always loaded by the time the icons were needed. It appears there was a change to Team Explorer assembly load ordering which caused this issue to surface.

Unfortunately changing this reference to a fully qualified assembly name doesn't seem to fix it. I think this is because we're using unqualified assembly names in many of our XAML file. It's a shame they don't get qualified when they're compiled down to BAML. I'm seem many people complain about this when researching the issue.

What is the best practice when dealing with the .imagemanifest files?

@Newrad0603
Copy link

The ImageService uses an image's source uri value to load the image and VS's assembly resolution should handle finding and loading the dll based on the uri. If the dll is failing to load it means VS can't find the dll which likely means you're missing a codebase entry. How do you get codebase entries into our exe config? I couldn't find a pkgdef file...

@jcansdale
Copy link
Collaborator Author

@Newrad0603,

How do you get codebase entries into our exe config? I couldn't find a pkgdef file...

The ProvideCodeBase attributes are defined here:
https://github.com/github/VisualStudio/blob/master/src/GitHub.VisualStudio/Properties/AssemblyInfo.cs

We don't actually have a ProvideCodeBase for the GitHub.VisualStudio assembly. The reason is that once an assembly has a ProvideCodeBase, this attribute must also be used on all of the assemblies it references. This leads to an explosion of ProvideCodeBase attributes, including ones for assemblies that we don't actually own.

Earlier today I tried putting the [ProvideBindingPath] attribute on one of our packages. It looks like this might actually resolve the issue. Do you know if there are any reasons why we shouldn't use this attribute?

@jcansdale jcansdale mentioned this issue Sep 21, 2017
Merged
@Newrad0603
Copy link

Here is the thread from the last time this issue came up:
#655 (comment)

During my testing, I found if the merged devenv.exe.config contained codebase entries for the GitHub.UI and GitHub.Exports dlls, then the image load worked. Did something change in that regard, of were new dependencies added?

ProviderBindingPath is a workable option, it's just more of a shotgun approach as opposed to a scalpel approach. ProvideBindingPath will tell VS to look in the provided folder whenever it is doing an assembly resolution which can slow down assembly load for everyone if there are lots of things to search. ProvideCodeBase is much more focused so it has less of a perf impact leak.

@jcansdale
Copy link
Collaborator Author
jcansdale commented Sep 21, 2017 •
edited
Loading

Did something change in that regard, of were new dependencies added?

This is an issue that has kept coming back. Adding codebase entries for the GitHub.UI and GitHub.Exports helped, but it still happened in certain circumstances.

Part of the problem is that XAML doesn't work well with fully qualified assembly names. We've ended up replying on the assembly being loaded before the XAML references are resolved. There are some situations where this doesn't happen.

I started noticing the missing icon issue again when VS 15.3 was released. I'm guessing this is something to do with a change in the timing of package/assembly loads. It happens when the user hasn't logged in and used the extension. Unfortunately this is when we most want them to notice the icon!

ProvideCodeBase also seems to work when using unsigned versions of this GitHub assemblies (which don't work with ProvideCodeBase). This happens when external users build the extension as we don't make the keys publicly available.

@Newrad0603
Copy link

That's interesting about the unsigned dll issue. I would assume anyone building private unsigned bits for this would be testing using the Exp instance of VS which I believe supports loading unsigned bits, so I find it odd that the ProvideCodeBase wouldn't work in that case. That being said, I definitely see it not working for normal VS since we generally don't want to load unsigned bits in the normal product due to security concerns.

@jcansdale
Copy link
Collaborator Author
jcansdale commented Sep 21, 2017 •
edited
Loading

I believe dependentAssembly elements are kind of like the GAC in that they're only considered when a signed assembly is being resolved. Unsigned assemblies can be loaded fine using Assembly.LoadFrom, it's just resolving them that can be an issue.

The other part of the problem is that XAML doesn't always use use fully qualified assembly names. This also seems to be required for resolving to happen.

I've just stumbled across the following:
https://docs.microsoft.com/en-us/dotnet/framework/configure-apps/file-schema/runtime/qualifyassembly-element

Do you know if there are any attributes that could be used to inject this element into the .config file?

@jcansdale
Copy link
Collaborator Author

@Newrad0603 I've added some notes about our proposed use of ProvideBindingPath here #976. Could you let me know if I've missed anything?

I've explored other options pretty thoroughly, but I'm left thinking this is our least worst option. 😄

@Newrad0603
Copy link

The change looks fine, just make sure to test it through several workflows to make sure it doesn't have a marked impact on VS perf :)
5B64

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
needs investigation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Enable assembly resolver to ensure that Start Page and other icons can be found github/VisualStudio
2 participants
@Newrad0603 @jcansdale
0