dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.07k stars 4.04k forks source link

Remove apphost from `csi` for source-build to avoid prebuilt dependency on a 3.1 apphost pack #57233

Open dagood opened 3 years ago

dagood commented 3 years ago

The csi project targets netcoreapp3.1 and outputs an Exe, so it uses Microsoft.NETCore.App.Host.linux-x64/3.1.18:

https://github.com/dotnet/roslyn/blob/1a415d2019d74bcf009134326d1c25798589de64/src/Interactive/csi/csi.csproj#L5-L9

This is a prebuilt dependency during source-build. The apphost pack contains binaries, so it isn't possible to make it available via https://github.com/dotnet/source-build-reference-packages.

I see that in the Microsoft-built SDK, csi doesn't seem to make it in, even as a framework-dependent DLL:

$ find . -iname csi*
[no output]

As a sanity check, csc does make it into the SDK as a framework-dependent assembly:

$ find . -iname csc*
./sdk/6.0.100-rc.2.21505.57/Roslyn/bincore/csc.deps.json
./sdk/6.0.100-rc.2.21505.57/Roslyn/bincore/csc.dll
./sdk/6.0.100-rc.2.21505.57/Roslyn/bincore/csc.runtimeconfig.json

csc uses <UseAppHost>false</UseAppHost>, which avoids this prebuilt:

https://github.com/dotnet/roslyn/blob/1a415d2019d74bcf009134326d1c25798589de64/src/Compilers/CSharp/csc/csc.csproj#L13

@sharwell @jaredpar


jaredpar commented 3 years ago

@tmat is there a reason you're aware of that we couldn't change this?

tmat commented 3 years ago

I think we'd want to keep building csi.exe. Can we make UseAppHost conditional on DotNetBuildFromSource?

dagood commented 3 years ago

I think we'd want to keep building csi.exe.

Do you have any more info about why? Whether it makes sense to exclude it from source-build depends on whether it's important for a source-build scenario.

tmat commented 3 years ago

So that one can run csi.exe from command line.

dagood commented 3 years ago

So that one can run csi.exe from command line.

If it's correct to say that one can only get csi/csi.exe from a NuGet package and the .NET product build never uses it, then it seems safe to exclude building it from source build conditionally. (Source-build packages aren't available to SDK users.)

I'd question the need to build csi/csi.exe for netcoreapp3.1 / Linux in the Microsoft build. If that can be removed, we can avoid a source-build-specific condition, and the Roslyn build ends up easier to maintain.

JoeRobich commented 3 years ago

I think we'd want to keep building csi.exe.

@tmat Since csi doesn't ship in the .NET SDK, why would we want it to be part of SourceBuild? We could always change this in the future if we wanted to ship csi as a inbox CLI tool (like fsi).

dagood commented 3 years ago

We could always change this in the future if we wanted to ship csi as a inbox CLI tool (like fsi).

That would actually be a problem, because the 3.1 apphost pack isn't available to source-build. csi would have to target the current target framework (net6.0 at the moment) and I don't know what requirements that would impose on the other projects and the layout of the compilers package.

(If there's some way to safely use the net6.0 apphost pack for a netcoreapp3.1 project, we could use that today as an alternative to everything discussed in this thread. Let me know if that's a thing, although I do doubt it.)

fsi actually isn't in the SDK right now either, and I have https://github.com/dotnet/fsharp/issues/12282 open about getting rid of that apphost (and the fsc apphost).

JoeRobich commented 3 years ago

@dagood Sorry for the phrasing. It was my hand wavy way of saying we can always enable Source Build in the future and do any necessary work to make it compatible.

tmat commented 3 years ago

I think we'd want to keep building csi.exe.

@tmat Since csi doesn't ship in the .NET SDK, why would we want it to be part of SourceBuild? We could always change this in the future if we wanted to ship csi as a inbox CLI tool (like fsi).

I meant we want to build it in general, not necessarily in source build. We can skip the whole project in source build.