Fix the SDK repo to stop using its own version of Microsoft.Extensions.Logging.Abstractions in the dotnet app

[trylek]

Our current work on publishing .NET containers using combined runtime-ASP.NET readytorun composite are complicated by the fact the the SDK repo publishes the dotnet app with its own version of the Microsoft.Extensions.* assemblies. Longer term we should fix SDK to cleanly consume the assembly from its aspnetcore dependency but to expedite publishing the composite container we're about to temporarily remove the assembly from the ASP.NET composite. The purpose of this tracking issue is to revert this short-term workaround and fix the primary problem i.e. SDK using its own version of the Microsoft.Extensions.Logging.Abstractions assembly.

Cf. dotnet/dotnet-docker#4538

/cc @dotnet/crossgen-contrib @ivdiazsa @wtgodbe @marcpopMSFT

[eerhardt]

Longer term we should fix SDK to cleanly consume the assembly from its aspnetcore dependency

This doesn't seem right to me. The SDK doesn't reference aspnetcore (at least the dotnet.csproj doesn't). If you look at its runtimeconfig.json file:

{
  "runtimeOptions": {
    "tfm": "netcoreapp3.1",
    "rollForward": "Major",
    "framework": {
      "name": "Microsoft.NETCore.App",
      "version": "8.0.0-preview.4.23217.4"
    },
    "configProperties": {
      "System.Reflection.Metadata.MetadataUpdater.IsSupported": false
    }
  }
}

It doesn't require the ASP.NET shared framework.

It would be interesting to see what dependency is bringing in all the Microsoft.Extensions.* assemblies into the SDK folder:

Maybe a more interesting approach would be to put these Microsoft.Extensions.* assemblies into the Microsoft.NETCore.App shared framework, instead of the SDK taking an explicit dependency on the ASP.NET shared framework.

FYI - @ericstj.

[trylek]

Thanks Eric for your feedback, I'll be certainly more than happy to adjust the preliminary design to whatever we decide is the cleanest. I do see that some SDK source code refers to the assembly M.E.Logging.Abstractions, e.g. here:

https://github.com/dotnet/sdk/blob/4ea3e8304c21022bbea72a29154b7602af4f9239/src/Cli/Microsoft.TemplateEngine.Cli/CliConsoleFormatter.cs#L6

There are also some tests in the SDK repo that reference the assembly but I guess those shouldn't affect the SDK as a shipping product. I agree that SDK dependency on ASP.NET sounds somewhat artificial; I guess that, as a first step, we need to understand whether the dependency of SDK on the assembly is intentional and what is its actual purpose - my initial assumption was that this is most likely somehow related to VS integration but I might be wrong. Based on the outcome we can initiate a broader discussion on whether it would make sense to make it a part of the runtime framework.

[ericstj]

#67487 tracks the most recent discussion about moving Microsoft.Extensions from dotnet/runtime into Microsoft.NETCore.App. It might be good to add to that issue.

[marcpopMSFT]

CC @YuliiaKovalova and @GangWang01 as the only usage I see of Logging.Abstractions in the sdk outside of tests is that one in the templating engine that is linked above for the CliConsoleFormatter.

[trylek]

We just discussed this at the Crossgen2 weekly sync-up meeting. I think @davidwrighton nicely summed up the gist of problem by stating it's basically a dependency flow issue. If the Microsoft.Whatnot assemblies are produced in the runtime repo and consumed in the SDK repo and there's another flow where the runtime repo (including the Microsoft.Whatnot assemblies) gets consumed by ASP.NET which gets subsequently also consumed by the SDK repo, we just need to make sure that the SDK uses the same version of the runtime as the one that was originally used by the ASP.NET repo for building its layer.

The ASP.NET composite container project makes it somewhat more challenging by coupling the runtime and ASP.NET bits in a tighter way but ultimately I think David is right that the flow is basically the same even in this case, if we were able to express somehow in the SDK repo that the runtime version we need must be the same as the version that was used by the aspnetcore repo to build its stuff, we'd be golden. If this is not doable today, I guess we're about to explore the territory of short-term hacks and longer-term proposals for the dependency flow model.

[jkotas]

longer-term proposals for the dependency flow model

This problem is going to be solved longer-term (.NET 9) by switching to Unified Build.

[eerhardt]

From what I can tell, this isn't a problem with a slightly different version of 8.0 assemblies. The 8.0 SDK is consuming and shipping the 6.0 assemblies of Microsoft.Extensions.*. See these two lines:

https://github.com/dotnet/sdk/blob/7020b6d1001eb56ef4de30fb743a8a33571d0080/src/Cli/Microsoft.TemplateEngine.Cli/Microsoft.TemplateEngine.Cli.csproj#L19

https://github.com/dotnet/sdk/blob/7020b6d1001eb56ef4de30fb743a8a33571d0080/eng/Versions.props#L59

Even with the already shipped 7.0.105 SDK, if I look at the file properties of the Microsoft.Extensions.DependencyInjection.dll in the SDK folder, I see:

[trylek]

Thanks Eric for your additional feedback. I guess the interesting part of the story is whether the aspnetcore repo consumes the same version of these assemblies. I may be wrong but my gut feeling is that for instance the Microsoft.Extensions.Logging.Abstractions assembly is somehow related to VS integration, in such case I can easily imagine that its API surface is quite stable to the point that .NET 6 vs. .NET 8 differences might not matter much.

[YuliiaKovalova]

CC @YuliiaKovalova and @GangWang01 as the only usage I see of Logging.Abstractions in the sdk outside of tests is that one in the templating engine that is linked above for the CliConsoleFormatter.

@vlada-shubina could you join the discussion?

[vlada-shubina]

Template Engine libraries that are used in dotnet/sdk are using the Microsoft.Extensions.Logging.
At the moment, in main we use the following versions:
https://github.com/dotnet/templating/blob/6536e8d9b4ae1e70047cf4c254c352db75b99903/eng/dependabot/Packages.props#L12-L14

We cannot update Microsoft.Extensions.Logging.Console due to the bug #79812