Runtime: System.Private.CoreLib designated as native in runtime pack

[nguerrera]

Steps to reproduce

1. dotnet new console
2. dotnet publish -r win-x64
3. Look at binDebugnetcoreapp3.0win-x64app.deps.json

Expected behavior

System.Private.Corelib is designated as a runtime asset

Actual behavior

System.Private.Corelib is designated as a native asset

Is there a reason for this? It is complicating my fix for dotnet/sdk#3109

The app works fine if I edit .deps.json to mark it as managed (runtime) asset, but I don't know if there are other consequences.

I'm not blocked on this as I can work around it, but it would be cleaner if System.Private.CoreLib were designated as managed if possible.

Environment data

λ dotnet --info                                                                                                                           
 .NET Core SDK (reflecting any global.json):
 Version:   3.0.100-preview8-013640
 Commit:    1d6c76dc2b

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18362
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.0.100-preview8-013640\

Host (useful for support):
  Version: 3.0.0-preview8-28380-08
  Commit:  c855ac7187

.NET Core SDKs installed:
  3.0.100-preview8-013437 [C:\Program Files\dotnet\sdk]
  3.0.100-preview8-013640 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0-preview8.19374.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0-preview8.19381.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0-preview8-28373-17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0-preview8-28380-08 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.0-preview8-28373-17 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.0.0-preview8-28380-08 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download

@vitek-karas @dagood

[dagood]

The root cause is that CoreCLR puts it into the transport package that way, and Core-Setup doesn't second-guess it: transport.runtime.win-x64.microsoft.netcore.runtime.coreclr\5.0.0-alpha1.19403.2 for example has runtimes\win-x64\native\System.Private.CoreLib.dll.

There is also a runtimes\win-x64\il\System.Private.CoreLib.dll. I'm drawing a blank on what that one's for. It doesn't get picked up for redist because it doesn't match the typical package content patterns.

The classification that ends up in RuntimeList.xml is based on IsNative, set around here:

https://github.com/dotnet/core-setup/blob/aae3e31d0011b09c8277d2f06f00b07e1fba8cb5/src/pkg/packaging-tools/framework.dependency.targets#L10-L14

[vitek-karas]

S.P.Corelib is special in the way that it gets crossgenes in the coreclr repo (everything else is crossgened in the core-setup repo) - so it comes from coreclr as a RID specific file I assume... that might be the reason it's marked as native (since it kind of is).
I would venture a guess that the weird "il" version is the pure IL corelib (with no R2R).

If the above is true - than I think it would be an OK solution to special case S.P.Corelib - it's already special cased in so many places (the host, the runtime, ...)... one more won't make a difference :smile:

[dagood]

Core-Setup puts the crossgen'd CoreFX in runtimes\win-x64\lib\netcoreapp5.0\, maybe it would make sense to follow the same convention in CoreCLR for this. That seems like a pretty far-reaching change, though. Special casing for RuntimeList.xml would be fairly trivial, on the other hand.

[dagood]

@nguerrera are there any other SDK scenarios that would be impacted by changing this classification?

[dagood]

Moved to 3.1 per https://github.com/dotnet/sdk/issues/3109#issuecomment-525033487.

[dagood]

@nguerrera, now that https://github.com/dotnet/sdk/issues/3109 is complete without this issue, is there still benefit to having a Core-Setup-side classification workaround in 3.1?

I'd like to move this to 5.0, and perhaps drive the fix into the CoreCLR nupkg structure rather than special-casing it in Core-Setup.

/cc @dleeapho

---

For context, it looks to me like this is what would be removed in SDK by implementing a workaround in Core-Setup:

https://github.com/nguerrera/sdk/blob/43764acc7e9a7faf762fa0a1a6129c447e4979f6/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets#L821-L828

  <Target Name="_ComputeManagedRuntimePackAssemblies" Returns="@(_ManagedRuntimePackAssembly)">
    <ItemGroup>
      <!-- Special case for System.Private.Corelib due to https://github.com/dotnet/core-setup/issues/7728 -->
      <_ManagedRuntimePackAssembly Include="@(RuntimePackAsset)"
                                   Condition="'%(RuntimePackAsset.AssetType)' == 'runtime' 
                                                or '%(RuntimePackAsset.Filename)' == 'System.Private.Corelib'" />
    </ItemGroup>
  </Target>

[nguerrera]

Yes, removing that workaround would potentially speed things up in addition to making the code cleaner, but it is not urgent, and I think 5.0 would be fine.

[dagood]

Seems like it would be possible to hack up an SDK to get numbers, but maybe more trouble than it's worth. Getting a scenario that shows perf issues might also be a challenge.

Feels bad to defer a trivial change like this, but seems like the right thing to do vs. the bar.

[nguerrera]

Looking at the code again, I doubt it will impact perf much. I was thinking it would remove a whole pass over the items but it will just remove that 'or' clause.

[jkoritzinsky]

Assigning to myself as I'm doing work with the shared framework creation at the moment.

[NikolaMilosavljevic]

@jkoritzinsky is this still on track for 5.0?

[jkoritzinsky]

I don't think this will make 5.0 unless the MSI unwrapping comes in soon. I'll move this to 6.0.