[release/3.1] Port 6.0 fix to AssemblyDependencyResolver to not throw for same file

[LakshanF]

Runtime follow up fixes for .NET 3.1 - dotnet/runtime#42055

Description

There is a problem with resolving multiple files with the same simple name (e.g. Module, module) in case insensitive OS like Windows for AssemblyDependencyResolver. Our internal storage for such cases throws that can cause problems for users who do not expect the error.

Customer Impact

Reported by customer who was migrating to .Net Core from the Desktop runtime.

After migrating the mixed-mode C++/CLI application from targeting .NET Framework over to .NET Core 3.1, several obstacles have been encountered. One of them is an exception related to adding a duplicate key to a Dictionary in the AssemblyDependencyResolver. Alternatively, they might get an exception about the assembly having already been found with a different extension. Both of these problem point back to .NET Core having a mixture of case sensitivity and insensitivity.

Regression

No, it has always existed.

Testing

Added test scenarios.

A customer who ran into this in 5.0 tried the 6.0 build and reported success.

Risk

Low.

Although getting casing correct with different OSs are notoriously hard, the resolution was to make AssemblyDependencyResolver consistent with other parts of the runtime by choosing the first simple file name and not throwing afterwards for same simple filename with different casing

The 6.0 issue is dotnet/runtime#37162

[vitek-karas]

We should try to port at least one test to validate this.

[jeffschwMSFT]

Once you are ready for review, please add the servicing template (here is an example dotnet/runtime#51421)

[agocke]

@vitek-karas @elinor-fung anything else?

[agocke]

Mac fix moved to #28163

[elinor-fung]

Can we get a run of the added AssemblyDependencyResolver tests on all platforms? They have CLRTestPriority=1, so I don't believe they were run with the PR.

[elinor-fung]

/azp help

[azure-pipelines]

Supported commands

• help:
• Get descriptions, examples and documentation about supported commands
• Example: help "command_name"
• list:
• List all pipelines for this repository using a comment.
• Example: "list"
• run:
• Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
• Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
• where:
• Report back the Azure DevOps orgs that are related to this repository and org
• Example: "where"

See additional documentation.

[elinor-fung]

/azp list

[azure-pipelines]

CI/CD Pipelines for this repository:

• coreclr-outerloop
• coreclr-ci
• dotnet-coreclr-nullablefeature
• coreclr-r2r
• coreclr-crossgen2

[agocke]

/azp run coreclr-outerloop

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

[agocke]

@elinor-fung Any luck on finding the right thing to run? I agree it looks like only P0 tests are being run, but I can't find a switch to run the rest,

[elinor-fung]

Looks like the triggers for it don't include release/3.*. I manually queued a run: https://dev.azure.com/dnceng/public/_build/results?buildId=1108340
I think it should end up reflected in the checks here.

[elinor-fung]

Test is failing on Linux: https://helix.dot.net/api/2019-06-17/jobs/552434ac-d9d3-439b-a1d2-2ffe86271a87/workitems/PayloadGroup0/console

TestAssemblyWithCaseReversed Start
TestAssemblyWithCaseReversed FAILED:
System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> Xunit.Sdk.EqualException: Assert.Equal() Failure
Expected: /home/helixbot/work/A9800949/w/9E9A0883/e/Loader/AssemblyDependencyResolver/AssemblyDependencyResolverTests/AssemblyDependencyResolverTests/TestComponent_15a025f6/TestAssemblyWithCaseReversed.dll
Actual:   (null)
   at Xunit.Assert.Equal[T](T expected, T actual, IEqualityComparer`1 comparer) in C:\projects\xunit\src\xunit.assert\Asserts\EqualityAsserts.cs:line 40
   at AssemblyDependencyResolverTests.AssemblyDependencyResolverTests.TestAssemblyWithCaseReversed() in /__w/1/s/tests/src/Loader/AssemblyDependencyResolver/AssemblyDependencyResolverTests/AssemblyDependencyResolverTests.cs:line 195
   --- End of inner exception stack trace ---
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at AssemblyDependencyResolverTests.TestBase.<>c__DisplayClass18_1.<RunTestsForInstance>b__1() in /__w/1/s/tests/src/Loader/AssemblyDependencyResolver/AssemblyDependencyResolverTests/TestBase.cs:line 87
   at AssemblyDependencyResolverTests.TestBase.RunSingleTest(Action test, String testName) in /__w/1/s/tests/src/Loader/AssemblyDependencyResolver/AssemblyDependencyResolverTests/TestBase.cs:line 70

I expect it is because AssemblyDependencyResolver.ResolveAssemblyToPath does a File.Exists check on the path, respecting case-sensitivity/insensitivity of the system, so when the first path doesn't match the actual file casing, the assert doesn't hold in case-sensitive scenarios.

[LakshanF]

Re-running manually, https://dev.azure.com/dnceng/public/_build/results?buildId=1109754

[LakshanF]

The test seems to be passing now

[agocke]

@jeffschwMSFT For servicing check

@wtgodbe for guidance on merging. Anything else we should do here?

[jeffschwMSFT]

Approved. I will take for consideration for 3.1.x.

[jeffschwMSFT]

@agocke @LakshanF is there a similar change needed for 5.0.x?

[wtgodbe]

If tactics approves this we can merge this once branches are open for June, which will be 5/4 - 5/10 (the Tuesday before Patch Tuesday thru the night before Patch Tuesday). Are the CI failures expected?

[LakshanF]

@wtgodbe, the OSX failures that are due to #28163. The test failures in other platforms are not related to this change