Do not report warning if submodule dir is missing (#1078)

* Do not report warning if submodule dir is missing

* Update test

Author: tmat

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

@@ -534,8 +534,9 @@ public void GetSourceRoots_RepoWithoutCommitsWithSubmodules()
             }, warnings.Select(TestUtilities.InspectDiagnostic));
         }
 
-        [Fact]
-        public void GetSourceRoots_RepoWithCommitsWithSubmodules()
+        [Theory]
+        [CombinatorialData]
+        public void GetSourceRoots_RepoWithCommitsWithSubmodules(bool warnOnMissingCommit)
         {
             var repo = CreateRepository(
                 commitSha: "0000000000000000000000000000000000000000",
@@ -547,16 +548,23 @@ public void GetSourceRoots_RepoWithCommitsWithSubmodules()
                     CreateSubmodule("2", "sub/2", "http://2.com", "2222222222222222222222222222222222222222")));
 
             var warnings = new List<(string, object?[])>();
-            var items = GitOperations.GetSourceRoots(repo, remoteName: null, warnOnMissingCommit: false, (message, args) => warnings.Add((message, args)));
+            var items = GitOperations.GetSourceRoots(repo, remoteName: null, warnOnMissingCommit, (message, args) => warnings.Add((message, args)));
 
             AssertEx.Equal(new[]
             {
                 $@"'{_workingDir}{s}' SourceControl='git' RevisionId='0000000000000000000000000000000000000000'",
                 $@"'{_workingDir}{s}sub{s}2{s}' SourceControl='git' RevisionId='2222222222222222222222222222222222222222' NestedRoot='sub/2/' ContainingRoot='{_workingDir}{s}' ScmRepositoryUrl='http://github.com/sub-2'",
             }, items.Select(TestUtilities.InspectSourceRoot));
 
-            AssertEx.Equal(new[] { string.Format(Resources.SourceCodeWontBeAvailableViaSourceLink, string.Format(Resources.SubmoduleWithoutCommit, "1")) }, 
-                warnings.Select(TestUtilities.InspectDiagnostic));
+            if (warnOnMissingCommit)
+            {
+                AssertEx.Equal(new[] { string.Format(Resources.SourceCodeWontBeAvailableViaSourceLink, string.Format(Resources.SubmoduleWithoutCommit, "1")) },
+                    warnings.Select(TestUtilities.InspectDiagnostic));
+            }
+            else
+            {
+                Assert.Empty(warnings);
+            }
         }
 
         [Fact]

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

@@ -453,13 +453,7 @@ public void Submodules_Errors()
             var repository = new GitRepository(GitEnvironment.Empty, GitConfig.Empty, gitDir.Path, gitDir.Path, workingDir.Path);
 
             var submodules = repository.GetSubmodules();
-            AssertEx.Equal(new[]
-            {
-                "S10: 'sub10' 'http://github.com'",
-                "S11: 'sub11' 'http://github.com'",
-                "S6: 'sub6' 'http://github.com'",
-                "S9: 'sub9' 'http://github.com'"
-            }, submodules.Select(s => $"{s.Name}: '{s.WorkingDirectoryRelativePath}' '{s.Url}'"));
+            Assert.Empty(submodules);
 
             var diagnostics = repository.GetSubmoduleDiagnostics();
             AssertEx.Equal(new[]
@@ -468,12 +462,6 @@ public void Submodules_Errors()
               string.Format(Resources.InvalidSubmodulePath, "S1", "  "),
               // The path of submodule 'S2' is missing or invalid: ''
               string.Format(Resources.InvalidSubmodulePath, "S2", ""),
-              // Could not find a part of the path 'sub3\.git'.
-              TestUtilities.GetExceptionMessage(() => File.ReadAllText(Path.Combine(workingDir.Path, "sub3", ".git"))),
-              // Could not find a part of the path 'sub4\.git'.
-              TestUtilities.GetExceptionMessage(() => File.ReadAllText(Path.Combine(workingDir.Path, "sub4", ".git"))),
-              // Could not find a part of the path 'sub5\.git'.
-              TestUtilities.GetExceptionMessage(() => File.ReadAllText(Path.Combine(workingDir.Path, "sub5", ".git"))),
               // The format of the file 'sub7\.git' is invalid.
               string.Format(Resources.FormatOfFileIsInvalid, Path.Combine(workingDir.Path, "sub7", ".git")),
               // Path specified in file 'sub8\.git' is invalid.
@@ -514,8 +502,8 @@ public void GetSubmoduleHeadCommitSha()
             submoduleRefsHeadsDir.CreateFile("master").WriteAllText("0000000000000000000000000000000000000000");
             submoduleGitDir.CreateFile("HEAD").WriteAllText("ref: refs/heads/master");
 
-            var repository = new GitRepository(GitEnvironment.Empty, GitConfig.Empty, gitDir.Path, gitDir.Path, workingDir.Path);
-            Assert.Equal("0000000000000000000000000000000000000000", repository.ReadSubmoduleHeadCommitSha(submoduleWorkingDir.Path));
+            Assert.Equal("0000000000000000000000000000000000000000",
+                GitRepository.GetSubmoduleReferenceResolver(submoduleWorkingDir.Path)?.ResolveHeadReference());
         }
 
         [Fact]
