Migrate Tasks.Git.LocateRepository to multithreaded task model #1734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

ViktorHofer merged 10 commits into dotnet:main from jankratochvilcz:mt/locate-repository

Jul 8, 2026

src/Common/Utilities/PathUtilities.cs

-Original file line number
+Diff line change
@@ Expand Up / @@ -50,5 +50,50 @@ public static string EndWithSeparator(this string path) @@
             public static string EndWithSeparator(this string path, char separator)
                 => path.EndsWithSeparator() ? path : path + separator;
+            /// <summary>
+            /// Determines whether <paramref name="path"/> is fully qualified (independent of the process current
+            /// directory and drive). Unlike <see cref="Path.IsPathRooted"/>, rejects Windows drive-relative
+            /// (<c>C:foo</c>) and root-relative (<c>\foo</c>) paths. Polyfills <c>Path.IsPathFullyQualified</c>,
+            /// which is unavailable on .NET Framework.
+            /// </summary>
+            public static bool IsPathFullyQualified(string path)
+            {
+                if (path == null)
+                {
+                    throw new ArgumentNullException(nameof(path));
+                }
+    #if NETFRAMEWORK
+                // Mirrors System.IO.PathInternal.IsPartiallyQualified (Windows).
+                if (path.Length < 2)
+                {
+                    // A single character (or empty) can't be fixed; it is relative.
+                    return false;
+                }
+                if (IsDirectorySeparator(path[0]))
+                {
+                    // A leading separator is only fully qualified for UNC (\\server) or device (\\?\) paths.
+                    return path[1] == '?' || IsDirectorySeparator(path[1]);
+                }
+                // The only other fixed form is drive-qualified: "X:\" or "X:/".
+                return path.Length >= 3
+                    && path[1] == Path.VolumeSeparatorChar
+                    && IsDirectorySeparator(path[2])
+                    && IsValidDriveChar(path[0]);
+    #else
+                return Path.IsPathFullyQualified(path);
+    #endif
+            }
+    #if NETFRAMEWORK
+            private static bool IsDirectorySeparator(char c)
+                => c == Path.DirectorySeparatorChar || c == Path.AltDirectorySeparatorChar;
+            private static bool IsValidDriveChar(char c)
+                => (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z');
+    #endif
         }
     }

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

-Original file line number
+Diff line change
@@ -0,0 +1,10 @@
+    // 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 Xunit;
+    // Some tests in this assembly temporarily change the process current working directory to validate that
+    // repository discovery resolves paths against the task's project directory rather than the process CWD.
+    // Disable test parallelization in this assembly so that the CWD mutation can't race with other tests.
+    [assembly: CollectionBehavior(DisableTestParallelization = true)]

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

-Original file line number
+Diff line change
@@ -0,0 +1,68 @@
+    // 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 TestUtilities;
+    using Xunit;
+    namespace Microsoft.Build.Tasks.Git.UnitTests
+    {
+        public class LocateRepositoryTests
+        {
+            /// <summary>
+            /// Verifies historical behavior is preserved: a fully-qualified <see cref="LocateRepository.Path"/>
+            /// pointing at a git repository is located and its outputs are populated.
+            /// </summary>
+            [Fact]
+            public void AbsolutePath_LocatesRepository()
+            {
+                using var temp = new TempRoot();
+                var (repoDir, gitDir) = CreateMinimalRepository(temp);
+                var engine = new MockEngine();
+                var task = new LocateRepository
+                {
+                    BuildEngine = engine,
+                    NoWarnOnMissingInfo = true,
+                    Path = repoDir.Path,
+                };
+                Assert.True(task.Execute(), engine.Log);
+                Assert.Equal(repoDir.Path, task.WorkingDirectory);
+                Assert.Equal(gitDir.Path, task.RepositoryId);
+            }
+            /// <summary>
+            /// Verifies that a non-fully-qualified (here: relative) <see cref="LocateRepository.Path"/> is rejected.
+            ///
+            /// This is the multithreaded (MT) task model behavior: the process CWD is shared across projects, so it
+            /// must never influence repository discovery. The migrated task requires a fully-qualified path; a relative
+            /// path (<c>"."</c>) is rejected and reported as the "missing repository" warning instead of being resolved
+            /// against the process current working directory.
+            /// </summary>
+            [Fact]
+            public void RelativePath_IsRejected()
+            {
+                var engine = new MockEngine();
+                var task = new LocateRepository
+                {
+                    BuildEngine = engine,
+                    Path = ".",
+                };
+                // Not an error: repository discovery degrades to a warning.
+                Assert.True(task.Execute(), engine.Log);
+                Assert.Null(task.WorkingDirectory);
+                Assert.Contains("WARNING", engine.Log);
+            }
+            private static (TempDirectory repoDir, TempDirectory gitDir) CreateMinimalRepository(TempRoot temp)
+            {
+                var repoDir = temp.CreateDirectory();
+                var gitDir = repoDir.CreateDirectory(".git");
+                gitDir.CreateFile("HEAD").WriteAllText("ref: refs/heads/main\n");
+                gitDir.CreateFile("config").WriteAllText("");
+                return (repoDir, gitDir);
+            }
+        }
+    }

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

