Migrate Tasks.Git.GetUntrackedFiles to multithreaded task model

src/Microsoft.Build.Tasks.Git.UnitTests/GetUntrackedFilesTests.cs

@@ -0,0 +1,149 @@
+// 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.txt file in the project root for more information.
+
+using System.IO;
+using System.Linq;
+using Microsoft.Build.Framework;
+using TestUtilities;
+using Xunit;
+
+namespace Microsoft.Build.Tasks.Git.UnitTests
+{
+    public class GetUntrackedFilesTests
+    {
+        /// <summary>
+        /// Verifies historical behavior is preserved: a fully-qualified <see cref="GetUntrackedFiles.ProjectDirectory"/>
+        /// is used as the base for resolving the relative file item specs, and only the file ignored by the
+        /// repository is reported as untracked. The original (relative) ItemSpec is preserved on the output item.
+        /// </summary>
+        [Fact]
+        public void AbsoluteProjectDirectory_ReportsIgnoredFileAsUntracked()
+        {
+            using var temp = new TempRoot();
+            var repoDir = CreateRepository(temp);
+
+            var engine = new MockEngine();
+            var task = new GetUntrackedFiles
+            {
+                BuildEngine = engine,
+                ConfigurationScope = "local",
+                ProjectDirectory = repoDir.Path,
+                Files = new ITaskItem[]
+                {
+                    new MockItem("ignored_file.cs"),
+                    new MockItem("included_file.cs"),
+                },
+            };
+
+            Assert.True(task.Execute());
+            Assert.DoesNotContain("ERROR", engine.Log);
+            Assert.NotNull(task.UntrackedFiles);
+            AssertEx.Equal(
+                new[] { MockItem.AdjustSeparators("ignored_file.cs") },
+                task.UntrackedFiles.Select(item => item.ItemSpec));
+        }
+
+        /// <summary>
+        /// Verifies that a relative <see cref="GetUntrackedFiles.ProjectDirectory"/> (with no
+        /// <see cref="GetUntrackedFiles.RepositoryId"/>) is rejected end-to-end rather than resolved against the
+        /// process current working directory. The CWD is pointed at a real git repository; a CWD-dependent
+        /// (pre-migration) implementation would resolve <c>"."</c> against it, locate the repository, populate the
+        /// output, and log no warning. The migrated code rejects the relative path and reports the "missing
+        /// repository" warning instead.
+        ///
+        /// Note: on this path (RepositoryId not set) either the base <see cref="RepositoryTask"/> guard or the
+        /// <see cref="GetUntrackedFiles"/> guard suffices, so this test does not isolate the base guard on its own
+        /// (the sibling LocateRepository PR covers the base guard in isolation, and
+        /// <see cref="RelativeProjectDirectory_OnCachedRepository_IsRejected"/> covers the task guard on the
+        /// cached path). It documents the observable end-to-end behavior.
+        /// </summary>
+        [Fact]
+        public void RelativeProjectDirectory_IsRejected_IndependentOfProcessCwd()
+        {
+            using var temp = new TempRoot();
+            var repoDir = CreateRepository(temp);
+
+            var originalCurrentDirectory = Directory.GetCurrentDirectory();
+            try
+            {
+                Directory.SetCurrentDirectory(repoDir.Path);
+
+                var engine = new MockEngine();
+                var task = new GetUntrackedFiles
+                {
+                    BuildEngine = engine,
+                    ConfigurationScope = "local",
+                    ProjectDirectory = ".",
+                    Files = new ITaskItem[] { new MockItem("ignored_file.cs") },
+                };
+
+                Assert.True(task.Execute());
+                Assert.DoesNotContain("ERROR", engine.Log);
+                Assert.Contains("WARNING", engine.Log);
+                Assert.Null(task.UntrackedFiles);
+            }
+            finally
+            {
+                Directory.SetCurrentDirectory(originalCurrentDirectory);
+            }
+        }
+
+        /// <summary>
+        /// Verifies the belt-and-suspenders guard in <see cref="GetUntrackedFiles"/>: when the repository is taken
+        /// from the cache (RepositoryId provided), the base <see cref="RepositoryTask"/> does not validate the
+        /// initial path, so the task itself must reject a relative <see cref="GetUntrackedFiles.ProjectDirectory"/>
+        /// before using it to resolve the file item specs. A LocateRepository run primes the shared cache, then a
+        /// GetUntrackedFiles run reuses it with a relative project directory. Because ProjectDirectory is expected
+        /// to be absolute on this path, a non-absolute value is reported as an error (Execute returns false).
+        /// </summary>
+        [Fact]
+        public void RelativeProjectDirectory_OnCachedRepository_IsRejected()
+        {
+            using var temp = new TempRoot();
+            var repoDir = CreateRepository(temp);
+
+            var engine = new MockEngine();
+
+            // Prime the repository cache and obtain the RepositoryId used as the cache key.
+            var locate = new LocateRepository
+            {
+                BuildEngine = engine,
+                ConfigurationScope = "local",
+                NoWarnOnMissingInfo = true,
+                Path = repoDir.Path,
+            };
+            Assert.True(locate.Execute());
+            Assert.NotNull(locate.RepositoryId);
+
+            var task = new GetUntrackedFiles
+            {
+                BuildEngine = engine,
+                ConfigurationScope = "local",
+                RepositoryId = locate.RepositoryId,
+                ProjectDirectory = ".",
+                Files = new ITaskItem[] { new MockItem("ignored_file.cs") },
+            };
+
+            Assert.False(task.Execute());
+            Assert.Contains("ERROR", engine.Log);
+            Assert.Null(task.UntrackedFiles);
+        }
+
+        private static TempDirectory CreateRepository(TempRoot temp)
+        {
+            // A real on-disk git repository that ignores 'ignored_file.cs' but not 'included_file.cs'.
+            var repoDir = temp.CreateDirectory();
+            var gitDir = repoDir.CreateDirectory(".git");
+            gitDir.CreateFile("HEAD").WriteAllText("ref: refs/heads/master");
+            gitDir.CreateFile("config").WriteAllText("");
+            gitDir.CreateDirectory("objects");
+            gitDir.CreateDirectory("refs").CreateDirectory("heads").CreateFile("master").WriteAllText("0000000000000000000000000000000000000000");
+
+            repoDir.CreateFile(".gitignore").WriteAllText("ignored_file.cs\n");
+            repoDir.CreateFile("ignored_file.cs").WriteAllText("");
+            repoDir.CreateFile("included_file.cs").WriteAllText("");
+            return repoDir;
+        }
+    }
+}

src/Microsoft.Build.Tasks.Git/GetUntrackedFiles.cs

@@ -4,12 +4,14 @@
 
 using System.Diagnostics.CodeAnalysis;
 using Microsoft.Build.Framework;
+using Microsoft.Build.Tasks.SourceControl;
 
 namespace Microsoft.Build.Tasks.Git
 {
     /// <summary>
     /// Selects files that are under the repository root but ignored.
     /// </summary>
+    [MSBuildMultiThreadableTask]
     public sealed class GetUntrackedFiles : RepositoryTask
     {
         public string? RepositoryId { get; set; }
@@ -28,6 +30,19 @@ public sealed class GetUntrackedFiles : RepositoryTask
 
         private protected override void Execute(GitRepository repository)
         {
+            // ProjectDirectory is the base for resolving the (relative) file ItemSpecs below, so it must be
+            // fully qualified: a relative base would be resolved against the shared process current working
+            // directory, which is unsafe under the multithreaded task model. Reaching this method with a
+            // non-fully-qualified ProjectDirectory is only possible on the cached-repository path (RepositoryId
+            // set), where the base RepositoryTask serves the repository from the cache without validating the
+            // initial path. ProjectDirectory is expected to be absolute ($(MSBuildProjectDirectory) in the
+            // shipped targets), so a non-absolute value is an error rather than a "missing repository" condition.
+            if (!PathUtilities.IsPathFullyQualified(ProjectDirectory))
+            {
+                Log.LogError(Resources.PathMustBeAbsolute);
+                return;
+            }
+
             UntrackedFiles = GitOperations.GetUntrackedFiles(repository, Files, ProjectDirectory);
         }
     }

src/Microsoft.Build.Tasks.Git/GitOperations.cs

@@ -350,7 +350,11 @@ internal static ITaskItem[] GetUntrackedFiles(GitRepository repository, ITaskIte
 
             return files.Where(file =>
             {
-                // file.ItemSpec are relative to projectDirectory.
+                // file.ItemSpec values are relative to projectDirectory, which callers pass as a fully-qualified
+                // path (GetUntrackedFiles rejects a non-fully-qualified ProjectDirectory). For relative or
+                // absolute item specs the inner Path.GetFullPath only canonicalizes an already-rooted path and is
+                // MT-safe. Drive-/root-relative item specs (never produced by @(Compile)) would bypass the base
+                // via Path.Combine and remain CWD/drive-dependent, but that is out of contract.
                 var fullPath = Path.GetFullPath(Path.Combine(projectDirectory, file.ItemSpec));
 
                 var containingDirectoryMatcher = GetContainingRepositoryMatcher(fullPath, directoryTree);