Repository navigation
bindingRedirects can become out of sync with installed extension #1217
Description
- 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.
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?
Activity
jcansdale commentedon Sep 5, 2017
Here is an example of the
Start Pagein Visual Studio 201715.3.3. This is immediately after Visual Studio Enterprise 201715.3.3was installed.Clicking on the
GitHublink brings up theTeam Explorer, but without any GitHub extension functionality.Here is the
devenv.exe.configfile fromC:\Users\jamie\AppData\Local\Microsoft\VisualStudio\15.0_886c2a72.devenv.exe.config.txt.txt
It is missing all GitHub brindingRedirects.
jcansdale commentedon Sep 19, 2017
It turns out the most common reason for the missing icon was our use of an unqualified assembly name (
/GitHub.VisualStudio;component) in theGitHub.VisualStudio.imagemanifestfile:VisualStudio/src/GitHub.VisualStudio/GitHub.VisualStudio.imagemanifest
Line 6 in 7d6e8de
Prior to VS 15.3 this wasn't an issue because the
GitHub.VisualStudiowas always loaded by the time the icons were needed. It appears there was a change toTeam Explorerassembly 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 commentedon Sep 20, 2017
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 commentedon Sep 21, 2017
@Newrad0603,
The
ProvideCodeBaseattributes are defined here:https://github.com/github/VisualStudio/blob/master/src/GitHub.VisualStudio/Properties/AssemblyInfo.cs
We don't actually have a
ProvideCodeBasefor theGitHub.VisualStudioassembly. The reason is that once an assembly has aProvideCodeBase, this attribute must also be used on all of the assemblies it references. This leads to an explosion ofProvideCodeBaseattributes, 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?Newrad0603 commentedon Sep 21, 2017
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 commentedon Sep 21, 2017
This is an issue that has kept coming back. Adding codebase entries for the
GitHub.UIandGitHub.Exportshelped, 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 commentedon Sep 21, 2017
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 commentedon Sep 21, 2017
I believe
dependentAssemblyelements 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 usingAssembly.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 commentedon Sep 22, 2017
@Newrad0603 I've added some notes about our proposed use of
ProvideBindingPathhere #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 commentedon Sep 26, 2017
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 :)