-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Plan for addressing torn SDK builds #41790
base: main
Are you sure you want to change the base?
Conversation
This is a _very rough_ draft
Co-authored-by: Fred Silberberg <[email protected]>
Co-authored-by: Rainer Sigwald <[email protected]>
Co-authored-by: Jan Jones <[email protected]>
Co-authored-by: Chris Sienkiewicz <[email protected]>
Co-authored-by: Sam Harwell <[email protected]>
Co-authored-by: Sam Harwell <[email protected]>
| dotnet msbuild | From .NET SDK | From .NET SDK | | ||
| Visual Studio Design Time | From Visual Studio | From Both | | ||
| Visual Studio Design Time | From Visual Studio | From Visuaal Studio | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Visual Studio Design Time | From Visual Studio | From Visuaal Studio | | |
| Visual Studio Design Time | From Visual Studio | From Visual Studio | |
1. The Windows .NET SDK increased in size even when it's not needed. For example our main Windows containers do not install Visual Studio hence are never in a torn state yet they'll see increased size cost here. Changing the .NET SDK such that these extra compilers are only installed sometimes would add significant complexity to our installer setup. | ||
2. This will make the Windows and Linux .NET SDK's visibly different in layout. Comparing them will show different contents which could be confusing to customers. At the same time the current proposal is functionally the same, it just changes where the framework compilers are placed on disk and that also could be confusing to customers. | ||
|
||
### Use the .NET Core compiler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping that we would explore this option as it would reduce of number of compilers and make the inbox assets in the .NET SDK the only driver. IIRC, other compiler teams in the .NET ecosystem actively consider going down that route.
I understand that there are drawbacks but to me this feels like the right option, long term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our source generators that ship inbox as part of the Microsoft.NETCore.App.Ref targeting pack need to target netstandard2.0 because we don't know which compiler loads them (the .NET Framework one vs the .NETCoreApp one). This is really weird as the API surface area is very limited and "Bcl" packages need to be referenced.
It would really be great if our inbox source generators could just target the TFM that the .NETCoreApp compiler targets (i.e. net8.0 / net9.0).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would really be great if our inbox source generators could just target the TFM that the .NETCoreApp compiler targets (i.e. net8.0 / net9.0).
This option only says that the .NET Core compiler would be used if you are in a torn state and run msbuild
. But your source generators would still need to be able to run in Visual Studio and hence target netstandard2.0, right?
I understand that there are drawbacks but to me this feels like the right option, long term.
This makes sense to me. There are drawbacks to the currently selected options as well, but we say, "well if you are in a torn state and don't like the drawbacks, get out of the torn state", maybe we could say that here as well. Furthermore, couldn't we make the .NET Core compiler when run in the torn state also run in a process named VBCSCompiler somehow? I didn't really understand the servicing lifetime drawback, doesn't it apply to the currently selected option as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This option only says that the .NET Core compiler would be used if you are in a torn state and run msbuild. But your source generators would still need to be able to run in Visual Studio and hence target netstandard2.0, right?
VS already carries a .NET runtime and SDK. Why not always depend on the .NETCoreApp version of the compiler, even inside VS? The .NETCoreApp variant might even be significantly faster (?).
- [MSBuild property for torn state detection](https://github.com/dotnet/installer/pull/19144) | ||
- [Long term build-server shutdown issue](https://github.com/dotnet/msbuild/issues/10035) | ||
|
||
[microsoft-common-tasks]: https://github.com/dotnet/msbuild/blob/main/src/Tasks/Microsoft.Common.tasks#L106-L109 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This link looks unused.
This describes the approach we're considering for addressing our torn .NET SDK build problem.