Don't reference netstandard1.x packages

[ViktorHofer]

... when building from source.

This was the only remaining project in roslyn that brought them in.

Contributes to dotnet/source-build#4482

[jjonescz]

@ViktorHofer source build is failing

[ViktorHofer]

@MichalStrehovsky just to double check, src/Scripting/Core/Microsoft.CodeAnalysis.Scripting.csproj shouldn't be referencing the old System.Runtime.Loader package on netstandard2.0 and we should convert AssemblyLoadContext usages to AssemblyLoader when targeting netfx or netstandard, right?

[MichalStrehovsky]

@MichalStrehovsky just to double check, src/Scripting/Core/Microsoft.CodeAnalysis.Scripting.csproj shouldn't be referencing the old System.Runtime.Loader package on netstandard2.0 and we should convert AssemblyLoadContext usages back to AssemblyLoader when targeting netfx or netstandard, right?

Yeah, I don't think the System.Runtime.Loader would do anything good on netfx. Cc @dotnet/appmodel

[ViktorHofer]

What makes this problematic is that Microsoft.CodeAnalysis.Scripting dynamically instantiates either an AssemblyLoadContext or an AssemblyLoader:

roslyn/src/Scripting/Core/Hosting/AssemblyLoader/AssemblyLoaderImpl.cs

Lines 24 to 34 in 7039c4c

	public static AssemblyLoaderImpl Create(InteractiveAssemblyLoader loader)
	{
	if (CoreClrShim.AssemblyLoadContext.Type != null)
	{
	return CreateCoreImpl(loader);
	}
	else
	{
	return new DesktopAssemblyLoaderImpl(loader);
	}
	}

[ViktorHofer]

cc @jaredpar @MichaelSimons @ericstj

The goal is to remove all netstandard1.x package dependencies when building from source (see dotnet/source-build#4482). Roslyn is one of the few remaining repos that bring a netstandard1.x package in. For roslyn it's only a single reference in a single place: System.Runtime.Loader/4.3.0 by Microsoft.CodeAnalysis.Scripting.csproj.

See my comments above. It's hard to get rid off that package as roslyn dynamically instantiates either an ALC or AssemblyLoader.

[jaredpar]

@tmat, @arkalyanms FYI

[ericstj]

For roslyn it's only a single reference in a single place: System.Runtime.Loader/4.3.0 by Microsoft.CodeAnalysis.Scripting.csproj.

As a workaround you could PackageDownload then raw-reference the file. That will avoid pulling the dependencies.

[ViktorHofer]

As a workaround you could PackageDownload then raw-reference the file. That will avoid pulling the dependencies.

Yes, I considered that. The goal here is to remove all netstandard1.x packages. If we keep the System.Runtime.Loader reference then we would need to continue building it in SBRP including its dependencies.

[jaredpar]

The compiler approaches this by representing assembly loading through an interface at the netstandard2.0 layer. Then we have our .NET Framework / Core implementations that are fed through by the host. The actual host is going to target Core or Framework so it can be definitive in what it references.

That model was designed into the compiler very early on though. Not sure if the scripting layer can take this approach or not.

[ViktorHofer]

@tmat can you please take a look? We need to stop referencing netstandard1.x packages directly as those will eventually be marked as deprecated and together with NuGet's new Audit mode, such package references will result in errors.

[ViktorHofer]

@arkalyanms / @jaredpar can you please help me find someone to take a look at this? I will be out for an extended time starting in about two weeks and would ideally want to complete this by then.

[jaredpar]

can you please help me find someone to take a look at this?

What are you looking for feedback on? Is the PR as written not meeting the goal here of removing netstandard1.x packages?

[ViktorHofer]

The changes in this PR aren't complete as when building from source, the src/Scripting/Core/Microsoft.CodeAnalysis.Scripting.csproj project also gets built for netstandard2.0 (which I realized after opening this PR and hence converted it to draft). When building for netstandard2.0, the System.Runtime.Loader package is still referenced.

I'm trying to understand how to avoid the package reference entirely and if such a change would be acceptable. As mentioned above, this will help the regular (msft) build and source-build as we will eventually deprecate these packages.

[tmat]

Scripting uses reflection light-up. If the app that loads the library runs on .NET Framework we use desktop AssemblyLoader via reflection. If it runs on .NET Core we use ALC (directly - hence the reference to System.Runtime.Loader).

The project is now multi-targeting so I think we can use #ifdef instead of light-up.

[ViktorHofer]

Searching through the dotnet GH org I found a few projects that currently target .NET Standard and rely on the "Microsoft.CodeAnalysis.Scripting.Common" package.

• https://github.com/dotnet/interactive/blob/99446ae099a78d71d36ad1044ebabf3771d154f9/src/Microsoft.DotNet.Interactive.CSharp/Microsoft.DotNet.Interactive.CSharp.csproj#L31

• roslyn/src/Interactive/Host/Microsoft.CodeAnalysis.InteractiveHost.csproj

  Line 21 in 5866fab

  <ProjectReference Include="..\..\Scripting\Core\Microsoft.CodeAnalysis.Scripting.csproj">

• roslyn/src/EditorFeatures/Core/Microsoft.CodeAnalysis.EditorFeatures.csproj

  Line 24 in 5866fab

  <ProjectReference Include="..\..\Scripting\Core\Microsoft.CodeAnalysis.Scripting.csproj" Aliases="Scripting,global" />

• roslyn/src/Features/Core/Portable/Microsoft.CodeAnalysis.Features.csproj

  Line 19 in 5866fab

  <ProjectReference Include="..\..\..\Scripting\Core\Microsoft.CodeAnalysis.Scripting.csproj" Aliases="Scripting,global" />

If we remove the netstandard2.0 TFM, then we would also need to at least update these projects.

The project is now multi-targeting so I think we can use #ifdef instead of light-up.

That sounds right to me. @tmat can you help with that?

[tmat]

Working on it.

[tmat]

PR ready: #74409

[ViktorHofer]

Closing in favor of #74409