Fix build task custom compiler logic #80948
Conversation
| /// hence we also compare <see cref="ToolTask.ToolExe"/> with <see cref="AppHostToolName"/> in addition to <see cref="ToolName"/>. | ||
| /// </remarks> | ||
| protected bool UsingBuiltinTool => string.IsNullOrEmpty(ToolPath) && ToolExe == ToolName; | ||
| protected bool UsingBuiltinTool => string.IsNullOrEmpty(ToolPath) && (ToolExe == ToolName || ToolExe == AppHostToolName); |
There was a problem hiding this comment.
I would just check against AppHostToolName only. Not sure there is vale in supporting CscToolExe comparing to csc.dll.
There was a problem hiding this comment.
That's what I had before I realized it doesn't work. If ToolExe is not overridden by a customer, it is equal to ToolName (e.g., csc.dll) and so ToolExe == AppHostToolName is false, which would lead to UsingBuiltinTool being false which is wrong. There are tests for that.
There was a problem hiding this comment.
You're right it also means we now support a scenario where customer explicitly sets ToolExe = "csc.dll" (but only if apphost is disabled) but I don't see a simple way around that.
|
@jaredpar @RikkiGibson @333fred for reviews, thanks |
| else | ||
| { | ||
| Assert.Contains("csc.dll", result.Output); | ||
| Assert.DoesNotContain("csc.exe", result.Output); |
There was a problem hiding this comment.
Should this be ExeExtension? Or do we need a regex to say that csc isn't matched, start/end word boundaries around it?
There was a problem hiding this comment.
Added ExeExtension and a trailing space to check for word boundary (regex would be more complicated since we would technically need to escape the pattern so . doesn't match everything...), thanks
Co-authored-by: Fred Silberberg <[email protected]>
3 participants
Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2615118.
Validation: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/684436 (the remaining failures seem to be because some parts of VS build set also ToolPath to a compiler that doesn't support the new core compiler option
/sdkpath- I will investigate if those can be fixed at the VS side - anyway those failures are not blocking as they won't appear until the workaround<RoslynCompilerType>Framework</RosynCompilerType>is reverted on the VS side - I did that only in this validation PR)