Roslyn has introduced many source-built prebuilts #69847
Comments
dotnet-issue-labeler
bot
added
Area-Infrastructure
untriaged
|
Background first: Microsoft.CodeAnalysis.Workspaces.MSBuild is a Roslyn package which a tool author can use to take a solution/projects on disk and load them into Roslyn so you can look at the syntax trees, semantics, etc. For example, dotnet format uses it to load the syntax trees to then start formatting them. One consistent problem we've had with MSBuildWorkspace is since it's using MSBuild in-process, it can only support projects that match your runtime type and/or version; for example a tool compiled against .NET Framework may choke on .NET Core projects and vice-versa. Because of needs for the C# extension we're generally enhancing it so it can launch a separate process (the BuildHost binary) and do out-of-proc builds and then pass the stuff back in process. Now a few questions I have:
@mthalman: There shouldn't be a BuildHost package, since at the moment we're still shipping the BuildHost binary as a "regular binary" to be loaded in-process as before since we're doing this work one step at a time. There was a bug that was fixed in this PR last night that removed an accidental reference to a non-existent package. So if you are still seeing a package with a reference to a that non-existent package, then something is up and I need to dig further.
At the moment, the code behavior is the same before for the package and this was supposed to be an under-the-hood change. We have other code outside the SDK consuming the BuildHost and the StreamJsonRpc portion of it, so for purposes of unblocking at the moment we absolutely can restrict those references to non-source-build only if that's the easiest solution. Although we will need to revisit this at some point; if there's a different RPC library that's also already sourcebuild friendly I can switch to that too...StreamJsonRpc was just chosen as we use it in other parts of our stack, not because of any specific feature in it. |
No issue here. I should have referred to them as projects instead of packages.
That doesn't seem to be the case. Or maybe I'm misinterpreting what you're saying. There was never a dependency on StreamJsonRpc from Microsoft.CodeAnalysis.Workspaces.MSBuild prior to the addition of the BuildHost.
It's not the code that's referencing BuildHost that's the issue. It's BuildHost itself, which has the dependency on StreamJsonRpc, that's the issue. If the implementation which uses StreamJsonRpc is not relevant for the SDK itself, can it be factored out into a separate project that we then don't include with source-build? |
I might have not explained well: yes, the dependency is new, but at the moment for purposes of the SDK we're not actually using the process-launching part of it. We're using that in other places for now. That'll change in a month or so though, so longer term we still need a plan. Stepping back for a moment, what should this look like? Either we have to make StreamJsonRpc be part of source-build-externals, or we move to a different RPC library? |
Got it. So any workaround to remove the dependency now won't be relevant once the SDK requires the use of the BuildHost.
Our only options are to depend on code which 1) ships with the .NET SDK or 2) is a package external to the SDK which would then need to be configured in source-build-externals. The issue with StreamJsonRpc is that it includes other dependencies as well which would need to be source-built: MessagePack and Nerdbank.Streams. If there are other vetted libraries available that have no dependencies on non-.NET-owned packages, that would be ideal. |
Or at least, I don't want to spend more effort on a workaround than necessary, or at least limit stuff we'd have to undo later. I know we have projects now that condition things on DotNetBuildFromSource in MSBuild; I can trivially add some condition checks to the particular source files to unblock this if that's an easy short term solution. |
If that's easy, that would buy some time and unblock things. Are the changes to eventually include the dependency from the SDK intended for .NET 8? |
The schedule for that isn't tied to 8.0; this is a bit of funky fallout of the fact that Roslyn repository ships things in different vehicles -- I'm honestly quite surprised that Roslyn main was even shipping in 8.0 given it's in RC. So we can delay after whenever that snap is, but I have no idea what that date is. |
|
@mthalman Is gRPC already within source build or not? |
genlu
removed
the
untriaged
Perfect. Glad to hear this isn't a .NET 8 thing. This gives us a lot of time then.
No. That's part of https://github.com/grpc/grpc-dotnet. Nothing which ships as part of the SDK has a dependency on that. @jasonmalinowski, can you log an issue for this that describes the need for RPC and the fact that it will have have source-build implications? It'd be good to have something to track this and we can discuss what our options are. |
|
I think rather than try (re)inventing an RPC approach we're just going to restructure our code a bit. What we were doing was spinning up an external process which is running MSBuild APIs; we can probably restructure things where we just launch the process and pass all the projects we want to analyze in command line switches, just have it output a single JSON blob and then exit. That might work fine and just avoid the RPC need entirely. |
|
I'm reopening since there are still many prebuilts remaining: These can be verified by looking at the source-build within roslyn CI. The prebuilts are listed at the bottom of the build log. |
|
Remaining fixes for prebuilts:
Here's a binlog that can be used to track down Microsoft.DiaSymReader: |
The following source-build prebuilts have been introduced by Roslyn in the dependency flow into installer: dotnet/installer#17319:
These stem from the following packages:
These prebuilts were not caught by roslyn CI because of #69846.
Here is the attached prebuilt report for more details:
prebuilt-usage.zip
One of the more concerning of these prebuilts is the dependency on StreamJsonRpc. This is produced outside of .NET and would need to be included in source-build-externals.
One thing we need to determine first is whether Microsoft.CodeAnalysis.Workspaces.MSBuild.BuildHost needs to be source-built at all. We need to understand its purpose and whether its relevant to a source-built SDK. @jasonmalinowski - Can you provide more info here?
cc @MichaelSimons
The text was updated successfully, but these errors were encountered: