Fix build task custom compiler logic

.vscode/launch.json

@@ -70,7 +70,27 @@
       "program": "C:/Program Files/Microsoft Visual Studio/2022/Preview/MSBuild/Current/Bin/amd64/MSBuild.exe",
       "args": [
         "-restore",
+        "-p:RoslynCompilerType=Custom",
         "-p:RoslynTargetsPath=${workspaceFolder}/artifacts/bin/Microsoft.Net.Compilers.Toolset.Package/Debug/tasks/net472",
+        "-t:Rebuild",
+      ],
+      // A simple project that can be used to debug the build tasks against.
+      "cwd": "${workspaceFolder}/src/Tools/Source/CompilerGeneratorTools/Source/BoundTreeGenerator",
+      "stopAtEntry": false,
+      "console": "internalConsole"
+    },
+    {
+      "name": "Launch Microsoft.Build.Tasks.CodeAnalysis.dll via MSBuild.exe (netfx bridge)",
+      "type": "clr",
+      "request": "launch",
+      "preLaunchTask": "build toolset",
+      "program": "C:/Program Files/Microsoft Visual Studio/2022/Preview/MSBuild/Current/Bin/amd64/MSBuild.exe",
+      "args": [
+        "-restore",
+        "-p:RoslynCompilerType=Custom",
+        "-p:RoslynTargetsPath=${workspaceFolder}/artifacts/bin/Microsoft.Net.Compilers.Toolset.Package/Debug/tasks/net472",
+        "-p:RoslynTasksAssembly=${workspaceFolder}/artifacts/bin/Microsoft.Net.Compilers.Toolset.Package/Debug/tasks/netcore/binfx/Microsoft.Build.Tasks.CodeAnalysis.Sdk.dll",
+        "-t:Rebuild",
       ],
       // A simple project that can be used to debug the build tasks against.
       "cwd": "${workspaceFolder}/src/Tools/Source/CompilerGeneratorTools/Source/BoundTreeGenerator",
@@ -85,7 +105,9 @@
       "program": "C:/Program Files/Microsoft Visual Studio/2022/Preview/MSBuild/Current/Bin/amd64/MSBuild.exe",
       "args": [
         "-restore",
+        "-p:RoslynCompilerType=Custom",
         "-p:RoslynTargetsPath=${workspaceFolder}/artifacts/bin/Microsoft.Net.Compilers.Toolset.Package/Debug/tasks/netcore",
+        "-t:Rebuild",
       ],
       // A simple project that can be used to debug the build tasks against.
       "cwd": "${workspaceFolder}/src/Tools/Source/CompilerGeneratorTools/Source/BoundTreeGenerator",
@@ -100,7 +122,9 @@
       "program": "dotnet",
       "args": [
         "build",
+        "-p:RoslynCompilerType=Custom",
         "-p:RoslynTargetsPath=${workspaceFolder}/artifacts/bin/Microsoft.Net.Compilers.Toolset.Package/Debug/tasks/netcore",
+        "-t:Rebuild",
       ],
       // A simple project that can be used to debug the build tasks against.
       "cwd": "${workspaceFolder}/src/Tools/Source/CompilerGeneratorTools/Source/BoundTreeGenerator",

src/Compilers/Core/MSBuildTask/ManagedToolTask.cs

@@ -11,7 +11,6 @@
 using System.Text;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
-using Microsoft.CodeAnalysis.CommandLine;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.BuildTasks
@@ -48,8 +47,12 @@ public abstract class ManagedToolTask : ToolTask
         /// explicitly overridden.  So, if both ToolPath is unset and
         /// ToolExe == ToolName, we know nothing is overridden, and
         /// we can use our own csc.
+        /// <para />
+        /// We want to continue using the builtin tool if user sets <see cref="ToolTask.ToolExe"/> = <c>csc.exe</c>,
+        /// regardless of whether apphosts will be used or not (in the former case, <see cref="ToolName"/> could be <c>csc.dll</c>),
+        /// 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);
 
         /// <summary>
         /// Is the builtin tool executed by this task running on .NET Core?
@@ -75,6 +78,8 @@ private bool UseAppHost
             }
         }
 
+        internal bool UseAppHost_TestOnly { set => _useAppHost = value; }
+
         protected ManagedToolTask(ResourceManager resourceManager)
             : base(resourceManager)
         {
@@ -144,6 +149,12 @@ protected sealed override string GenerateFullPathToTool()
         {
             if (UsingBuiltinTool)
             {
+                // Even if we return full path to tool as "C:\Program Files\dotnet\dotnet.exe" for example,
+                // an MSBuild logic (caller of this function) can turn that into "C:\Program Files\dotnet\csc.exe" if `ToolExe` is set explicitly to "csc.exe".
+                // Resetting `ToolExe` to `null` skips that logic. That is a correct thing to do wince we already checked `UsingBuiltinTool`
+                // which means `ToolExe` is not really overridden by user (yes, the user sets it but basically to its default value).
+                ToolExe = null;
+
                 return UseAppHost ? PathToBuiltInTool : RuntimeHostInfo.GetDotNetPathOrDefault();
             }
 
@@ -162,7 +173,7 @@ protected sealed override string GenerateFullPathToTool()
         /// <remarks>
         /// ToolName is only used in cases where <see cref="UsingBuiltinTool"/> returns true.
         /// It returns the name of the managed assembly, which might not be the path returned by
-        /// GenerateFullPathToTool, which can return the path to e.g. the dotnet executable.
+        /// <see cref="GenerateFullPathToTool"/>, which can return the path to e.g. the dotnet executable.
         /// </remarks>
         protected sealed override string ToolName
         {
@@ -210,7 +221,7 @@ protected static ITaskItem[] GenerateCommandLineArgsTaskItems(List<string> comma
             return items;
         }
 
-        private static string GetToolDirectory()
+        internal static string GetToolDirectory()
         {
             var buildTaskDirectory = GetBuildTaskDirectory();
 #if NET

src/Compilers/Core/MSBuildTaskTests/CscTests.cs

@@ -494,6 +494,30 @@ public void EmptyCscToolExe()
             AssertEx.Equal(Path.Combine("path", "to", "custom_csc", $"csc{PlatformInformation.ExeExtension}"), csc.GeneratePathToTool());
         }
 
+        /// <summary>
+        /// Setting ToolExe to "csc.exe" should use the built-in compiler regardless of apphost being used or not.
+        /// </summary>
+        [Theory, CombinatorialData, WorkItem("https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2615118")]
+        public void BuiltInToolExe(bool useAppHost, bool setToolExe)
+        {
+            var csc = new Csc();
+            csc.UseAppHost_TestOnly = useAppHost;
+            if (setToolExe)
+            {
+                csc.ToolExe = $"csc{PlatformInformation.ExeExtension}";
+            }
+            if (useAppHost)
+            {
+                AssertEx.Equal(csc.PathToBuiltInTool, csc.GeneratePathToTool());
+                AssertEx.Equal("", csc.GenerateCommandLineContents());
+            }
+            else
+            {
+                AssertEx.Equal(RuntimeHostInfo.GetDotNetPathOrDefault(), csc.GeneratePathToTool());
+                AssertEx.Equal(RuntimeHostInfo.GetDotNetExecCommandLine(csc.PathToBuiltInTool, ""), csc.GenerateCommandLineContents());
+            }
+        }
+
         [Fact]
         public void EditorConfig()
         {

src/Compilers/Core/MSBuildTaskTests/TestUtilities/IntegrationTestBase.cs

@@ -7,6 +7,7 @@
 using System.Linq;
 using Microsoft.CodeAnalysis.Test.Utilities;
 using Roslyn.Test.Utilities;
+using Roslyn.Utilities;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -119,72 +120,151 @@ protected static ProcessResult RunCommandLineCompiler(
             additionalEnvironmentVars);
     }
 
+    /// <param name="overrideToolExe">
+    /// Setting ToolExe to "csc.exe" should use the built-in compiler regardless of apphost being used or not.
+    /// </param>
     [Theory, CombinatorialData]
-    public void SdkBuild_Csc(bool useSharedCompilation)
+    [WorkItem("https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2615118")]
+    public void SdkBuild_Csc(bool useSharedCompilation, bool overrideToolExe, bool useAppHost)
     {
-        var result = RunMsbuild(
-            "/v:n /m /nr:false /t:Build /restore Test.csproj",
-            _tempDirectory,
-            new Dictionary<string, string>
+        if (!ManagedToolTask.IsBuiltinToolRunningOnCoreClr && !useAppHost)
+        {
+            _output.WriteLine("Skipping test case: netfx compiler always uses apphost.");
+            return;
+        }
+
+        var originalAppHost = Path.Combine(ManagedToolTask.GetToolDirectory(), $"csc{PlatformInformation.ExeExtension}");
+        var backupAppHost = originalAppHost + ".bak";
+        if (!useAppHost)
+        {
+            _output.WriteLine($"Apphost: {originalAppHost}");
+            File.Move(originalAppHost, backupAppHost);
+        }
+
+        ProcessResult? result;
+
+        try
+        {
+            result = RunMsbuild(
+                "/v:n /m /nr:false /t:Build /restore Test.csproj" +
+                    (overrideToolExe ? $" /p:CscToolExe=csc{PlatformInformation.ExeExtension}" : ""),
+                _tempDirectory,
+                new Dictionary<string, string>
+                {
+                    { "File.cs", """
+                        class Program { static void Main() { System.Console.WriteLine("Hello from file"); } }
+                        """ },
+                    { "Test.csproj", $"""
+                        <Project Sdk="Microsoft.NET.Sdk">
+                            <UsingTask TaskName="Microsoft.CodeAnalysis.BuildTasks.Csc" AssemblyFile="{_buildTaskDll}" />
+                            <PropertyGroup>
+                                <TargetFramework>netstandard2.0</TargetFramework>
+                                <UseSharedCompilation>{useSharedCompilation}</UseSharedCompilation>
+                            </PropertyGroup>
+                        </Project>
+                        """ },
+                });
+        }
+        finally
+        {
+            if (!useAppHost)
             {
-                { "File.cs", """
-                    class Program { static void Main() { System.Console.WriteLine("Hello from file"); } }
-                    """ },
-                { "Test.csproj", $"""
-                    <Project Sdk="Microsoft.NET.Sdk">
-                        <UsingTask TaskName="Microsoft.CodeAnalysis.BuildTasks.Csc" AssemblyFile="{_buildTaskDll}" />
-                        <PropertyGroup>
-                            <TargetFramework>netstandard2.0</TargetFramework>
-                            <UseSharedCompilation>{useSharedCompilation}</UseSharedCompilation>
-                        </PropertyGroup>
-                    </Project>
-                    """ },
-            });
+                File.Move(backupAppHost, originalAppHost);
+            }
+        }
 
         if (result == null) return;
 
         _output.WriteLine(result.Output);
 
         Assert.Equal(0, result.ExitCode);
         Assert.Contains(useSharedCompilation ? "server processed compilation" : "using command line tool by design", result.Output);
-        Assert.DoesNotContain("csc.dll", result.Output);
-        Assert.Contains(ExecutionConditionUtil.IsWindows ? "csc.exe" : "csc", result.Output);
+
+        if (useAppHost)
+        {
+            Assert.DoesNotContain("csc.dll", result.Output);
+            Assert.Contains($"csc{PlatformInformation.ExeExtension}", result.Output);
+        }
+        else
+        {
+            Assert.Contains("csc.dll", result.Output);
+            Assert.DoesNotContain("csc.exe", result.Output);
+        }
     }
 
+    /// <param name="overrideToolExe">
+    /// Setting ToolExe to "vbc.exe" should use the built-in compiler regardless of apphost being used or not.
+    /// </param>
     [Theory, CombinatorialData]
-    public void SdkBuild_Vbc(bool useSharedCompilation)
+    [WorkItem("https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2615118")]
+    public void SdkBuild_Vbc(bool useSharedCompilation, bool overrideToolExe, bool useAppHost)
     {
-        var result = RunMsbuild(
-            "/v:n /m /nr:false /t:Build /restore Test.vbproj",
-            _tempDirectory,
-            new Dictionary<string, string>
+        if (!ManagedToolTask.IsBuiltinToolRunningOnCoreClr && !useAppHost)
+        {
+            _output.WriteLine("Skipping test case: netfx compiler always uses apphost.");
+            return;
+        }
+
+        var originalAppHost = Path.Combine(ManagedToolTask.GetToolDirectory(), $"vbc{PlatformInformation.ExeExtension}");
+        var backupAppHost = originalAppHost + ".bak";
+        if (!useAppHost)
+        {
+            File.Move(originalAppHost, backupAppHost);
+        }
+
+        ProcessResult? result;
+
+        try
+        {
+            result = RunMsbuild(
+                "/v:n /m /nr:false /t:Build /restore Test.vbproj" +
+                    (overrideToolExe ? $" /p:VbcToolExe=vbc{PlatformInformation.ExeExtension}" : ""),
+                _tempDirectory,
+                new Dictionary<string, string>
+                {
+                    { "File.vb", """
+                        Public Module Program
+                            Public Sub Main()
+                                System.Console.WriteLine("Hello from file")
+                            End Sub
+                        End Module
+                        """ },
+                    { "Test.vbproj", $"""
+                        <Project Sdk="Microsoft.NET.Sdk">
+                            <UsingTask TaskName="Microsoft.CodeAnalysis.BuildTasks.Vbc" AssemblyFile="{_buildTaskDll}" />
+                            <PropertyGroup>
+                                <TargetFramework>netstandard2.0</TargetFramework>
+                                <UseSharedCompilation>{useSharedCompilation}</UseSharedCompilation>
+                            </PropertyGroup>
+                        </Project>
+                        """ },
+                });
+        }
+        finally
+        {
+            if (!useAppHost)
             {
-                { "File.vb", """
-                    Public Module Program
-                        Public Sub Main()
-                            System.Console.WriteLine("Hello from file")
-                        End Sub
-                    End Module
-                    """ },
-                { "Test.vbproj", $"""
-                    <Project Sdk="Microsoft.NET.Sdk">
-                        <UsingTask TaskName="Microsoft.CodeAnalysis.BuildTasks.Vbc" AssemblyFile="{_buildTaskDll}" />
-                        <PropertyGroup>
-                            <TargetFramework>netstandard2.0</TargetFramework>
-                            <UseSharedCompilation>{useSharedCompilation}</UseSharedCompilation>
-                        </PropertyGroup>
-                    </Project>
-                    """ },
-            });
+                File.Move(backupAppHost, originalAppHost);
+            }
+        }
 
         if (result == null) return;
 
         _output.WriteLine(result.Output);
 
         Assert.Equal(0, result.ExitCode);
         Assert.Contains(useSharedCompilation ? "server processed compilation" : "using command line tool by design", result.Output);
-        Assert.DoesNotContain("vbc.dll", result.Output);
-        Assert.Contains(ExecutionConditionUtil.IsWindows ? "vbc.exe" : "vbc", result.Output);
+
+        if (useAppHost)
+        {
+            Assert.DoesNotContain("vbc.dll", result.Output);
+            Assert.Contains($"vbc{PlatformInformation.ExeExtension}", result.Output);
+        }
+        else
+        {
+            Assert.Contains("vbc.dll", result.Output);
+            Assert.DoesNotContain("vbc.exe", result.Output);
+        }
     }
 
     [Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/79907")]

src/Compilers/Core/MSBuildTaskTests/VbcTests.cs

@@ -7,6 +7,7 @@
 using Microsoft.CodeAnalysis.BuildTasks;
 using Microsoft.CodeAnalysis.BuildTasks.UnitTests.TestUtilities;
 using Roslyn.Test.Utilities;
+using Roslyn.Utilities;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -434,6 +435,30 @@ public void DisableSdkPath()
             Assert.Equal(@"/optionstrict:custom /nosdkpath", vbc.GenerateResponseFileContents());
         }
 
+        /// <summary>
+        /// Setting ToolExe to "vbc.exe" should use the built-in compiler regardless of apphost being used or not.
+        /// </summary>
+        [Theory, CombinatorialData, WorkItem("https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2615118")]
+        public void BuiltInToolExe(bool useAppHost, bool setToolExe)
+        {
+            var vbc = new Vbc();
+            vbc.UseAppHost_TestOnly = useAppHost;
+            if (setToolExe)
+            {
+                vbc.ToolExe = $"vbc{PlatformInformation.ExeExtension}";
+            }
+            if (useAppHost)
+            {
+                AssertEx.Equal(vbc.PathToBuiltInTool, vbc.GeneratePathToTool());
+                AssertEx.Equal("", vbc.GenerateCommandLineContents());
+            }
+            else
+            {
+                AssertEx.Equal(RuntimeHostInfo.GetDotNetPathOrDefault(), vbc.GeneratePathToTool());
+                AssertEx.Equal(RuntimeHostInfo.GetDotNetExecCommandLine(vbc.PathToBuiltInTool, ""), vbc.GenerateCommandLineContents());
+            }
+        }
+
         [Fact]
         public void EditorConfig()
         {