Merge pull request #22487 from khyperia/csctoolexe_fullpath

Allow empty CscToolPath

src/Compilers/Core/MSBuildTask/DotnetHost.cs

@@ -6,46 +6,40 @@ namespace Microsoft.CodeAnalysis.BuildTasks
 {
     internal sealed class DotnetHost
     {
-        public string ToolNameOpt { get; }
-
         public string PathToToolOpt { get; }
 
         public string CommandLineArgs { get; }
 
-        private DotnetHost(string toolNameOpt, string pathToToolOpt, string commandLineArgs)
+        /// <summary>
+        /// True if this tool is a managed executable, and will be invoked with `dotnet`.
+        /// False if this executable is being invoked directly
+        /// (it may still be a managed executable, but will be invoked directly).
+        /// </summary>
+        public bool IsManagedInvocation { get; }
+
+        private DotnetHost(string pathToToolOpt, string commandLineArgs, bool isManagedInvocation)
         {
-            ToolNameOpt = toolNameOpt;
             PathToToolOpt = pathToToolOpt;
             CommandLineArgs = commandLineArgs;
+            IsManagedInvocation = isManagedInvocation;
         }
 
-        public static DotnetHost CreateUnmanagedToolInvocation(string pathToTool, string commandLineArgs)
+        public static DotnetHost CreateNativeInvocationTool(string pathToTool, string commandLineArgs)
         {
-            return new DotnetHost(null, pathToTool, commandLineArgs);
+            return new DotnetHost(pathToTool, commandLineArgs, isManagedInvocation: false);
         }
 
-        public static DotnetHost CreateManagedToolInvocation(string toolNameWithoutExtension, string commandLineArgs)
+        public static DotnetHost CreateManagedInvocationTool(string toolName, string commandLineArgs)
         {
-            string pathToToolOpt;
-            string toolName;
-            if (IsCliHost(out string pathToDotnet))
-            {
-                toolName = $"{toolNameWithoutExtension}.dll";
-                pathToToolOpt = Utilities.GenerateFullPathToTool(toolName);
-                if (pathToToolOpt != null)
-                {
-                    commandLineArgs = PrependFileToArgs(pathToToolOpt, commandLineArgs);
-                    pathToToolOpt = pathToDotnet;
-                }
-            }
-            else
+            var pathToToolOpt = Utilities.GenerateFullPathToTool(toolName);
+            // Desktop executes tool directly, only prepend if we're on CLI
+            if (pathToToolOpt != null && IsCliHost(out string pathToDotnet))
             {
-                // Desktop executes tool directly
-                toolName = $"{toolNameWithoutExtension}.exe";
-                pathToToolOpt = Utilities.GenerateFullPathToTool(toolName);
+                commandLineArgs = PrependFileToArgs(pathToToolOpt, commandLineArgs);
+                pathToToolOpt = pathToDotnet;
             }
 
-            return new DotnetHost(toolName, pathToToolOpt, commandLineArgs);
+            return new DotnetHost(pathToToolOpt, commandLineArgs, isManagedInvocation: true);
         }
 
         private static bool IsCliHost(out string pathToDotnet)

src/Compilers/Core/MSBuildTask/InteractiveCompiler.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using System.IO;
 using System.Linq;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
@@ -190,21 +191,7 @@ private DotnetHost DotnetHostInfo
                     AddCommandLineCommands(commandLineBuilder);
                     var commandLine = commandLineBuilder.ToString();
 
-                    if (string.IsNullOrEmpty(ToolPath))
-                    {
-                        _dotnetHostInfo = DotnetHost.CreateManagedToolInvocation(ToolNameWithoutExtension, commandLine);
-
-                        // See comment in ManagedCompiler.cs on why this if statement is here.
-                        if (ToolExe != _dotnetHostInfo.ToolNameOpt)
-                        {
-                            _dotnetHostInfo = DotnetHost.CreateUnmanagedToolInvocation(ToolPath, commandLine);
-                        }
-                    }
-                    else
-                    {
-                        // Explicitly provided ToolPath, don't try to figure anything out
-                        _dotnetHostInfo = DotnetHost.CreateUnmanagedToolInvocation(ToolPath, commandLine);
-                    }
+                    _dotnetHostInfo = ManagedCompiler.CreateDotnetHostInfo(ToolPath, ToolExe, ToolName, commandLine);
                 }
                 return _dotnetHostInfo;
             }
