diff --git a/src/Microsoft.Build.Tasks.Git.UnitTests/GetUntrackedFilesTests.cs b/src/Microsoft.Build.Tasks.Git.UnitTests/GetUntrackedFilesTests.cs new file mode 100644 index 0000000000..c7c044322e --- /dev/null +++ b/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 + { + /// + /// Verifies historical behavior is preserved: a fully-qualified + /// 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. + /// + [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)); + } + + /// + /// Verifies that a relative (with no + /// ) 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 "." 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 guard or the + /// 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 + /// covers the task guard on the + /// cached path). It documents the observable end-to-end behavior. + /// + [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); + } + } + + /// + /// Verifies the belt-and-suspenders guard in : when the repository is taken + /// from the cache (RepositoryId provided), the base does not validate the + /// initial path, so the task itself must reject a relative + /// 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). + /// + [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; + } + } +} diff --git a/src/Microsoft.Build.Tasks.Git/GetUntrackedFiles.cs b/src/Microsoft.Build.Tasks.Git/GetUntrackedFiles.cs index 0be4a3d6cf..c4407aacfa 100644 --- a/src/Microsoft.Build.Tasks.Git/GetUntrackedFiles.cs +++ b/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 { /// /// Selects files that are under the repository root but ignored. /// + [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); } } diff --git a/src/Microsoft.Build.Tasks.Git/GitOperations.cs b/src/Microsoft.Build.Tasks.Git/GitOperations.cs index fc411c6ba3..2d3ab83902 100644 --- a/src/Microsoft.Build.Tasks.Git/GitOperations.cs +++ b/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);