Use Roslyn to create ref/ assemblies

[dougbu]

• Investigate using C# compiler to generate reference assemblies in 5.0 #14801

Suggest reviewing the commits individually and mostly ignoring the first ("Remove all ref/ projects").

This set of changes

• reduces the complexity of our build, mostly because it removes all ref/ projects
• avoids any need for manual changes when APIs change
• increases the targeting pack size slightly e.g. Microsoft.AspNetCore.App.Ref.*.nupkg was about 1.5 MB before this change and 2 MB after
  • most of this comes from including more information in the assemblies e.g. internal symbols when IVT is used
• makes it difficult to display public API changes -- until we decide on the baselines to compare against and implement that comparison

[Tratcher]

@dougbu you are my new favorite person 😁.

Can /docs/ReferenceAssemblies.md be removed?

Fixes https://github.com/dotnet/aspnetcore-internal/issues/2693

[dougbu]

Can /docs/ReferenceAssemblies.md be removed?

Yes 😺

[wtgodbe]

I must have missed it - where/how do we actually hook in to the roslyn ref project generation?

[dougbu]

where/how do we actually hook in to the roslyn ref project generation?

It's a single property setting here.

[wtgodbe]

Everything looks good except for the question about building against old refs & running against new impl's - will approve once that's resolved

[ericstj]

increases the targeting pack size slightly

One idea we had in dotnet/runtime was to use the ILLinker to trim the resulting reference assemblies that are shipped in the ref pack. We already do this for our runtime assemblies, doing so for ref might be a good use of linker. The settings we use with it to root public API and trim unreachable code are here: https://github.com/dotnet/runtime/blob/69b0d160953f5c920e52f021366743623693c158/eng/illink.targets#L192

[dougbu]

One idea we had in dotnet/runtime was to use the ILLinker to trim the resulting reference assemblies that are shipped in the ref pack.

That sounds like fun 😺 but I'm not that sure the small size bump merits the extra work. My latest build produced a 2.02 MB Microsoft.AspNetCore.App.Ref.5.0.0-ci.nupkg while the one from the most recent rolling build of master was 1.74 MB. Thoughts @Pilchie❔

[Pilchie]

I suspect it might be a fair bit of work, given that our reference assemblies need to have some internals so that test assemblies can be built that have InternalsVisibleTo. It seems like a significant thing to manage for a negligible benefit.

[dougbu]

🆗 I declare this PR working.

The https://dev.azure.com/dnceng/public/_build/results?buildId=731663 build might still time out (unrelated to my changes) but otherwise did all the Right Thing:tm:s Have confirmed the downlevel assemblies landed in Microsoft.AspNetCore.App.Ref.5.0.0-ci.nupkg and shipped assemblies all built against what they should have e.g. Microsoft.AspNetCore.Http.Features.dll references the expected (older) ref/ assembly for System.IO.Pipelines.

System.Collections, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
System.ComponentModel, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
System.IO.Pipelines, Version=5.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
System.Linq, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
System.Net.Primitives, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a

I'll clean up the commits and remove the "test only" parts in preparation for a final review. In the meantime, I would appreciate comments on below !!! bit since I'd rather not check that question in 😈

[dougbu]

@Pilchie

I suspect it might be a fair bit of work

Maybe not quite as much as you suspect. My objection was based on the size of the target @ericstj pointed us to and the potential build perf degradation due to a lot of <Exec> invocations. If we did anything, it would be in the Microsoft.AspNetCore.App.Ref project and should not affect anything else building in this repo.

[ericstj]

Fair enough. FWIW we saw a lot larger size differences when we were looking at roslyn's references. For example, I spot checked System.Text.Json right now and our ref is 25KB vs Roslyn's which is 75KB. The nice thing about the linker is that it can solve the internals problem for you. IIRC we were trying idefs and this was hard because the compiler needs legit syntax, whereas with the linker you can root just the internals you need and it will walk the type graph.

[jaredpar]

FWIW we saw a lot larger size differences when we were looking at roslyn's references. For example, I spot checked System.Text.Json right now and our ref is 25KB vs Roslyn's which is 75KB

This is to some degree expected. The reference assemblies generated by the runtime team are omitting members. This can be done in runtime because they have a good grasp of their specific domain and can safely omit some types / members. Though this has bit them before and ended up causing real world customer issues (the stripping of struct fields that were private as an example).

The general compiler feature though must produce reference assemblies that are correct in any customer domain. That means we'll generally include more data because it's a superset of what runtime would produce.

[dougbu]

🆙📅 to consolidate changes since @wtgodbe and @JunTaoLuo reviewed (equivalent of c65704f) into two commits, remove the test-only bits, and rebase on preview8 branch. Please review or re-review in case there's anything I missed.

@Pilchie we didn't chat about this but I chose to label this a tell-mode change. LMK if that's not right.

[dougbu]

The shift-click menu doesn't always work. The URI for the last 9 commits (all but removing the ref/ projects) is https://github.com/dotnet/aspnetcore/pull/23403/files/32dd707601f5b832060e8bbe49be5197c844545f..7ebf486c9a29c462ad8956d117b224c231111d53 at the moment.

[wtgodbe]

Can't be parameterized?

[dougbu]

Not unless I remove the Directory.Build.* files. I'll test out whether that causes any problems after getting this in and doing the merge to 'master'. It's no worse at the moment than the hard-coded TFMs in (say) our project templates. But, I'll clean it up next if possible.

[Pilchie]

I think tell mode is fine - I'll explicitly mention it in tactics, since it is a pretty significant change.

[Pilchie]

Yipee!!