@@ -214,7 +201,7 @@ private DotnetHost DotnetHostInfo
 
         protected abstract string ToolNameWithoutExtension { get; }
 
-        protected sealed override string ToolName => DotnetHostInfo.ToolNameOpt;
+        protected sealed override string ToolName => ManagedCompiler.GenerateToolName(ToolNameWithoutExtension);
 
         protected override int ExecuteTool(string pathToTool, string responseFileCommands, string commandLineCommands)
         {

src/Compilers/Core/MSBuildTask/ManagedCompiler.cs

@@ -409,6 +409,25 @@ public string LangVersion
 
         #endregion
 
+        internal static DotnetHost CreateDotnetHostInfo(string toolPath, string toolExe, string toolName, string commandLine)
+        {
+            // ToolExe delegates back to ToolName if the override is not
+            // set.  So, if ToolExe == ToolName, we know ToolExe is not
+            // explicitly overriden.  So, if both ToolPath is unset and
+            // ToolExe == ToolName, we know nothing is overridden, and
+            // we can use our own csc.
+            if (string.IsNullOrEmpty(toolPath) && toolExe == toolName)
+            {
+                return DotnetHost.CreateManagedInvocationTool(toolName, commandLine);
+            }
+            else
+            {
+                // Explicitly provided ToolPath or ToolExe, don't try to
+                // figure anything out
+                return DotnetHost.CreateNativeInvocationTool(Path.Combine(toolPath ?? "", toolExe), commandLine);
+            }
+        }
+
         private DotnetHost _dotnetHostInfo;
         private DotnetHost DotnetHostInfo
         {
@@ -420,34 +439,7 @@ private DotnetHost DotnetHostInfo
                     AddCommandLineCommands(commandLineBuilder);
                     var commandLine = commandLineBuilder.ToString();
 
-                    // ToolExe delegates back to ToolName if the override is not
-                    // set.
-                    // So, we can't check if it's unset, as that will recur
-                    // and stackoverflow.
-                    // However, checking only ToolPath is inadequate, as some
-                    // callers only set ToolExe, and not ToolPath (e.g. CLI).
-                    // So, do the check after _dotnetHostInfo is assigned, and
-                    // if ToolExe routes back here and returns
-                    // _dotnetHostInfo.ToolName, we know that ToolExe is unset.
-                    // So, if it does *not* return such, we know ToolExe is
-                    // explicitly overriden - so swap out the DotnetHost with
-                    // the passthrough implementation, as otherwise the command
-                    // line would be incorrect (it would have csc.dll in the
-                    // arguments).
-                    if (string.IsNullOrEmpty(ToolPath))
-                    {
-                        _dotnetHostInfo = DotnetHost.CreateManagedToolInvocation(ToolNameWithoutExtension, commandLine);
-
-                        if (ToolExe != _dotnetHostInfo.ToolNameOpt)
-                        {
-                            _dotnetHostInfo = DotnetHost.CreateUnmanagedToolInvocation(ToolPath, commandLine);
-                        }
-                    }
-                    else
-                    {
-                        // Explicitly provided ToolPath, don't try to figure anything out
-                        _dotnetHostInfo = DotnetHost.CreateUnmanagedToolInvocation(ToolPath, commandLine);
-                    }
+                    _dotnetHostInfo = CreateDotnetHostInfo(ToolPath, ToolExe, ToolName, commandLine);
                 }
                 return _dotnetHostInfo;
             }
@@ -455,7 +447,27 @@ private DotnetHost DotnetHostInfo
 
         protected abstract string ToolNameWithoutExtension { get; }
 
