Fixed an issue where the AspNetCoreSharedFrameworkResolver was never called in .net core 3.1

[jhartmann123]

Continuing from #654 (comment)

See also CecilAssemblyResolver:185:
string path = library.ResolveReferencePaths(_compositeResolver.Value).FirstOrDefault();

AspNetCoreSharedFrameworkResolver.TryResolveAssemblyPaths() was never called when it was the last entry in the compositeResolver. Just moving it up to be the first resolver fixes the issue.

[jhartmann123]

A bit of background info: Checking the msbuild.binlog (as instructed in the linked ticket) I saw the following issues:

Stack:

[coverlet] TryWithCustomResolverOnDotNetCore exception: System.InvalidOperationException: Cannot find reference assembly 'Microsoft.AspNetCore.Antiforgery.dll' file for package Microsoft.AspNetCore.Antiforgery.Reference
   at Microsoft.Extensions.DependencyModel.Resolution.ReferenceAssemblyPathResolver.TryResolveAssemblyPaths(CompilationLibrary library, List`1 assemblies)
   at Microsoft.Extensions.DependencyModel.Resolution.CompositeCompilationAssemblyResolver.TryResolveAssemblyPaths(CompilationLibrary library, List`1 assemblies)
   at Microsoft.Extensions.DependencyModel.CompilationLibrary.ResolveReferencePaths(ICompilationAssemblyResolver[] customResolvers)
   at Coverlet.Core.Instrumentation.NetstandardAwareAssemblyResolver.TryWithCustomResolverOnDotNetCore(AssemblyNameReference name) in D:\a\1\s\src\coverlet.core\Instrumentation\CecilAssemblyResolver.cs:line 155

The messages appeared basically for all .NET Framework libs, just ending in the "Unable to instrument module:"-message. After a bit of debugging I found out, that the TryResolveAssemblyPaths was never called for the AspNetCoreSharedFrameworkResolver. I just moved it up in the composite resolver and voila, it works.

Maybe there's a bug in the .net core 2.2 resolvers, that it resolves the wrong version and then does not continue? Just a wild guess - there were quite a few changes in that area with .NET Core3

[MarcoRossignoli]

It's weird this tests should fail https://github.com/tonerdo/coverlet/blob/master/test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs#L457 I did also manual check, but I was on 2.2, I'll do some more check to understand if make sense move on top...my idea was search before on "defaults" location(took from default DependencyModel code) and after try with my one.

[MarcoRossignoli]

@jhartmann123 are you building 2.2 app with 3.1 runtime?

[jhartmann123]

Nope, all projects target netcoreapp3.1, except one class lib which is netstandard2.1.

I also tried the same constellation with my repro (where coverlet had no issues with). Also used some libs which reference net core 2.2 (like hangfire, swagger/swashbuckle) in my repro to no avail - everything worked there.

[MarcoRossignoli]

I see Microsoft.AspNetCore.Antiforgery.dll but you said that the app is a console app...how are you using that lib in a console?I mean looks suspicious, I'd like to have a repro for instance a console app with some reference to other that bring asp.net libs referenced to xunit test...seem a scenario like that.

[jhartmann123]

No, its an ASP.net core API project (<Project Sdk="Microsoft.NET.Sdk.Web">) - the console app is the one that works :)

[MarcoRossignoli]

Oh I did a mess so, we need a repro, I just debugged tests and works as expected I don't understand why yours not...maybe I should add asm version to log, but the behaviour you described is conform to exception.

[jhartmann123]

Ok, so I reverted the last change (as it really didn't make lots of sense) and continued on my journey to create a repro. No chance. I only get the issue on that specific project and I still don't know why.

So I jumped back into debugging - and got a result as you see. I still don't know what exactly the issue was, because its late and I'm tired, but I'll leave it here for review.

The changes explained:

Removed the libraries-Dictionary: it seems to me that too many resolves were done. For each call to that method

• the deps file was read
• each library in there was resolved
• only the input library was taken, if available
• everything else got thrown away
  So I've taken the liberty to optimize that away.

The story behind the resolving part: I checked, what library.ResolveReferencePaths does and I checked, what the CompositeCompilationAssemblyResolver does. It's simple: they iterate over the defined resolvers and the result of the first one who finds something gets returned.

So for easier debugging I just did the same manually (iterate over the resolvers, first result wins), also removed the check for the lib type "project" and to my surprise everything started working.

Any combination of filtering for "project" or using the composite resolver again caused the error to reappear. Tbh, I don't know why - but as it is not the first time I had to fight with .net core 2 resolving, I'll take any solution :)