@@ -534,8 +522,22 @@ public void GetOldStyleSubmoduleHeadCommitSha()
             oldStyleSubmoduleRefsHeadDir.CreateFile("branch1").WriteAllText("1111111111111111111111111111111111111111");
             oldStyleSubmoduleGitDir.CreateFile("HEAD").WriteAllText("ref: refs/heads/branch1");
 
-            var repository = new GitRepository(GitEnvironment.Empty, GitConfig.Empty, gitDir.Path, gitDir.Path, workingDir.Path);
-            Assert.Equal("1111111111111111111111111111111111111111", repository.ReadSubmoduleHeadCommitSha(oldStyleSubmoduleWorkingDir.Path));
+            Assert.Equal("1111111111111111111111111111111111111111",
+                GitRepository.GetSubmoduleReferenceResolver(oldStyleSubmoduleWorkingDir.Path)?.ResolveHeadReference());
+        }
+
+        [Fact]
+        public void GetSubmoduleHeadCommitSha_NoGitFile()
+        {
+            using var temp = new TempRoot();
+
+            var gitDir = temp.CreateDirectory();
+            var workingDir = temp.CreateDirectory();
+
+            var submoduleGitDir = temp.CreateDirectory();
+            var submoduleWorkingDir = workingDir.CreateDirectory("sub").CreateDirectory("abc");
+
+            Assert.Null(GitRepository.GetSubmoduleReferenceResolver(submoduleWorkingDir.Path)?.ResolveHeadReference());
         }
     }
 }

src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs

@@ -182,28 +182,30 @@ public static GitRepository OpenRepository(GitRepositoryLocation location, GitEn
             => _referenceResolver.ResolveHeadReference();
 
         /// <summary>
-        /// Reads and resolves the commit SHA of the current HEAD tip of the specified submodule.
+        /// Creates <see cref="GitReferenceResolver"/> for a submodule located in the specified <paramref name="submoduleWorkingDirectoryFullPath"/>.
         /// </summary>
         /// <exception cref="IOException"/>
         /// <exception cref="InvalidDataException"/>
-        /// <returns>Null if the HEAD tip reference can't be resolved.</returns>
-        internal string? ReadSubmoduleHeadCommitSha(string submoduleWorkingDirectoryFullPath)
+        /// <returns>Null if the submodule can't be located.</returns>
+        public static GitReferenceResolver? GetSubmoduleReferenceResolver(string submoduleWorkingDirectoryFullPath)
         {
             // Submodules don't usually have their own .git directories but this is still legal.
             // This can occur with older versions of Git or other tools, or when a user clones one
             // repo into another's source tree (but it was not yet registered as a submodule).
             // See https://git-scm.com/docs/gitsubmodules#_forms for more details.
             var dotGitPath = Path.Combine(submoduleWorkingDirectoryFullPath, GitDirName);
 
-            var gitDirectory = Directory.Exists(dotGitPath) ? dotGitPath : ReadDotGitFile(dotGitPath);
-            if (!IsGitDirectory(gitDirectory, out var commonDirectory))
+            var gitDirectory =
+                Directory.Exists(dotGitPath) ? dotGitPath :
+                File.Exists(dotGitPath) ? ReadDotGitFile(dotGitPath) : null;
+
+            if (gitDirectory == null || !IsGitDirectory(gitDirectory, out var commonDirectory))
             {
                 return null;
             }
 
-            var resolver = new GitReferenceResolver(gitDirectory, commonDirectory);
-            return resolver.ResolveHeadReference();
-        } 
+            return new GitReferenceResolver(gitDirectory, commonDirectory);
+        }
 
         private string GetWorkingDirectory()
             => WorkingDirectory ?? throw new InvalidOperationException(Resources.RepositoryDoesNotHaveWorkingDirectory);
@@ -263,7 +265,15 @@ void reportDiagnostic(string diagnostic)
                 string? headCommitSha;
                 try
                 {
-                    headCommitSha = ReadSubmoduleHeadCommitSha(fullPath);
+                    var resolver = GetSubmoduleReferenceResolver(fullPath);
+                    if (resolver == null)
+                    {
+                        // If we can't locate the submodule directory then it won't have any source files
+                        // and we can safely ignore the submodule.
+                        continue;
+                    }
+
+                    headCommitSha = resolver.ResolveHeadReference();
                 }
                 catch (Exception e) when (e is IOException or InvalidDataException)
                 {

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

@@ -280,8 +280,11 @@ public static ITaskItem[] GetSourceRoots(GitRepository repository, string? remot
                 var commitSha = submodule.HeadCommitSha;
                 if (commitSha == null)
                 {
-                    logWarning(Resources.SourceCodeWontBeAvailableViaSourceLink, 
-                        new[] { string.Format(Resources.SubmoduleWithoutCommit, new[] { submodule.Name }) });
+                    if (warnOnMissingCommit)
+                    {
+                        logWarning(Resources.SourceCodeWontBeAvailableViaSourceLink,
+                            new[] { string.Format(Resources.SubmoduleWithoutCommit, new[] { submodule.Name }) });
+                    }
 
                     continue;
                 }