From 6b88ba1382c9493f8683ee28d069991d8a89ce41 Mon Sep 17 00:00:00 2001 From: Chris Rummel Date: Tue, 4 Aug 2020 20:28:53 -0500 Subject: [PATCH 1/6] Allow using .git directory instead of gitdir redirect in submodules. --- .../GitDataReader/GitRepository.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs index 9f1648f8..de259f85 100644 --- a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs +++ b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs @@ -209,6 +209,15 @@ public static GitRepository OpenRepository(GitRepositoryLocation location, GitEn /// Null if the HEAD tip reference can't be resolved. internal string? ReadSubmoduleHeadCommitSha(string submoduleWorkingDirectoryFullPath) { + // submodules don't usually have their own .git directories but can when Arcade uberclone + // was used. Handle this case first since the other case throws. + var dotGitPath = Path.Combine(submoduleWorkingDirectoryFullPath, GitDirName); + if (IsGitDirectory(dotGitPath, out var directSubmoduleGitDirectory)) + { + var submoduleGitDirResolver = new GitReferenceResolver(directSubmoduleGitDirectory, submoduleWorkingDirectoryFullPath); + return submoduleGitDirResolver.ResolveHeadReference(); + } + var gitDirectory = ReadDotGitFile(Path.Combine(submoduleWorkingDirectoryFullPath, GitDirName)); if (!IsGitDirectory(gitDirectory, out var commonDirectory)) { From d7ad81a0ff1a6d341194c6fd8ef1efd9bce4c14a Mon Sep 17 00:00:00 2001 From: Chris Rummel Date: Tue, 6 Oct 2020 15:11:30 -0500 Subject: [PATCH 2/6] Address review comments and add test for old-style submodule behavior. --- .../GitRepositoryTests.cs | 23 +++++++++++++------ .../GitDataReader/GitRepository.cs | 5 ++-- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs b/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs index 442a8dcc..ec0794ee 100644 --- a/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs +++ b/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs @@ -344,17 +344,26 @@ public void GetSubmoduleHeadCommitSha() var gitDir = temp.CreateDirectory(); var workingDir = temp.CreateDirectory(); - var submoduleGitDir = temp.CreateDirectory(); + var basicSubmoduleGitDir = temp.CreateDirectory(); - var submoduleWorkingDir = workingDir.CreateDirectory("sub").CreateDirectory("abc"); - submoduleWorkingDir.CreateFile(".git").WriteAllText("gitdir: " + submoduleGitDir.Path + "\t \v\f\r\n\n\r"); + var basicSubmoduleWorkingDir = workingDir.CreateDirectory("sub").CreateDirectory("abc"); + basicSubmoduleWorkingDir.CreateFile(".git").WriteAllText("gitdir: " + basicSubmoduleGitDir.Path + "\t \v\f\r\n\n\r"); - var submoduleRefsHeadsDir = submoduleGitDir.CreateDirectory("refs").CreateDirectory("heads"); - submoduleRefsHeadsDir.CreateFile("master").WriteAllText("0000000000000000000000000000000000000000"); - submoduleGitDir.CreateFile("HEAD").WriteAllText("ref: refs/heads/master"); + var basicSubmoduleRefsHeadsDir = basicSubmoduleGitDir.CreateDirectory("refs").CreateDirectory("heads"); + basicSubmoduleRefsHeadsDir.CreateFile("master").WriteAllText("0000000000000000000000000000000000000000"); + basicSubmoduleGitDir.CreateFile("HEAD").WriteAllText("ref: refs/heads/master"); + + // this is a unusual but legal case which can occur with older versions of Git or other tools. + // see https://git-scm.com/docs/gitsubmodules#_forms for more details. + var oldStyleSubmoduleWorkingDir = workingDir.CreateDirectory("old-style-submodule"); + var oldStyleSubmoduleGitDir = oldStyleSubmoduleWorkingDir.CreateDirectory(".git"); + var oldStyleSubmoduleRefsHeadDir = oldStyleSubmoduleGitDir.CreateDirectory("refs").CreateDirectory("heads"); + 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("0000000000000000000000000000000000000000", repository.ReadSubmoduleHeadCommitSha(submoduleWorkingDir.Path)); + Assert.Equal("0000000000000000000000000000000000000000", repository.ReadSubmoduleHeadCommitSha(basicSubmoduleWorkingDir.Path)); + Assert.Equal("1111111111111111111111111111111111111111", repository.ReadSubmoduleHeadCommitSha(oldStyleSubmoduleWorkingDir.Path)); } } } diff --git a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs index de259f85..71462c3d 100644 --- a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs +++ b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs @@ -209,8 +209,9 @@ public static GitRepository OpenRepository(GitRepositoryLocation location, GitEn /// Null if the HEAD tip reference can't be resolved. internal string? ReadSubmoduleHeadCommitSha(string submoduleWorkingDirectoryFullPath) { - // submodules don't usually have their own .git directories but can when Arcade uberclone - // was used. Handle this case first since the other case throws. + // submodules don't usually have their own .git directories but this is still legal. + // see https://git-scm.com/docs/gitsubmodules#_forms for more details. + // Handle this case first since the other case throws. var dotGitPath = Path.Combine(submoduleWorkingDirectoryFullPath, GitDirName); if (IsGitDirectory(dotGitPath, out var directSubmoduleGitDirectory)) { From 3a62f1b49e81539d6520415d8031cde8ead82615 Mon Sep 17 00:00:00 2001 From: Chris Rummel Date: Tue, 6 Oct 2020 17:00:47 -0500 Subject: [PATCH 3/6] Use .git dir as the common directory as well. --- src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs index 71462c3d..9179e927 100644 --- a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs +++ b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs @@ -215,7 +215,7 @@ public static GitRepository OpenRepository(GitRepositoryLocation location, GitEn var dotGitPath = Path.Combine(submoduleWorkingDirectoryFullPath, GitDirName); if (IsGitDirectory(dotGitPath, out var directSubmoduleGitDirectory)) { - var submoduleGitDirResolver = new GitReferenceResolver(directSubmoduleGitDirectory, submoduleWorkingDirectoryFullPath); + var submoduleGitDirResolver = new GitReferenceResolver(directSubmoduleGitDirectory, directSubmoduleGitDirectory); return submoduleGitDirResolver.ResolveHeadReference(); } From 8994c0984c71c9dc17c36d8ddc0dc6b5172c0b55 Mon Sep 17 00:00:00 2001 From: Chris Rummel Date: Tue, 6 Oct 2020 17:11:00 -0500 Subject: [PATCH 4/6] Add more details to the comments about how this case can happen. --- .../GitDataReader/GitRepository.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs index 9179e927..c079197b 100644 --- a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs +++ b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs @@ -209,8 +209,10 @@ public static GitRepository OpenRepository(GitRepositoryLocation location, GitEn /// Null if the HEAD tip reference can't be resolved. internal string? ReadSubmoduleHeadCommitSha(string submoduleWorkingDirectoryFullPath) { - // submodules don't usually have their own .git directories but this is still legal. - // see https://git-scm.com/docs/gitsubmodules#_forms for more details. + // 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. // Handle this case first since the other case throws. var dotGitPath = Path.Combine(submoduleWorkingDirectoryFullPath, GitDirName); if (IsGitDirectory(dotGitPath, out var directSubmoduleGitDirectory)) From 88e7e5ec16a3ee5bb9af2773272926b297cc5fe1 Mon Sep 17 00:00:00 2001 From: Chris Rummel Date: Thu, 5 Aug 2021 15:35:51 -0500 Subject: [PATCH 5/6] Address code review comments. --- .../GitRepositoryTests.cs | 25 +++++++++++++------ .../GitDataReader/GitRepository.cs | 13 +++++----- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs b/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs index ec0794ee..17819b0c 100644 --- a/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs +++ b/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs @@ -344,14 +344,26 @@ public void GetSubmoduleHeadCommitSha() var gitDir = temp.CreateDirectory(); var workingDir = temp.CreateDirectory(); - var basicSubmoduleGitDir = temp.CreateDirectory(); + var submoduleGitDir = temp.CreateDirectory(); - var basicSubmoduleWorkingDir = workingDir.CreateDirectory("sub").CreateDirectory("abc"); - basicSubmoduleWorkingDir.CreateFile(".git").WriteAllText("gitdir: " + basicSubmoduleGitDir.Path + "\t \v\f\r\n\n\r"); + var submoduleWorkingDir = workingDir.CreateDirectory("sub").CreateDirectory("abc"); + submoduleWorkingDir.CreateFile(".git").WriteAllText("gitdir: " + submoduleGitDir.Path + "\t \v\f\r\n\n\r"); - var basicSubmoduleRefsHeadsDir = basicSubmoduleGitDir.CreateDirectory("refs").CreateDirectory("heads"); - basicSubmoduleRefsHeadsDir.CreateFile("master").WriteAllText("0000000000000000000000000000000000000000"); - basicSubmoduleGitDir.CreateFile("HEAD").WriteAllText("ref: refs/heads/master"); + var submoduleRefsHeadsDir = submoduleGitDir.CreateDirectory("refs").CreateDirectory("heads"); + 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)); + } + + [Fact] + public void GetOldStyleSubmoduleHeadCommitSha() + { + using var temp = new TempRoot(); + + var gitDir = temp.CreateDirectory(); + var workingDir = temp.CreateDirectory(); // this is a unusual but legal case which can occur with older versions of Git or other tools. // see https://git-scm.com/docs/gitsubmodules#_forms for more details. @@ -362,7 +374,6 @@ public void GetSubmoduleHeadCommitSha() oldStyleSubmoduleGitDir.CreateFile("HEAD").WriteAllText("ref: refs/heads/branch1"); var repository = new GitRepository(GitEnvironment.Empty, GitConfig.Empty, gitDir.Path, gitDir.Path, workingDir.Path); - Assert.Equal("0000000000000000000000000000000000000000", repository.ReadSubmoduleHeadCommitSha(basicSubmoduleWorkingDir.Path)); Assert.Equal("1111111111111111111111111111111111111111", repository.ReadSubmoduleHeadCommitSha(oldStyleSubmoduleWorkingDir.Path)); } } diff --git a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs index c079197b..d7d0d93a 100644 --- a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs +++ b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs @@ -215,14 +215,13 @@ public static GitRepository OpenRepository(GitRepositoryLocation location, GitEn // See https://git-scm.com/docs/gitsubmodules#_forms for more details. // Handle this case first since the other case throws. var dotGitPath = Path.Combine(submoduleWorkingDirectoryFullPath, GitDirName); - if (IsGitDirectory(dotGitPath, out var directSubmoduleGitDirectory)) - { - var submoduleGitDirResolver = new GitReferenceResolver(directSubmoduleGitDirectory, directSubmoduleGitDirectory); - return submoduleGitDirResolver.ResolveHeadReference(); - } - var gitDirectory = ReadDotGitFile(Path.Combine(submoduleWorkingDirectoryFullPath, GitDirName)); - 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; } From 941e41d8ee200e4b1473798fc09c82d6a149d3ed Mon Sep 17 00:00:00 2001 From: tmat Date: Thu, 12 Aug 2021 09:55:54 -0700 Subject: [PATCH 6/6] Allow empty dir --- .../GitRepositoryTests.cs | 3 +-- .../GitDataReader/GitRepository.cs | 9 ++------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs b/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs index 17819b0c..d8c25e4c 100644 --- a/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs +++ b/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs @@ -295,6 +295,7 @@ public void Submodules_Errors() { "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}'")); @@ -311,8 +312,6 @@ public void Submodules_Errors() 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"))), - // Access to the path 'sub6\.git' is denied - TestUtilities.GetExceptionMessage(() => File.ReadAllText(Path.Combine(workingDir.Path, "sub6", ".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. diff --git a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs index d7d0d93a..532c4feb 100644 --- a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs +++ b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs @@ -213,15 +213,10 @@ public static GitRepository OpenRepository(GitRepositoryLocation location, GitEn // 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. - // Handle this case first since the other case throws. var dotGitPath = Path.Combine(submoduleWorkingDirectoryFullPath, GitDirName); - var gitDirectory = - Directory.Exists(dotGitPath) ? dotGitPath : - File.Exists(dotGitPath) ? ReadDotGitFile(dotGitPath) : - null; - - if (gitDirectory == null || !IsGitDirectory(gitDirectory, out var commonDirectory)) + var gitDirectory = Directory.Exists(dotGitPath) ? dotGitPath : ReadDotGitFile(dotGitPath); + if (!IsGitDirectory(gitDirectory, out var commonDirectory)) { return null; }