Exclude System.* reference assemblies in ILCompiler.Build.Tasks

[jkotas]

We expect the implementation of these assemblies to come as part of msbuild.

Fixes #83695

[ghost]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

We expect the implementation of these assemblies to come as part of msbuild.

Fixes #83695

Author:	jkotas
Assignees:	-
Labels:	area-NativeAOT-coreclr
Milestone:	-

[jkotas]

It is not the most robust fix, but I cannot think about anything better that would not be too complicated.

[agocke]

I think this looks good, but I'm not sure why we're getting reference assemblies being copied (per your other comment). This is netstandard2.0, so I would expect that we would get either real assemblies for the non-inbox stuff, or facade assemblies. Why reference assemblies?

[agocke]

Actually, SRM should be in the SDK as well. Can we add PrivateAssets here as well?

[jkotas]

Delete CopyLocalLockFileAssemblies property instead.

[agocke]

OK, I think I'm even more confused after thinking about this more.

I previously said S.R.M is in the SDK. I think that's nominally correct -- the MSBuild.deps.json file lists S.R.M as a dependency.

But presumably if we're running the desktop MSBuild, that will not load the MSBuild from the SDK.

But if we're running desktop MSBuild I would presume that we do actually need things like System.Memory and System.Numerics.Vectors. Those are not in-box in 48, are they?

So this kind of leads me back to the original question: why are there ref assemblies of those DLLs in the output? Shouldn't those be actual DLLs that are loaded only on desktop FX?

Looking at Roslyn, they eventually moved to multi-targeting the task, for a variety of reasons, but one of them was this kind of confusion about what needed to be included in the package. I don't really have an opinion either way on whether we should go in that direction, but I find it notable that System.Memory and System.Numerics.Vectors do appear in the 472 version of their task (and are missing from the netcore version).

[jkotas]

But presumably if we're running the desktop MSBuild, that will not load the MSBuild from the SDK.

Desktop MSBuild in supported VS versions comes with these System.* assemblies.

 Directory of c:\Program Files\Microsoft Visual Studio\2022\Community\MSBuild\Current\Bin

05/17/2023  10:57 AM            20,856 System.Buffers.dll
05/17/2023  10:57 AM           198,784 System.Collections.Immutable.dll
05/17/2023  10:57 AM           142,240 System.Memory.dll
05/17/2023  10:57 AM           115,856 System.Numerics.Vectors.dll
05/17/2023  10:57 AM           466,576 System.Reflection.Metadata.dll
05/17/2023  10:57 AM           245,888 System.Reflection.MetadataLoadContext.dll
05/17/2023  10:57 AM            61,568 System.Resources.Extensions.dll
05/17/2023  10:57 AM            18,024 System.Runtime.CompilerServices.Unsafe.dll
05/17/2023  10:57 AM            78,976 System.Text.Encodings.Web.dll
05/17/2023  10:57 AM           582,800 System.Text.Json.dll
05/17/2023  10:57 AM           181,376 System.Threading.Tasks.Dataflow.dll
05/17/2023  10:57 AM            25,984 System.Threading.Tasks.Extensions.dll
05/17/2023  10:57 AM            25,232 System.ValueTuple.dll

We should be able to delete System.Reflection.Metadata and System.Collections.Immutable too. Other msbuild tasks in the repo assume that msbuild comes with good version of these assemblies (e.g. see conversation at #85738 (comment)).

why are there ref assemblies of those DLLs in the output? Shouldn't those be actual DLLs that are loaded only on desktop FX?

No idea. Publishing of libraries projects has rough edges. I count this as one of the rough edges.

Looking at Roslyn, they eventually moved to multi-targeting the task, for a variety of reasons, but one of them was this kind of confusion about what needed to be included in the package.

I do not think we need to do that. We have much smaller support matrix compared to Roslyn (last SDK only, VS versions that have support for the given SDK only), so the current scheme is good enough for us. Also, other tasks in the repo are on the same plan.