-        protected sealed override string ToolName => DotnetHostInfo.ToolNameOpt;
+        internal static string GenerateToolName(string toolNameWithoutExtension)
+        {
+            if (CoreClrShim.IsRunningOnCoreClr)
+            {
+                return $"{toolNameWithoutExtension}.dll";
+            }
+            else
+            {
+                return $"{toolNameWithoutExtension}.exe";
+            }
+        }
+
+        protected sealed override string ToolName => GenerateToolName(ToolNameWithoutExtension);
+
+        /// <summary>
+        /// Method for testing only
+        /// </summary>
+        public string GeneratePathToTool()
+        {
+            return GenerateFullPathToTool();
+        }
 
         /// <summary>
         /// Return the path to the tool to execute.
@@ -488,7 +500,7 @@ protected override int ExecuteTool(string pathToTool, string responseFileCommand
             try
             {
                 if (!UseSharedCompilation ||
-                !string.IsNullOrEmpty(ToolPath) ||
+                !DotnetHostInfo.IsManagedInvocation ||
                 !BuildServerConnection.IsCompilerServerSupported)
                 {
                     return base.ExecuteTool(pathToTool, responseFileCommands, commandLineCommands);
@@ -707,6 +719,14 @@ protected override string GenerateResponseFileCommands()
             return commandLineBuilder.ToString();
         }
 
+        /// <summary>
+        /// Method for testing only
+        /// </summary>
+        public string GenerateCommandLine()
+        {
+            return GenerateCommandLineCommands();
+        }
+
         protected override string GenerateCommandLineCommands()
         {
             return DotnetHostInfo.CommandLineArgs;

src/Compilers/Core/MSBuildTaskTests/CscTests.cs

@@ -6,6 +6,7 @@
 using Microsoft.CodeAnalysis.BuildTasks;
 using Xunit;
 using Moq;
+using System.IO;
 
 namespace Microsoft.CodeAnalysis.BuildTasks.UnitTests
 {
@@ -334,5 +335,40 @@ public void SharedCompilationId()
             csc.SharedCompilationId = "testPipeName";
             Assert.Equal("/out:test.exe test.cs", csc.GenerateResponseFileContents());
         }
+
+        [Fact]
+        public void EmptyCscToolPath()
+        {
+            var csc = new Csc();
+            csc.ToolPath = "";
+            csc.ToolExe = Path.Combine("path", "to", "custom_csc");
+            csc.Sources = MSBuildUtil.CreateTaskItems("test.cs");
+            Assert.Equal("", csc.GenerateCommandLine());
+            Assert.Equal(Path.Combine("path", "to", "custom_csc"), csc.GeneratePathToTool());
+
+            csc = new Csc();
+            csc.ToolExe = Path.Combine("path", "to", "custom_csc");
+            csc.Sources = MSBuildUtil.CreateTaskItems("test.cs");
+            Assert.Equal("", csc.GenerateCommandLine());
+            Assert.Equal(Path.Combine("path", "to", "custom_csc"), csc.GeneratePathToTool());
+        }
+
+        [Fact]
+        public void EmptyCscToolExe()
+        {
+            var csc = new Csc();
+            csc.ToolPath = Path.Combine("path", "to", "custom_csc");
+            csc.ToolExe = "";
+            csc.Sources = MSBuildUtil.CreateTaskItems("test.cs");
+            Assert.Equal("", csc.GenerateCommandLine());
+            // StartsWith because it can be csc.exe or csc.dll
+            Assert.StartsWith(Path.Combine("path", "to", "custom_csc", "csc."), csc.GeneratePathToTool());
+
+            csc = new Csc();
+            csc.ToolPath = Path.Combine("path", "to", "custom_csc");
+            csc.Sources = MSBuildUtil.CreateTaskItems("test.cs");
+            Assert.Equal("", csc.GenerateCommandLine());
+            Assert.StartsWith(Path.Combine("path", "to", "custom_csc", "csc."), csc.GeneratePathToTool());
+        }
     }
 }