-Original file line number
+Diff line change
@@ -0,0 +1,60 @@
+    // 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;
+    using Microsoft.Build.Tasks.SourceControl;
+    using TestUtilities;
+    using Xunit;
+    namespace Microsoft.Build.Tasks.Git.UnitTests
+    {
+        public class PathUtilitiesTests
+        {
+            [ConditionalTheory(typeof(WindowsOnly))]
+            [InlineData(@"C:\")]
+            [InlineData(@"C:\repo")]
+            [InlineData(@"C:/repo")]
+            [InlineData(@"c:\repo")]
+            [InlineData(@"Z:\a\b")]
+            [InlineData(@"\\server\share")]
+            [InlineData(@"//server/share")]
+            [InlineData(@"\\?\C:\repo")]
+            public void IsPathFullyQualified_Windows_True(string path)
+                => Assert.True(PathUtilities.IsPathFullyQualified(path));
+            [ConditionalTheory(typeof(WindowsOnly))]
+            [InlineData(@"C:foo")]  // drive-relative: depends on the current directory of drive C
+            [InlineData(@"\foo")]   // root-relative: depends on the current drive
+            [InlineData(@"/foo")]   // root-relative (alt separator)
+            [InlineData(@"C:")]     // drive with no path
+            [InlineData(@"1:\foo")] // invalid drive character
+            public void IsPathFullyQualified_Windows_False(string path)
+                => Assert.False(PathUtilities.IsPathFullyQualified(path));
+            [ConditionalTheory(typeof(UnixOnly))]
+            [InlineData(@"/")]
+            [InlineData(@"/foo")]
+            [InlineData(@"/foo/bar")]
+            public void IsPathFullyQualified_Unix_True(string path)
+                => Assert.True(PathUtilities.IsPathFullyQualified(path));
+            [ConditionalTheory(typeof(UnixOnly))]
+            [InlineData(@"C:\foo")] // not a special form on Unix
+            [InlineData(@"foo")]
+            public void IsPathFullyQualified_Unix_False(string path)
+                => Assert.False(PathUtilities.IsPathFullyQualified(path));
+            [Theory]
+            [InlineData("")]
+            [InlineData("foo")]
+            [InlineData("foo/bar")]
+            [InlineData(".")]
+            [InlineData("..")]
+            public void IsPathFullyQualified_Relative_False(string path)
+                => Assert.False(PathUtilities.IsPathFullyQualified(path));
+            [Fact]
+            public void IsPathFullyQualified_Null_Throws()
+                => Assert.Throws<ArgumentNullException>(() => PathUtilities.IsPathFullyQualified(null!));
+        }
+    }

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

-Original file line number
+Diff line change
@@ Expand Up / @@ -8,6 +8,7 @@ @@
     namespace Microsoft.Build.Tasks.Git
     {
+        [MSBuildMultiThreadableTask]
         public sealed class LocateRepository : RepositoryTask
         {
             public string? RemoteName { get; set; }
@@ Expand Down @@

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

-Original file line number
+Diff line change
@@ Expand Up / @@ -6,6 +6,7 @@ @@
     using System.Diagnostics.CodeAnalysis;
     using System.IO;
     using Microsoft.Build.Framework;
+    using Microsoft.Build.Tasks.SourceControl;
     using Microsoft.Build.Utilities;
     namespace Microsoft.Build.Tasks.Git
@@ Expand Down Expand Up / @@ -114,6 +115,14 @@ private void ExecuteImpl() @@
                 var initialPath = GetInitialPath();
+                // Repository discovery must not depend on the shared process CWD/drive under the multithreaded task
+                // model, so reject a non-fully-qualified path with the standard "missing repository" warning.
+                if (string.IsNullOrEmpty(initialPath) || !PathUtilities.IsPathFullyQualified(initialPath))
+                {
+                    ReportMissingRepositoryWarning(initialPath);
+                    return null;
+                }
                 if (!GitRepository.TryFindRepository(initialPath, out var location))
                 {
                     ReportMissingRepositoryWarning(initialPath);
@@ Expand Down @@

src/TestUtilities/MockEngine.cs

-Original file line number
+Diff line change
@@ Expand Up / @@ -4,16 +4,19 @@ @@
     using System;
     using System.Collections;
+    using System.Collections.Generic;
     using System.Text;
     using Microsoft.Build.Framework;
     namespace TestUtilities
     {
-        public sealed class MockEngine : IBuildEngine
+        public sealed class MockEngine : IBuildEngine4
         {
             private StringBuilder _log = new StringBuilder();
             public MessageImportance MinimumMessageImportance = MessageImportance.Low;
+            private readonly Dictionary<object, object?> _registeredTaskObjects = new();
             public string Log
             {
                 set { _log = new StringBuilder(value); }
@@ Expand Down Expand Up / @@ -55,5 +58,40 @@ public void LogMessageEvent(BuildMessageEventArgs eventArgs) @@
             public bool BuildProjectFile(string projectFileName, string[] targetNames, IDictionary globalProperties, IDictionary targetOutputs)
                 => throw new NotImplementedException();
+            // IBuildEngine2
+            public bool IsRunningMultipleNodes => false;
+            public bool BuildProjectFile(string projectFileName, string[] targetNames, IDictionary globalProperties, IDictionary targetOutputs, string toolsVersion)
+                => throw new NotImplementedException();
+            public bool BuildProjectFilesInParallel(string[] projectFileNames, string[] targetNames, IDictionary[] globalProperties, IDictionary[] targetOutputsPerProject, string[] toolsVersion, bool useResultsCache, bool unloadProjectsOnCompletion)
+                => throw new NotImplementedException();
+            // IBuildEngine3
+            public BuildEngineResult BuildProjectFilesInParallel(string[] projectFileNames, string[] targetNames, IDictionary[] globalProperties, IList<string>[] removeGlobalProperties, string[] toolsVersion, bool returnTargetOutputs)
+                => throw new NotImplementedException();
+            public void Yield() { }
+            public void Reacquire() { }
+            // IBuildEngine4
+            public void RegisterTaskObject(object key, object? obj, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection)
+                => _registeredTaskObjects[key] = obj;
+            public object? GetRegisteredTaskObject(object key, RegisteredTaskObjectLifetime lifetime)
+                => _registeredTaskObjects.TryGetValue(key, out var value) ? value : null;
+            public object? UnregisterTaskObject(object key, RegisteredTaskObjectLifetime lifetime)
+            {
+                if (_registeredTaskObjects.TryGetValue(key, out var value))
+                {
+                    _registeredTaskObjects.Remove(key);
+                    return value;
+                }
+                return null;
+            }
         }
     }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Migrate Tasks.Git.LocateRepository to multithreaded task model #1734

Uh oh!

Diff view

Diff view

There are no files selected for viewing

tmat Jul 8, 2026

Uh oh!

Uh oh!

Migrate Tasks.Git.LocateRepository to multithreaded task model #1734

Uh oh!

Migrate Tasks.Git.LocateRepository to multithreaded task model #1734

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

tmat Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!