Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dagood opened this issue Oct 19, 2021 · 9 comments

Comments

@dagood
Copy link
Member

dagood commented Oct 19, 2021

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

<PropertyGroup>
<OutputType>Exe</OutputType>
<RootNamespace>CSharpInteractive</RootNamespace>
<TargetFrameworks>netcoreapp3.1;net472</TargetFrameworks>
</PropertyGroup>

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:

<UseAppHost>false</UseAppHost>

  • Is it safe to add to csi for source-build?
  • Is the framework-dependent executable needed for the Microsoft build, or can we remove apphost usage altogether for csi to unify the builds?

@sharwell @jaredpar


@jaredpar
Copy link
Member

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

@tmat
Copy link
Member

tmat commented Oct 19, 2021

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

@dagood
Copy link
Member Author

dagood commented Oct 19, 2021

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
Copy link
Member

tmat commented Oct 20, 2021

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

@dagood
Copy link
Member Author

dagood commented Oct 20, 2021

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.

@jinujoseph jinujoseph added Concept-Continuous Improvement and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 20, 2021
@jinujoseph jinujoseph added this to the 17.1 milestone Oct 20, 2021
@JoeRobich
Copy link
Member

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
Copy link
Member Author

dagood commented Oct 20, 2021

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 dotnet/fsharp#12282 open about getting rid of that apphost (and the fsc apphost).

@JoeRobich
Copy link
Member

@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
Copy link
Member

tmat commented Oct 21, 2021

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.

lbussell pushed a commit to lbussell/roslyn that referenced this issue Jan 7, 2022
Creating an apphost for a netcoreapp3.1 project uses the apphost pack as a
prebuilt. Stopping these projects from creating the apphost removes the prebuilt
for source-build.

See: dotnet#57233

PR: dotnet#57306
lbussell pushed a commit to lbussell/roslyn that referenced this issue Jan 25, 2022
Creating an apphost for a netcoreapp3.1 project uses the apphost pack as a
prebuilt. Stopping these projects from creating the apphost removes the prebuilt
for source-build.

See: dotnet#57233

PR: dotnet#57306
@jinujoseph jinujoseph modified the milestones: 17.1, 17.3 Apr 27, 2022
@arunchndr arunchndr modified the milestones: 17.3, 17.6 P2 Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants