Make compiler detection more resilient (#78402)

* Make compiler detection more resilient

This changes the way our build task finds the csc to launch to longer
depend on the `CompilerType` task argument. At the core for every
deployment kind of the task there is only one possible compiler that can
be loaded. Hence change our logic to better assert which deployment we
are in and locate the compiler based on that.

* more

* more

* PR feedback and tests

* rename APIs

* PR feedback

* Apply suggestions from code review

Co-authored-by: Fred Silberberg <fred@silberberg.xyz>

* PR feedback

* PR feedback

* PR feedback

---------

Co-authored-by: Fred Silberberg <fred@silberberg.xyz>

Author: jaredpar

src/Compilers/Core/MSBuildTask/ManagedCompiler.cs

@@ -493,21 +493,6 @@ public string? CompilerType
             get { return (string?)_store[nameof(CompilerType)]; }
         }
 
-        protected override RoslynCompilerType GetCompilerType()
-        {
-            if (string.IsNullOrWhiteSpace(CompilerType))
-            {
-                return DefaultCompilerType;
-            }
-
-            if (Enum.TryParse<RoslynCompilerType>(CompilerType, ignoreCase: true, out var compilerType))
-            {
-                return compilerType;
-            }
-
-            throw new ArgumentException($"Invalid {nameof(CompilerType)} '{CompilerType}' specified. Valid values are {string.Join(", ", Enum.GetNames(typeof(RoslynCompilerType)))}.");
-        }
-
         #endregion
 
         /// <summary>
@@ -544,7 +529,7 @@ internal int ExecuteTool(string pathToTool, string responseFileCommands, string
                 string? tempDirectory = Path.GetTempPath();
 
                 if (!UseSharedCompilation ||
-                    !IsManagedTool ||
+                    !UsingBuiltinTool ||
                     !BuildServerConnection.IsCompilerServerSupported)
                 {
                     LogCompilationMessage(logger, requestId, CompilationKind.Tool, $"using command line tool by design '{pathToTool}'");
@@ -555,10 +540,10 @@ internal int ExecuteTool(string pathToTool, string responseFileCommands, string
                 logger.Log($"CommandLine = '{commandLineCommands}'");
                 logger.Log($"BuildResponseFile = '{responseFileCommands}'");
 
-                var clientDirectory = Path.GetDirectoryName(PathToManagedTool);
+                var clientDirectory = Path.GetDirectoryName(PathToBuiltInTool);
                 if (clientDirectory is null || tempDirectory is null)
                 {
-                    LogCompilationMessage(logger, requestId, CompilationKind.Tool, $"using command line tool because we could not find client or temp directory '{PathToManagedTool}'");
+                    LogCompilationMessage(logger, requestId, CompilationKind.Tool, $"using command line tool because we could not find client or temp directory '{PathToBuiltInTool}'");
                     return base.ExecuteTool(pathToTool, responseFileCommands, commandLineCommands);
                 }
 
@@ -1247,11 +1232,4 @@ internal string? GetWin32ManifestSwitch
             return win32Manifest;
         }
     }
-
-    public enum RoslynCompilerType
-    {
-        Core,
-        Framework,
-        FrameworkPackage,
-    }
 }

src/Compilers/Core/MSBuildTask/ManagedToolTask.cs

@@ -4,6 +4,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.IO;
 using System.Resources;
 using System.Text;
@@ -15,8 +16,10 @@ namespace Microsoft.CodeAnalysis.BuildTasks
 {
     public abstract class ManagedToolTask : ToolTask
     {
+        private static bool DefaultIsSdkFrameworkToCoreBridgeTask { get; } = CalculateIsSdkFrameworkToCoreBridgeTask();
+
         /// <summary>
-        /// Is the standard tool being used here? When false the developer has specified a custom tool
+        /// Is the builtin tool being used here? When false the developer has specified a custom tool
         /// to be run by this task
         /// </summary>
         /// <remarks>
@@ -26,39 +29,33 @@ public abstract class ManagedToolTask : ToolTask
         /// ToolExe == ToolName, we know nothing is overridden, and
         /// we can use our own csc.
         /// </remarks>
-        protected bool IsManagedTool => string.IsNullOrEmpty(ToolPath) && ToolExe == ToolName;
+        protected bool UsingBuiltinTool => string.IsNullOrEmpty(ToolPath) && ToolExe == ToolName;
 
         /// <summary>
-        /// Used to determine the directory where the tools (like csc) are located.
-        /// See <see cref="Utilities.GenerateFullPathToTool"/>.
+        /// A copy of this task, compiled for .NET Framework, is deployed into the .NET SDK. It is a bridge task
+        /// that is loaded into .NET Framework MSBuild but launches the .NET Core compiler. This task necessarily
+        /// has different behaviors than the standard build task compiled for .NET Framework and loaded into the 
+        /// .NET Framework MSBuild.
         /// </summary>
-        protected virtual RoslynCompilerType GetCompilerType() => DefaultCompilerType;
-
-        protected const RoslynCompilerType DefaultCompilerType
-#if NET
-            = RoslynCompilerType.Core;
-#else
-            = RoslynCompilerType.Framework;
-#endif
+        /// <remarks>
+        /// This is mutable to facilitate testing
+        /// </remarks>
+        internal bool IsSdkFrameworkToCoreBridgeTask { get; init; } = DefaultIsSdkFrameworkToCoreBridgeTask;
 
-        internal string PathToManagedTool => Utilities.GenerateFullPathToTool(ToolName, GetCompilerType());
+        /// <summary>
+        /// Is the builtin tool executed by this task running on .NET Core?
+        /// </summary>
+        internal bool IsBuiltinToolRunningOnCoreClr => RuntimeHostInfo.IsCoreClrRuntime || IsSdkFrameworkToCoreBridgeTask;
 
-        private string PathToManagedToolWithoutExtension
-        {
-            get
-            {
-                var extension = Path.GetExtension(PathToManagedTool);
-                return PathToManagedTool.Substring(0, PathToManagedTool.Length - extension.Length);
-            }
-        }
+        internal string PathToBuiltInTool => Path.Combine(GetToolDirectory(), ToolName);
 
         protected ManagedToolTask(ResourceManager resourceManager)
             : base(resourceManager)
         {
         }
 
         /// <summary>
-        /// Generate the arguments to pass directly to the managed tool. These do not include
+        /// Generate the arguments to pass directly to the buitin tool. These do not include
         /// arguments in the response file.
         /// </summary>
         /// <remarks>
@@ -77,9 +74,9 @@ internal string GenerateToolArguments()
         protected sealed override string GenerateCommandLineCommands()
         {
             var commandLineArguments = GenerateToolArguments();
-            if (IsManagedTool)
+            if (UsingBuiltinTool && IsBuiltinToolRunningOnCoreClr)
             {
-                (_, commandLineArguments, _) = RuntimeHostInfo.GetProcessInfo(PathToManagedToolWithoutExtension, commandLineArguments);
+                commandLineArguments = RuntimeHostInfo.GetDotNetExecCommandLine(PathToBuiltInTool, commandLineArguments);
             }
 
             return commandLineArguments;
@@ -96,7 +93,7 @@ protected sealed override string GenerateResponseFileCommands()
         }
 
         /// <summary>
-        /// Generate the arguments to pass directly to the managed tool. These do not include
+        /// Generate the arguments to pass directly to the buitin tool. These do not include
         /// arguments in the response file.
         /// </summary>
         /// <remarks>
@@ -117,10 +114,12 @@ protected sealed override string GenerateResponseFileCommands()
         /// This could be the managed assembly itself (on desktop .NET on Windows),
         /// or a runtime such as dotnet.
         /// </summary>
-        protected sealed override string GenerateFullPathToTool() =>
-            IsManagedTool
-                ? RuntimeHostInfo.GetProcessInfo(PathToManagedToolWithoutExtension, string.Empty).processFilePath
-                : Path.Combine(ToolPath ?? "", ToolExe);
+        protected sealed override string GenerateFullPathToTool() => (UsingBuiltinTool, IsBuiltinToolRunningOnCoreClr) switch
+        {
+            (true, true) => RuntimeHostInfo.GetDotNetPathOrDefault(),
+            (true, false) => PathToBuiltInTool,
+            (false, _) => Path.Combine(ToolPath ?? "", ToolExe)
+        };
 
         protected abstract string ToolNameWithoutExtension { get; }
 
@@ -129,18 +128,17 @@ protected sealed override string GenerateFullPathToTool() =>
         protected abstract void AddResponseFileCommands(CommandLineBuilderExtension commandLine);
 
         /// <summary>
-        /// ToolName is only used in cases where <see cref="IsManagedTool"/> 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.
+        /// This is the file name of the builtin tool that will be executed.
         /// </summary>
         /// <remarks>
-        /// We *cannot* actually call IsManagedTool in the implementation of this method,
-        /// as the implementation of IsManagedTool calls this property. See the comment in
-        /// <see cref="ManagedToolTask.IsManagedTool"/>.
+        /// 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.
         /// </remarks>
-        protected sealed override string ToolName => RuntimeHostInfo.IsCoreClrRuntime
-            ? $"{ToolNameWithoutExtension}.dll"
-            : $"{ToolNameWithoutExtension}.exe";
+        protected sealed override string ToolName =>
+            IsBuiltinToolRunningOnCoreClr
+                ? $"{ToolNameWithoutExtension}.dll"
+                : $"{ToolNameWithoutExtension}.exe";
 
         /// <summary>
         /// This generates the command line arguments passed to the tool.
@@ -177,5 +175,60 @@ protected static ITaskItem[] GenerateCommandLineArgsTaskItems(List<string> comma
 
             return items;
         }
+
+        private string GetToolDirectory()
+        {
+            var buildTask = typeof(ManagedToolTask).Assembly;
+            var buildTaskDirectory = GetBuildTaskDirectory();
+#if NET
+            return Path.Combine(buildTaskDirectory, "bincore");
+#else
+            return IsSdkFrameworkToCoreBridgeTask
+                ? Path.Combine(buildTaskDirectory, "..", "bincore")
+                : buildTaskDirectory;
+#endif
+        }
+
+        /// <summary>
+        /// <see cref="IsSdkFrameworkToCoreBridgeTask"/>
+        /// </summary>
+        /// <remarks>
+        /// Using the file system as a way to differentiate between the two tasks is not ideal, but it is effective
+        /// and allows us to avoid significantly complicating the build process. The alternative is another parameter
+        /// to the Csc / Vbc / etc ... tasks that all invocations would need to pass along.
+        /// </remarks>
+        internal static bool CalculateIsSdkFrameworkToCoreBridgeTask()
+        {
+#if NET
+            return false;
+#else
+            // This logic needs to be updated when this issue is fixed. That moves csc.exe out to a subdirectory
+            // and hence the check below will need to change
+            //
+            // https://github.com/dotnet/roslyn/issues/78001
+
+            var buildTaskDirectory = GetBuildTaskDirectory();
+            var buildTaskDirectoryName = Path.GetFileName(buildTaskDirectory);
+            return
+                string.Equals(buildTaskDirectoryName, "binfx", StringComparison.OrdinalIgnoreCase) &&
+                !File.Exists(Path.Combine(buildTaskDirectory, "csc.exe")) &&
+                Directory.Exists(Path.Combine(buildTaskDirectory, "..", "bincore"));
+#endif
+        }
+
+        internal static string GetBuildTaskDirectory()
+        {
+            var buildTask = typeof(ManagedToolTask).Assembly;
+            var buildTaskDirectory = Path.GetDirectoryName(buildTask.Location);
+            if (buildTaskDirectory is null)
+            {
+                // This should not happen in supported product scenarios but could happen if 
+                // a non-supported scenario tried to load our task (like AOT) and call
+                // through these members.
+                throw new InvalidOperationException("Unable to determine the location of the build task assembly.");
+            }
+
+            return buildTaskDirectory;
+        }
     }
 }

src/Compilers/Core/MSBuildTask/TaskTestHost.cs

@@ -0,0 +1,21 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+
+namespace Microsoft.CodeAnalysis.BuildTasks
+{
+#if NETFRAMEWORK && DEBUG
+
+    /// <summary>
+    /// This class is used in <see cref="AppDomain"/> to allow us to test custom
+    /// disk layouts and its behavior in the context of the managed tool task.
+    /// </summary>
+    internal sealed class TaskTestHost : MarshalByRefObject
+    {
+        internal bool IsSdkFrameworkToCoreBridgeTask => ManagedToolTask.CalculateIsSdkFrameworkToCoreBridgeTask();
+    }
+
+#endif
+}

src/Compilers/Core/MSBuildTask/Utilities.cs

@@ -167,19 +167,5 @@ internal static Exception GetLocalizedArgumentException(string errorString,
             return assembly.Location;
 #endif
         }
-
-        /// <summary>
-        /// Generate the full path to the tool that is deployed with our build tasks.
-        /// </summary>
-        internal static string GenerateFullPathToTool(string toolFileName, RoslynCompilerType compilerType)
-        {
-            var buildTask = typeof(Utilities).GetTypeInfo().Assembly;
-            var assemblyPath = buildTask.Location;
-            var assemblyDirectory = Path.GetDirectoryName(assemblyPath)!;
-
-            return RuntimeHostInfo.IsDesktopRuntime
-                ? Path.Combine(assemblyDirectory, compilerType is RoslynCompilerType.Core ? "../bincore" : "", toolFileName)
-                : Path.Combine(assemblyDirectory, "bincore", toolFileName);
-        }
     }
 }