[edit] fixed lib type in last two paragraphs: package -> project

[MarcoRossignoli]

Thanks for your deep analysis, I'll continue from here!

[jhartmann123]

Found the issue: I restarted debugging from your current master state. The breakpoint in the screenshot is just before resolving the "unresolvable" lib. Note from the Watch-Window: library.Assemblies is an empty array:

Also note: Both ResolveReferecePaths and the composite resolver return the result of the first resolver that returns true. The frist resolver is the AppBaseCompilationAssemblyResolver. Check this line: https://github.com/dotnet/core-setup/blob/074c3bece80ca300e2f9d053b18748b6537d6f67/src/managed/Microsoft.Extensions.DependencyModel/Resolution/AppBaseCompilationAssemblyResolver.cs#L91

If library.Assemblies is empty, this just returns true. So the thing that fixes it in my branch is the additional check to assemblies.Count > 0.

[jhartmann123]

Just compared the deps files between .net core 3.0 and 3.1. Turns out, it slightly changed.

Example entry for 3.0:

"Microsoft.Extensions.Caching.Memory/3.0.1": {
        "dependencies": {
          "Microsoft.Extensions.Caching.Abstractions": "3.0.1",
          "Microsoft.Extensions.DependencyInjection.Abstractions": "3.0.1",
          "Microsoft.Extensions.Logging.Abstractions": "3.0.1",
          "Microsoft.Extensions.Options": "3.0.1"
        },
        "runtime": {
          "lib/netcoreapp3.0/Microsoft.Extensions.Caching.Memory.dll": {
            "assemblyVersion": "3.0.1.0",
            "fileVersion": "3.0.119.53103"
          }
        },
        "compile": {
          "lib/netcoreapp3.0/Microsoft.Extensions.Caching.Memory.dll": {}
        }
      },

Same entry for 3.1:

"Microsoft.Extensions.Caching.Memory/3.1.0": {
        "dependencies": {
          "Microsoft.Extensions.Caching.Abstractions": "3.1.0",
          "Microsoft.Extensions.DependencyInjection.Abstractions": "3.1.0",
          "Microsoft.Extensions.Logging.Abstractions": "3.1.0",
          "Microsoft.Extensions.Options": "3.1.0"
        }
      },

So I guess the .NET Core 2.2 DependencyContextJsonReader just doesn't read .NET Core 3.1 dependency files correctly anymore.

[jhartmann123]

I've just commited a workaround, based on the master branch. Its a bit less intrusive than the solution from yesterday.

[MarcoRossignoli]

But in your test project have you added <PreserveCompilationContext>true</PreserveCompilationContext>?

https://github.com/dotnet/cli/blob/master/Documentation/specs/runtime-configuration-file.md#opt-in-compilation-data

Seem that 3.1 version is without compilation context, only "runtime dependecies"

[jhartmann123]

Yes, I've added this property in both, Test and API-Project.

[edit] ... while it is unneccessary for the API-Project:

Note that ASP.NET projects (those using Microsoft.NET.Sdk.Web SDK) has this property set to true by default.

[jhartmann123]

I do actually have some entries with runtime/compile, but only if they reference an older framework, like netstandard2.0 or older. Example:

      "Microsoft.Extensions.DependencyInjection/3.1.0": {
        "dependencies": {
          "Microsoft.Extensions.DependencyInjection.Abstractions": "3.1.0"
        }
      },
      "Microsoft.Extensions.DependencyInjection.Abstractions/3.1.0": {},
      "Microsoft.Extensions.DependencyModel/2.1.0": {
        "dependencies": {
          "Microsoft.DotNet.PlatformAbstractions": "2.1.0",
          "Newtonsoft.Json": "12.0.2",
          "System.Diagnostics.Debug": "4.3.0",
          "System.Dynamic.Runtime": "4.3.0",
          "System.Linq": "4.3.0"
        },
        "runtime": {
          "lib/netstandard1.6/Microsoft.Extensions.DependencyModel.dll": {
            "assemblyVersion": "2.1.0.0",
            "fileVersion": "2.1.0.0"
          }
        },
        "compile": {
          "lib/netstandard1.6/Microsoft.Extensions.DependencyModel.dll": {}
        }
      },

