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

Change GacFileResolver.IsAvailable to check a type from System.Core #46493

Merged
merged 2 commits into from
Aug 11, 2020

Conversation

JoeRobich
Copy link
Member

I've been looking at unit test failure on Mono platforms in an OmniSharp-Roslyn PR (OmniSharp/omnisharp-roslyn#1867) where I update to a more recent build of Roslyn.

It is unable to resolve "System.Xml" with the RuntimeMetadataReferenceResolver. This is because the gacFileResolver is being set to null and the AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES") is also coming back null.

For the object type which is in mscorlib, I am seeing mscorlib loaded from "/Library/Frameworks/Mono.framework/Versions/6.12.0/lib/mono/4.5/mscorlib.dll" and returns false for Assembly.GlobalAssemblyCache.

However, the Enumerable type from System.Core loads from "/Library/Frameworks/Mono.framework/Versions/6.12.0/lib/mono/gac/System.Core/4.0.0.0__b77a5c561934e089/System.Core.dll" and returns true for Assembly.GlobalAssemblyCache.

@JoeRobich JoeRobich requested a review from a team as a code owner August 1, 2020 01:15
@JoeRobich JoeRobich requested a review from tmat August 1, 2020 01:41
@@ -20,7 +20,9 @@ internal sealed class GacFileResolver : IEquatable<GacFileResolver>
/// <summary>
/// Returns true if GAC is available on the current platform.
/// </summary>
public static bool IsAvailable => typeof(object).Assembly.GlobalAssemblyCache;
public static bool IsAvailable
// Check a type from System.Core since Mono doesn't load mscorlib from the GAC.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type is actually in System.Linq.dll in certain distributions. Part of the problem is that it's hard to depend on the assembly location of a given type because the runtime team moves them around.

Think a better approach would be to pick a type like this then assert it's in a different assembly than object. That way we catch ourselves if this ever gets invalidated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaredpar Any idea why mscorlib is not loaded from GAC?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. I guess what I'm really looking for is an Assembly that will always load from the GAC for platforms that support a GAC and a repeatable method for locating the Assembly. mscorlib does not satisfy that requirement for at least some versions of Mono.

Could look through AppDomain.GetAssemblies() to see if any were loaded from the GAC and cache the result.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't seem like it would be reliable in the same way checking a specific one isn't. Maybe we could detect presence of GAC on Mono in a different way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea why mscorlib is not loaded from GAC?

On mono? No idea.

Maybe we could detect presence of GAC on Mono in a different way.

Is there a case on Mono where the GAC isn't used?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, on Mono. The current code works fine on .NET Framework.

Is there a case on Mono where the GAC isn't used?

Apparently there is - @JoeRobich just found out.

Maybe @marek-safar knows?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Mono GAC is optional (there is also option to have multiple GACs). The most reliable way is probably check GAC location calling into https://github.com/mono/mono/blob/master/mcs/class/corlib/System/Environment.cs#L975 and then compare the paths/MVIDs/what-ever-is-appropriate

Copy link
Member

@tmat tmat Aug 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the runtime know to look (or not) into GAC? Does it always look there and if it's not found it looks somewhere else? Where else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is whole assembly loader logic with tons of rules and config options (it's similar to FW assembly binder). There is no documentation covering all the possible combinations and the behaviour though. You'd need to read the code.

Copy link
Member

@tmat tmat Aug 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoeRobich I think the best thing we can do here to return true if we run on Mono: Type.GetType("Mono.Runtime") != null http://sourceroslyn.io/#Microsoft.CodeAnalysis.Scripting/Hosting/Resolvers/MonoGlobalAssemblyCache.cs,160 will get called if this property returns true.

@JoeRobich
Copy link
Member Author

@tmat Updated to check for Mono.

@JoeRobich JoeRobich merged commit f68f090 into master Aug 11, 2020
@ghost ghost added this to the Next milestone Aug 11, 2020
@JoeRobich JoeRobich deleted the dev/jorobich/check-system.core branch August 11, 2020 01:33
@RikkiGibson RikkiGibson modified the milestones: Next, 16.8.P2 Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants