Clean up warnings building CLR tests #34155
Conversation
The CLR test build was producing two different warnings: 1. Invalid use of C# nullable reference types 1. Double import of disableversioncheck.targets Unwinding the double import was tricky due to the way several of the projects in this tree were double built with different variables which changed the directories that code was imported from. Eventually settled on an old C++ trick of using a set variable to avoid the future double import. Not particularly happy with this trick but also not a clear way to unwind the double imports here. Overall these warnings snuck into the build because warn as error was disabled for all the MSBuild invocations. That was apparently due to coreclr#19922 which has long since been closed. Hence I flipped back on warn as error here.
| powershell -NoProfile -ExecutionPolicy ByPass -NoLogo -Command "%__RepoRootDir%\eng\common\msbuild.ps1" %__ArcadeScriptArgs%^ | ||
| %__ProjectDir%\tests\build.proj -warnAsError:0 /t:BatchRestorePackages /nodeReuse:false^ | ||
| %__ProjectDir%\tests\build.proj -warnAsError:1 /t:BatchRestorePackages /nodeReuse:false^ |
There was a problem hiding this comment.
Should this also be changed for Unix:
runtime/src/coreclr/build-test.sh
Lines 438 to 439 in 7d67d17
There was a problem hiding this comment.
Good catch. I have another PR coming up this afternoon. Will include this.
Wanted to get this merged so I can start getting the timeline dumps cleaner. Makes it significantly easier to see what is causing build flakiness that way.
|
Most failures are known:
The XmlResolver failure appears to be a helix hiccup. I've started a FR thread to track this down: |
|
I don't know that warn-as-error is a good idea unless we have a way to force outerloop CI runs with any change that affects outerloop (or other non-innerloop pipeline). |
|
@BruceForstall the pendulum swings both ways here:
My instinct though is to ask why aren't we building these assets in the core CI loop? Is building the tests that burdensome here? |
ghost
locked as resolved and limited conversation to collaborators
The CLR test build was producing two different warnings:
Unwinding the double import was tricky due to the way several of the
projects in this tree were double built with different variables which
changed the directories that code was imported from. Eventually settled
on an old C++ trick of using a set variable to avoid the future double
import.
Not particularly happy with this trick but also not a clear way to
unwind the double imports here.
Overall these warnings snuck into the build because warn as error was
disabled for all the MSBuild invocations. That was apparently due to
coreclr#19922 which has long since been closed. Hence I flipped back on
warn as error here.