-
Notifications
You must be signed in to change notification settings - Fork 447
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
Patches to fix SB bootstrap flow #15532
Patches to fix SB bootstrap flow #15532
Conversation
Date: Tue, 14 Feb 2023 10:57:06 -0600 | ||
Subject: [PATCH] Disable errors for FS1182 | ||
|
||
--- |
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.
All patches should have a Backport: <Issue/PR reference>
- e.g. https://github.com/dotnet/installer/blob/main/src/SourceBuild/patches/command-line-api/0001-Build-System.CommandLine.NamingConventionBinder-duri.patch#L7. This makes patch maintenance easier.
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.
The issue/pr when fixed should remove the need for the patch.
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 didn't do this for the roslyn patch because that's something to be addressed on the source-build side.
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.
We should still link the source-build issue(s) that will address the need for this patch.
This is replaced by the repo fix: dotnet/fsharp#14750
fsharp:
An analyzer warning, treated as an error, for FS1182 was causing a build failure due to an unused variable.
roslyn-analyzers:
An invalid cast exception was occurring when attempting to run a Roslyn analyzer during the sdk build. This is fixed by dotnet/roslyn-analyzers#6476 which I've applied the patch for here.
roslyn:
The IdeBenchmarks project was failing to build because it was attempting to load the Microsoft.NET.Sdk.WindowsDesktop.targets. It seemed that the
ToolsDir
property wasn't flowing correctly which prevented the use of the EmptySdk for this scenario. However, the IdeBenchmarks project shouldn't be built at all. It's already configured to be excluded from source-build: https://github.com/dotnet/roslyn/blob/f3e29bfd6cfccdbd40bf28f295da11155a62f284/src/Tools/Directory.Build.props#L4. But there are existing issues with that flag: dotnet/source-build#1736, dotnet/source-build#1825.