diff --git a/src/Microsoft.Build.Tasks.Git.UnitTests/GitOperationsTests.cs b/src/Microsoft.Build.Tasks.Git.UnitTests/GitOperationsTests.cs index 23eee87b..115f5809 100644 --- a/src/Microsoft.Build.Tasks.Git.UnitTests/GitOperationsTests.cs +++ b/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,7 +548,7 @@ 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[] { @@ -555,8 +556,15 @@ public void GetSourceRoots_RepoWithCommitsWithSubmodules() $@"'{_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] diff --git a/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs b/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs index db6520be..1f9b3b99 100644 --- a/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs +++ b/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()); } } } diff --git a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs index f1b314c5..9aea9a66 100644 --- a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs +++ b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs @@ -182,12 +182,12 @@ public static GitRepository OpenRepository(GitRepositoryLocation location, GitEn => _referenceResolver.ResolveHeadReference(); /// - /// Reads and resolves the commit SHA of the current HEAD tip of the specified submodule. + /// Creates for a submodule located in the specified . /// /// /// - /// Null if the HEAD tip reference can't be resolved. - internal string? ReadSubmoduleHeadCommitSha(string submoduleWorkingDirectoryFullPath) + /// Null if the submodule can't be located. + 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 @@ -195,15 +195,17 @@ public static GitRepository OpenRepository(GitRepositoryLocation location, GitEn // 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) { diff --git a/src/Microsoft.Build.Tasks.Git/GitOperations.cs b/src/Microsoft.Build.Tasks.Git/GitOperations.cs index c45cd683..0fd5187c 100644 --- a/src/Microsoft.Build.Tasks.Git/GitOperations.cs +++ b/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; }