[MarcoRossignoli]

Strange...I have to understand if docs is old and behaviour is changed.
For now if you can use a package build in your branch, I have to do some check and after we could merge PR if it's the right workaround.

[MarcoRossignoli]

@jhartmann123 I did some test I created an api+test project adding some dummy test but if I compile 3.0-3.1 I get same deps file for unit test project...the only difference are versions...can you try to do the same with your project?

My sample AspNetTest.zip and the two deps files

30_XUnitTestProject1.deps.json.txt
31_XUnitTestProject1.deps.json.txt

I used https://www.diffchecker.com/diff

[jhartmann123]

Same result here. But with your sample I got a new clue.

Here my repro:
coverlet-repro.zip (update below)

So that solution basically just contains default projects generated with VS16.4 - an asp.net and a xunit test project. I've added some references, fiddled around a bit with it yesterday.

Then I added a console project just to check if the netcore3.1 dependency context maybe returns an entry in the assembly array for Microsoft.Extensions.DependencyInjection.Abstractions - it didn't.

Now I just tried the same with the deps file generated by your test project, and voila - there is an assembly. Just have a look at the ConsoleApp1 in the attached solution. Your deps file works, the one from the project doesn't. WTF?

[edit] here the output for completeness:

[edit2] The references in the API-Project (Serilog.AspNetCore...) still reference the old netcore2-APIs. That's why I chose them... Maybe that has something to do with it. But in the real project we have plenty of such libs that haven't been updated to core3 yet.

[jhartmann123]

Ah, they actually return different versions. With your deps file I get 3.1.0 (as expected), with my deps file I get 2.1.0 - and its the only one. There isn't a second one with 3.1.0 getting returned in CompileLibraries :/

[jhartmann123]

Updated repro a bit with more fiddling around:
coverlet-repro.zip

[MarcoRossignoli]

If you remove <PackageReference Include="Serilog.AspNetCore" Version="3.2.0" /> also your deps report correct version...seem that when compile it found a lower version and add that to deps but at runtime load new one

The package has got reference to old version, ones added to deps file

[MarcoRossignoli]

@jhartmann123 returning to #655 (comment) that could be the core of the issue can you share a piece of deps of missing libs(if it doesn't expose sensible data)?I would like to understand when asm list is loaded and when not. My concern is that the deps is redacted in very "complex way"(the possible refs/deps/runtime version matrix) and so I would like to be sure that https://github.com/dotnet/core-setup/blob/074c3bece80ca300e2f9d053b18748b6537d6f67/src/managed/Microsoft.Extensions.DependencyModel/Resolution/AppBaseCompilationAssemblyResolver.cs#L91 is correct and expected for some context, my idea is to open a ticket on core-setup repro to ask, maybe they help us to understand if your workaround is acceptable, or something better can be done(for instance search also in lower runtime shared forlders, in tests above we saw that deps doesn't reflect "real loaded libs" so some rules seem hard coded inside runtime load code).

[jhartmann123]

Omg. Found it. (The fourth time :) )

https://github.com/dotnet/sdk/blob/2aeb1b77027b4eced14d86dd54082b04eb9aa492/src/Tasks/Microsoft.NET.Build.Tasks/DependencyContextBuilder.cs#L761

If a library name isn't unique, ".Reference", ".Reference1", ".Reference2" and so on gets appended.

Example:

My "missing" assembly can be resolved with a .Reference suffix...

[MarcoRossignoli]

You mean different version are included but with .ReferenceX appended?

[jhartmann123]

Yes. And different versions of the dependency injection lib are once referenced by the web project (core 3.1) and by serilog (core 2.2) there are two entries. Could be even more.

Here a current repro
repro.zip

[jhartmann123]

Sorry for vanishing for so long on this topic. I actually planned to update this to properly support the .ReferenceX suffixes when using different library versions.

Now, I've just updated the packages in our project, including the coverlet packages, and suprisingly I got the coverage reports back. Is there still anything to do on this topic or was it acutally resolved?

[MarcoRossignoli]

I didn't work on it anymore because I was waiting info from ms team on other issue(that return true on resolver, but no answer at the moment dotnet/runtime#1047).
BTW if it's solved for now feel free to close this and we'll re-open if needed, for now seem only your isolated case.

[jhartmann123]

Ok, then I'll close it for now. Thanks for all the support :)

[MarcoRossignoli]

Thank you for using Coverlet!