Migrate Exec task to task environment API

src/Tasks.UnitTests/Exec_Tests.cs

@@ -1068,6 +1068,115 @@ public void ConsoleOutputDoesNotTrimLeadingWhitespace()
                 exec.ConsoleOutput[0].ItemSpec.ShouldBe(lineWithLeadingWhitespace);
             }
         }
+
+        /// <summary>
+        /// Runs an Exec task that lists directory contents and asserts expected/unexpected files in the output.
+        /// </summary>
+        /// <param name="taskEnvironment">The TaskEnvironment to configure on the Exec task.</param>
+        /// <param name="workingDirectory">The WorkingDirectory to set, or null to use the default.</param>
+        /// <param name="expectedFile">A filename that must appear in the output.</param>
+        /// <param name="notExpectedFile">A filename that must NOT appear in the output, or null to skip.</param>
+        private void ExecuteListCommandInDirectory(
+            TaskEnvironment taskEnvironment,
+            string workingDirectory,
+            string expectedFile,
+            string notExpectedFile = null)
+        {
+            Exec exec = new Exec();
+            exec.TaskEnvironment = taskEnvironment;
+            exec.BuildEngine = new MockEngine(_output);
+            exec.Command = NativeMethodsShared.IsWindows ? "dir /b" : "ls";
+            exec.ConsoleToMSBuild = true;
+
+            if (workingDirectory != null)
+            {
+                exec.WorkingDirectory = workingDirectory;
+            }
+
+            bool result = exec.Execute();
+
+            result.ShouldBeTrue();
+            ((MockEngine)exec.BuildEngine).AssertLogContains(expectedFile);
+            if (notExpectedFile != null)
+            {
+                ((MockEngine)exec.BuildEngine).AssertLogDoesntContain(notExpectedFile);
+            }
+        }
+
+        [Fact]
+        public void ExecUsesProjectDirectoryAsDefaultWorkingDirectory()
+        {
+            using (var testEnv = TestEnvironment.Create(_output))
+            {
+                var projectDir = testEnv.CreateFolder();
+                File.WriteAllText(Path.Combine(projectDir.Path, "projectfile.txt"), "project content");
+
+                var differentCwd = testEnv.CreateFolder();
+                File.WriteAllText(Path.Combine(differentCwd.Path, "decoyfile.txt"), "decoy content");
+
+                string originalDirectory = Directory.GetCurrentDirectory();
+                TaskEnvironment taskEnvironment = null;
+                try
+                {
+                    Directory.SetCurrentDirectory(differentCwd.Path);
+
+                    taskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(projectDir.Path);
+                    ExecuteListCommandInDirectory(
+                        taskEnvironment,
+                        workingDirectory: null,
+                        expectedFile: "projectfile.txt",
+                        notExpectedFile: "decoyfile.txt");
+                }
+                finally
+                {
+                    taskEnvironment?.Dispose();
+                    Directory.SetCurrentDirectory(originalDirectory);
+                }
+            }
+        }
+
+        [Fact]
+        public void InvalidRelativeWorkingDirectory_LogsOriginalPathNotAbsolutized()
+        {
+            using var testEnv = TestEnvironment.Create(_output);
+            var projectDir = testEnv.CreateFolder();
+            const string relativeDir = "testDir";
+            string absolutePath = Path.Combine(projectDir.Path, relativeDir);
+
+            var exec = new Exec
+            {
+                BuildEngine = new MockEngine(_output),
+                Command = "echo hi",
+                WorkingDirectory = relativeDir,
+                TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(projectDir.Path),
+            };
+
+            exec.Execute().ShouldBeFalse();
+
+            var engine = (MockEngine)exec.BuildEngine;
+            engine.AssertLogContains("MSB6003");
+            engine.AssertLogContains(relativeDir);
+            engine.AssertLogDoesntContain(absolutePath);
+        }
+
+        [Fact]
+        public void Exec_RelativeWorkingDirectory_ResolvedAgainstProjectDirectory()
+        {
+            using var testEnv = TestEnvironment.Create(_output);
+            var projectDir = testEnv.CreateFolder();
+            Directory.CreateDirectory(Path.Combine(projectDir.Path, "builddir"));
+
+            var exec = new Exec
+            {
+                BuildEngine = new MockEngine(_output),
+                Command = "echo hi",
+                WorkingDirectory = "builddir",
+                TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(projectDir.Path),
+            };
+
+            exec.ValidateParametersAccessor().ShouldBeTrue();
+            exec.GetWorkingDirectoryAccessor().ShouldBe(Path.Combine(projectDir.Path, "builddir"));
+        }
     }
 
     internal sealed class ExecWrapper : Exec

src/Tasks/Exec.cs

@@ -21,6 +21,7 @@ namespace Microsoft.Build.Tasks
     /// for it to complete, and then returns True if the process completed successfully, and False if an error occurred.
     /// </summary>
     // UNDONE: ToolTask has a "UseCommandProcessor" flag that duplicates much of the code in this class. Remove the duplication.
