GetDocumentInsider: use System.Diagnostics.DiagnosticSource from runtime (not OOB)

[mgravell]

(via build-ops)

(probably ignore the actual code here for now; this is becoming more of a place-holder for investigation and discussion)

GetDocumentInsider: use System.Diagnostics.DiagnosticSource from runtime

GetDocumentInsider has historically been using a very old version of the archived System.Diagnostics.DiagnosticSource from NuGet (OOB); however, System.Diagnostics.DiagnosticSource has recently been transitively updated in runtime via a resurected Microsoft.Extensions.Diagnostics.Abstractions by @Tratcher ; this is causing CI failures due to competition between two routes to the assembly, as illustrated by the CI here:

• GetDocument.Insider -> Microsoft.Extensions.Hosting.Abstractions 8.0.0-rc.1.23410.15 -> Microsoft.Extensions.Diagnostics.Abstractions 8.0.0-rc.1.23410.15 -> System.Diagnostics.DiagnosticSource (>= 8.0.0-rc.1.23410.15)
• GetDocument.Insider -> System.Diagnostics.DiagnosticSource (>= 4.6.0)

To resolve this, I propose changing GetDocumentInsider to use the runtime version of System.Diagnostics.DiagnosticSource (since I can't reasonably remove Microsoft.Extensions.Hosting.Abstractions).

Notes:

• I do not know how to test this!
• the transitive use of Unsafe was unsupported on netcoreapp2.1; I propose dropping this TFM, but I do not know if this has any significance (edit: yes, it very much does have significance; we can't do this 😢 )

C:\Users\marcg.nuget\packages\system.runtime.compilerservices.unsafe\6.0.0\buildTransitive\netcoreapp2.0\System.Runtime.CompilerServices.Unsafe.targets(4,5): error : System.Runtime.CompilerServices.Unsafe doesn't support netcoreapp2.1. Consi
der updating your TargetFramework to netcoreapp3.1 or later. [C:\Code\aspnet\marc-diagnostics\src\Tools\GetDocumentInsider\src\GetDocument.Insider.csproj::TargetFramework=netcoreapp2.1]

I could have changed to netcoreapp3.1, but that too is end-of-life, so... 🤷

I've done a quick scan of the repo and GetDocumentInsider appears to be the only likely conflict, which is also supported by the CI failure above only reporting this one package.

[mgravell]

Proposed solution isn't going to be enough by itself; has impact via:

• https://github.com/dotnet/aspnetcore/blob/bfeb3bf9005c36b081d1e48725531ee0e15a9dfb/src/Tools/dotnet-getdocument/src/dotnet-getdocument.csproj
• ⁠https://github.com/dotnet/aspnetcore/blob/bfeb3bf9005c36b081d1e48725531ee0e15a9dfb/src/Tools/Extensions.ApiDescription.Server/src/Microsoft.Extensions.ApiDescription.Server.csproj
• ⁠https://github.com/dotnet/aspnetcore/blob/bfeb3bf9005c36b081d1e48725531ee0e15a9dfb/src/Tools/Extensions.ApiDescription.Server/src/Microsoft.Extensions.ApiDescription.Server.nuspec

Trying other options... next plan is to change System.Diagnostics.DiagnosticSource in runtime to not ref System.Memory in down-level build; however, this has a non-trivial number of span uses internally; even if we fixed that, we'd still have a problem in that System.Diagnostics.DiagnosticSource "current" targets netstandard 2.0, so it isn't going to build in a netcoreapp2.1 environment; the last version to target 2.1 was 5.0, so we could have a lot of fixes and we'd still have the System.Memory problem

Open to suggestions here... my thinking:

1. stop targeting netcoreapp2.1 for the command-line-tools, but that's a huge thing to change last minute (maybe this should be focused for net9?)
2. revert the diagnostic source update via Microsoft.Extensions.Hosting.Abstractions
3. create a new project that is a clone of Microsoft.Extensions.Hosting.Abstractions just used from the command-line-tools, that has what we need without the new refs? or maybe add a netcoreapp2.1 TFM to Microsoft.Extensions.Hosting.Abstractions for this purpose?

[wtgodbe]

Stop targeting netcoreapp2.1 for the command-line-tools, but that's a huge thing to change last minute (maybe this should be focused for net9?)

I agree we should do this, but it's probably too late 😢. Can you open an issue in 9 planning to track it?

Revert the diagnostic source update via Microsoft.Extensions.Hosting.Abstractions

I think we should do this unless @Tratcher knows of a quick fix in time for the RC1 build (branch snaps today at 5 PST)

create a new project that is a clone of Microsoft.Extensions.Hosting.Abstractions just used from the command-line-tools, that has what we need without the new refs? or maybe add a netcoreapp2.1 TFM to Microsoft.Extensions.Hosting.Abstractions for this purpose?

I think the latter option makes sense, maybe that's the quick fix we need

[wtgodbe]

Actually I just had another idea, see code comment

[wtgodbe]

Try bringing back the 4.6.0 reference & conditioning it on '$(TargetFramework)' == 'netcoreapp2.1', then having this non-versioned reference conditioned on '$(TargetFramework)' != 'netcoreapp2.1'

[wtgodbe]

Just pushed that to this branch

[wtgodbe]

I folded this change into #50019, closing in favor of that