Skip to content

Do not report warning if submodule dir is missing #1078

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.

Sign up for GitHub

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
merged 2 commits into from
Aug 2, 2023
Merged

Do not report warning if submodule dir is missing #1078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f545c2d
Do not report warning if submodule dir is missing
tmat Aug 1, 2023
73f58df
Update test
tmat Aug 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions src/Microsoft.Build.Tasks.Git.UnitTests/GitOperationsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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]
Expand Down
36 changes: 19 additions & 17 deletions src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[]
Expand All @@ -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.
Expand Down Expand Up @@ -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]
Expand All @@ -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());
}
}
}
28 changes: 19 additions & 9 deletions src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
{
Expand Down
7 changes: 5 additions & 2 deletions src/Microsoft.Build.Tasks.Git/GitOperations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down