-
Notifications
You must be signed in to change notification settings - Fork 253
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
Resolve dependencies from GAC #950
Conversation
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.
Can you also explain how these changes fix the issue, cause I didn't get it fully.
src/Adapter/PlatformServices.Desktop/Deployment/AssemblyLoadWorker.cs
Outdated
Show resolved
Hide resolved
src/Adapter/PlatformServices.Desktop/Deployment/AssemblyLoadWorker.cs
Outdated
Show resolved
Hide resolved
src/Adapter/PlatformServices.Desktop/Utilities/DesktopAssemblyUtility.cs
Show resolved
Hide resolved
There is DeploymentItem attribute, and this resolver is trying to figure out which assemblies from the bin folder should be copied into Out folder to reduce the amount of copying of unnecessary dependencies. Before .NET Core it was rare that a GAC assembly (GAC = Global Assembly Cache) would be distributed and acquired outside of the project. But in modern times you can get your dependencies from Nuget rather than from GAC when upgrading them. And then there is .NET Standard, which is a kind of meta assembly that does not pack any implementations, but rather just forwards to other dlls based on the current platform, this gives you unified API surface across platforms, with the flexibility of forwarding to the actual implementation as needed. The original algorithm searches dependencies of your test assembly recursively, and as soon as it finds a GAC assembly, it will stop searching in that branch of the tree. netstdard.dll is a GAC assembly, so the algo would terminate the search in that branch. BUT under netstandard there are assemblies that can be acquired as Nugets, e.g. System.ValueTuple.dll, which can be a newer version than the one linked by net standard. Say your project looks like this:
You have your My.Tests.dll test assembly, which depend on netstandard.dll. Netstandard normally depends on System.ValueTuple.dll from GAC, but because you installed it from Nuget, it will actually end up with System.Value.Tuple 4.0.3 in the bin folder, and a bindind redirect from 4.0.0 to 4.0.3. When the dependency resolver algorithm kicks in, it will look at My.Tests.dll, will see that it depends on netstandard, and will see that it is from GAC and will search no furher. This will result in System.ValueTuple not being copied into the Out folder. Instead what needs to happen, is that we will look at the dependencies of the netstandard GAC assembly, and all of it's dependencies to see if any of them is coming from the bin folder and copy all of those over. Which is what this change does, instead of stopping at any GAC assembly, it resolves all dependencies of all assemblies, and returns all of them that are coming from the bin folder. This is because the bin folder is the king, and we are just trying to reduce what we copy, but never add more. |
The change to hashset is just because sets are better structure to use when you are repeatedly looking for "contains" because they are made for that purpose. |
Cherry-picked from #950 > When there are dependencies that the project installs, but they would only resolve via a reference that is located in GAC (e.g. System.ValueTuple via netstandard) then the deployment into Out folder would ignore it. > > If user then installs a version that is newer than what is in GAC it will get into their assembly redirects, but won't get copied into Out folder, resulting into TypeLoad exception for tests with DeploymentItem attribute. > > This change fixes the resolver to look through all the dependencies including GAC dependencies and then copy over only the ones that are found in the bin folder, because ultimately Out should be just a subset (or the same) as the contents in bin folder. > > Looking through GAC assemblies does not seem to add significant overhead the whole resolve is under 300ms so hopefully we don't need an option to configure enabling this. > > (The new log messages won't go into diag log, because it probably is not correctly initialized in the new appdomain that loads the dll, but they will be written to Debug Trace and can be observed by DebugView++ or DebugView.) Co-authored-by: nohwnd <me@jakubjares.com> Co-authored-by: Sanan Yuzbashiyev <Sanan07@users.noreply.github.com>
When there are dependencies that the project installs, but they would only resolve via a reference that is located in GAC (e.g. System.ValueTuple via netstandard) then the deployment into Out folder would ignore it.
If user then installs a version that is newer than what is in GAC it will get into their assembly redirects, but won't get copied into Out folder, resulting into TypeLoad exception for tests with DeploymentItem attribute.
This change fixes the resolver to look through all the dependencies including GAC dependencies and then copy over only the ones that are found in the bin folder, because ultimately Out should be just a subset (or the same) as the contents in bin folder.
Looking through GAC assemblies does not seem to add significant overhead the whole resolve is under 300ms so hopefully we don't need an option to configure enabling this.
(The new log messages won't go into diag log, because it probably is not correctly initialized in the new appdomain that loads the dll, but they will be written to Debug Trace and can be observed by DebugView++ or DebugView.)
Fix #949