+    [MSBuildMultiThreadableTask]
     public class Exec : ToolTaskExtension
     {
         #region Constructors
@@ -46,7 +47,7 @@ public Exec()
 
         // Are the encodings for StdErr and StdOut streams valid
         private bool _encodingParametersValid = true;
-        private string _workingDirectory;
+        private AbsolutePath _workingDirectory;
         private ITaskItem[] _outputs;
         internal bool workingDirectoryIsUNC; // internal for unit testing
         private string _batchFile;
@@ -196,7 +197,7 @@ public ITaskItem[] Outputs
         /// </summary>
         private void CreateTemporaryBatchFile()
         {
-            var encoding = EncodingUtilities.BatchFileEncoding(Command + WorkingDirectory, UseUtf8Encoding);
+            var encoding = EncodingUtilities.BatchFileEncoding(Command + _workingDirectory.Value, UseUtf8Encoding);
 
             // Temporary file with the extension .Exec.bat
             _batchFile = FileUtilities.GetTemporaryFileName(".exec.cmd");
@@ -244,7 +245,7 @@ private void CreateTemporaryBatchFile()
                     // https://support.microsoft.com/en-us/kb/156276
                     if (workingDirectoryIsUNC)
                     {
-                        sw.WriteLine("pushd " + _workingDirectory);
+                        sw.WriteLine("pushd " + _workingDirectory.Value);
                     }
                 }
                 else
@@ -458,10 +459,12 @@ protected override bool ValidateParameters()
             }
 
             // determine what the working directory for the exec command is going to be -- if the user specified a working
-            // directory use that, otherwise it's the current directory
+            // directory use that, otherwise default to the project directory (TaskEnvironment.ProjectDirectory). Using the
+            // project directory instead of the process current directory is important for correctness in multithreaded (/mt)
+            // builds, where the process working directory may not match the project being built.
             _workingDirectory = !string.IsNullOrEmpty(WorkingDirectory)
-                ? WorkingDirectory
-                : Directory.GetCurrentDirectory();
+                ? TaskEnvironment.GetAbsolutePath(WorkingDirectory)
+                : TaskEnvironment.ProjectDirectory;
 
             // check if the working directory we're going to use for the exec command is a UNC path
             workingDirectoryIsUNC = FileUtilitiesRegex.StartsWithUncPattern(_workingDirectory);
@@ -470,7 +473,10 @@ protected override bool ValidateParameters()
             // will not be able to auto-map to the UNC path
             if (workingDirectoryIsUNC && NativeMethods.AllDrivesMapped())
             {
-                Log.LogErrorWithCodeFromResources("Exec.AllDriveLettersMappedError", _workingDirectory);
+                Log.LogErrorWithCodeFromResources(
+                    "Exec.AllDriveLettersMappedError",
+                    _workingDirectory.OriginalValue,
+                    _workingDirectory.Value);
                 return false;
             }
 
@@ -492,7 +498,8 @@ internal bool ValidateParametersAccessor()
         /// <returns>path to cmd.exe</returns>
         protected override string GenerateFullPathToTool()
         {
-            return CommandProcessorPath.Value;
+            return CommandProcessorPath.Value
+                ?? TaskEnvironment.GetEnvironmentVariable("ComSpec");
         }
 
         private static readonly Lazy<string> CommandProcessorPath = new Lazy<string>(() =>
@@ -506,9 +513,11 @@ protected override string GenerateFullPathToTool()
                 // Work around https://github.com/dotnet/msbuild/issues/2273 and
                 // https://github.com/dotnet/corefx/issues/19110, which result in
                 // a bad path being returned above on Nano Server SKUs of Windows.
+                // Returning null signals GenerateFullPathToTool to consult TaskEnvironment
+                // for the ComSpec environment variable.
                 if (!FileSystems.Default.FileExists(systemCmd))
                 {
-                    return Environment.GetEnvironmentVariable("ComSpec");
+                    return null;
                 }
 #endif
 
@@ -533,7 +542,7 @@ protected override string GetWorkingDirectory()
             // So verify it's valid here.
             if (!FileSystems.Default.DirectoryExists(_workingDirectory))
             {
-                throw new DirectoryNotFoundException(ResourceUtilities.FormatResourceStringStripCodeAndKeyword("Exec.InvalidWorkingDirectory", _workingDirectory));
+                throw new DirectoryNotFoundException(ResourceUtilities.FormatResourceStringStripCodeAndKeyword("Exec.InvalidWorkingDirectory", _workingDirectory.OriginalValue));
             }
 
             if (workingDirectoryIsUNC)

src/Tasks/Resources/Strings.resx

@@ -394,7 +394,7 @@
         If this bucket overflows, pls. contact 'vsppbdev'.
   -->
   <data name="Exec.AllDriveLettersMappedError">
-    <value>MSB3071: All drive letters from A: through Z: are currently in use. Since the working directory "{0}" is a UNC path, the "Exec" task needs a free drive letter to map the UNC path to. Disconnect from one or more shared resources to free up drive letters, or specify a local working directory before attempting this command again.</value>
+    <value>MSB3071: All drive letters from A: through Z: are currently in use. Since the working directory "{0}" (resolved to UNC path "{1}") requires a UNC mapping, the "Exec" task needs a free drive letter to map the UNC path to. Disconnect from one or more shared resources to free up drive letters, or specify a local working directory before attempting this command again.</value>
     <comment>{StrBegin="MSB3071: "}LOCALIZATION: "Exec", "A:", and "Z:" should not be localized.</comment>
   </data>
   <data name="Exec.MissingCommandError">