diff --git a/docs/README.md b/docs/README.md index 9487e869..49758ff9 100644 --- a/docs/README.md +++ b/docs/README.md @@ -220,14 +220,18 @@ for Source Link to operate properly. configuration file that specifies `origin` remote URL (e.g. `[remote "origin"] url="http://server.com/repo"`) -If the repository has submodules the file `.gitmodules` must be present in the repository root and must list the URLs and -relative paths of all submodules. - -For example, +If the repository has submodules the file `.gitmodules` must be present in the repository root and must list the +relative paths of all submodules: ``` [submodule "submodule-name"] path = submodule-path +``` + +The `.git/config` file must contain URLs of all initialized submodules: + +``` +[submodule "submodule-name"] url = https://server.com/subrepo ``` diff --git a/src/Microsoft.Build.Tasks.Git.UnitTests/GitDataTests.cs b/src/Microsoft.Build.Tasks.Git.UnitTests/GitDataTests.cs index f179ae64..809ef4ea 100644 --- a/src/Microsoft.Build.Tasks.Git.UnitTests/GitDataTests.cs +++ b/src/Microsoft.Build.Tasks.Git.UnitTests/GitDataTests.cs @@ -20,7 +20,12 @@ public void MinimalGitData() var gitDir = repoDir.CreateDirectory(".git"); gitDir.CreateFile("HEAD").WriteAllText("1111111111111111111111111111111111111111"); - gitDir.CreateFile("config").WriteAllText(@"[remote ""origin""]url=""http://github.com/test-org/test-repo"""); + gitDir.CreateFile("config").WriteAllText(@" +[remote ""origin""] +url=http://github.com/test-org/test-repo +[submodule ""my submodule""] +url=https://github.com/test-org/test-sub +"); gitDir.CreateDirectory("objects"); gitDir.CreateDirectory("refs"); repoDir.CreateFile(".gitignore").WriteAllText("ignore_this_*"); @@ -29,7 +34,7 @@ public void MinimalGitData() var gitModules = repoDir.CreateFile(".gitmodules").WriteAllText(@" [submodule ""my submodule""] path = sub - url = https://github.com/test-org/test-sub + url = xyz "); var subDir = repoDir.CreateDirectory("sub"); diff --git a/src/Microsoft.Build.Tasks.Git.UnitTests/GitOperationsTests.cs b/src/Microsoft.Build.Tasks.Git.UnitTests/GitOperationsTests.cs index eaef98e8..d4a25ca0 100644 --- a/src/Microsoft.Build.Tasks.Git.UnitTests/GitOperationsTests.cs +++ b/src/Microsoft.Build.Tasks.Git.UnitTests/GitOperationsTests.cs @@ -9,6 +9,8 @@ namespace Microsoft.Build.Tasks.Git.UnitTests { + using static KeyValuePairUtils; + public class GitOperationsTests { private static readonly bool IsUnix = Path.DirectorySeparatorChar == '/'; @@ -64,11 +66,11 @@ private static GitVariableName CreateVariableName(string str) private static GitConfig CreateConfig(params (string Name, string Value)[] variables) => new GitConfig(ImmutableDictionary.CreateRange( - variables.Select(v => new KeyValuePair>(CreateVariableName(v.Name), ImmutableArray.Create(v.Value))))); + variables.Select(v => KVP(CreateVariableName(v.Name), ImmutableArray.Create(v.Value))))); private static GitConfig CreateConfig(params (string Name, string[] Values)[] variables) => new GitConfig(ImmutableDictionary.CreateRange( - variables.Select(v => new KeyValuePair>(CreateVariableName(v.Name), ImmutableArray.CreateRange(v.Values))))); + variables.Select(v => KVP(CreateVariableName(v.Name), ImmutableArray.CreateRange(v.Values))))); [Fact] public void GetRepositoryUrl_NoRemotes() @@ -179,8 +181,8 @@ public void GetRepositoryUrl_InsteadOf() { var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[] { - new KeyValuePair>(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create("http://?")), - new KeyValuePair>(new GitVariableName("url", "git@github.com:org/repo", "insteadOf"), ImmutableArray.Create("http://?")) + KVP(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create("http://?")), + KVP(new GitVariableName("url", "git@github.com:org/repo", "insteadOf"), ImmutableArray.Create("http://?")) }))); var warnings = new List<(string, object?[])>(); @@ -206,7 +208,7 @@ public void GetRepositoryUrl_Local(bool useFileUrl) var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[] { - new KeyValuePair>(new GitVariableName("remote", "origin", "url"), + KVP(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create(useFileUrl ? new Uri(mainWorkingDir.Path).AbsolutePath : mainWorkingDir.Path)), }))); @@ -231,7 +233,7 @@ public void GetRepositoryUrl_Local_CustomRemoteName() var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[] { - new KeyValuePair>(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create(mainWorkingDir.Path)), + KVP(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create(mainWorkingDir.Path)), }))); var warnings = new List<(string, object?[])>(); @@ -253,7 +255,7 @@ public void GetRepositoryUrl_Local_NoRemoteOriginUrl() var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[] { - new KeyValuePair>(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create(mainWorkingDir.Path)), + KVP(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create(mainWorkingDir.Path)), }))); var warnings = new List<(string, object?[])>(); @@ -281,7 +283,7 @@ public void GetRepositoryUrl_Local_NoWorkingDirectory() var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[] { - new KeyValuePair>(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create(gitDir2.Path)), + KVP(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create(gitDir2.Path)), }))); var warnings = new List<(string, object?[])>(); @@ -302,7 +304,7 @@ public void GetRepositoryUrl_Local_BadRepo() var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[] { - new KeyValuePair>(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create(mainWorkingDir.Path)), + KVP(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create(mainWorkingDir.Path)), }))); var warnings = new List<(string, object?[])>(); @@ -322,7 +324,7 @@ public void GetRepositoryUrl_LocalRecursion() var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[] { - new KeyValuePair>(new GitVariableName("remote", "origin", "url"), + KVP(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create(mainWorkingDir.Path)), }))); @@ -461,21 +463,37 @@ public void GetSourceRoots_RepoWithoutCommitsWithSubmodules() { var repo = CreateRepository( commitSha: null, - config: CreateConfig(("url.ssh://.insteadOf", "http://")), + config: CreateConfig( + ("url.ssh://.insteadOf", "http://"), + ("submodule.sub1.url", "http://github.com/sub-1"), + ("submodule.sub3.url", "https://github.com/sub-3"), + ("submodule.sub4.url", "https:///"), + ("submodule.sub6.url", "https://github.com/sub-6")), submodules: ImmutableArray.Create( - CreateSubmodule("1", "sub/1", "http://1.com", "1111111111111111111111111111111111111111"), - CreateSubmodule("1", "sub/2", "http://2.com", "2222222222222222222222222222222222222222"))); ; + CreateSubmodule("sub1", "sub/1", "http://1.com", "1111111111111111111111111111111111111111"), + CreateSubmodule("sub2", "sub/2", "http://2.com", "2222222222222222222222222222222222222222"), + CreateSubmodule("sub3", "sub/3", "http://3.com", "3333333333333333333333333333333333333333"), + CreateSubmodule("sub4", "sub/4", "http://4.com", "4444444444444444444444444444444444444444"), + CreateSubmodule("sub5", "sub/5", "http:///", "5555555555555555555555555555555555555555"), + CreateSubmodule("sub6", "sub/6", "", "6666666666666666666666666666666666666666"))); var warnings = new List<(string, object?[])>(); var items = GitOperations.GetSourceRoots(repo, remoteName: null, (message, args) => warnings.Add((message, args))); + // Module without a configuration entry is not initialized. + // URLs listed in .submodules are ignored (they are used by git submodule initialize to generate URLs stored in config). AssertEx.Equal(new[] { - $@"'{_workingDir}{s}sub{s}1{s}' SourceControl='git' RevisionId='1111111111111111111111111111111111111111' NestedRoot='sub/1/' ContainingRoot='{_workingDir}{s}' ScmRepositoryUrl='ssh://1.com/'", - $@"'{_workingDir}{s}sub{s}2{s}' SourceControl='git' RevisionId='2222222222222222222222222222222222222222' NestedRoot='sub/2/' ContainingRoot='{_workingDir}{s}' ScmRepositoryUrl='ssh://2.com/'", + $@"'{_workingDir}{s}sub{s}1{s}' SourceControl='git' RevisionId='1111111111111111111111111111111111111111' NestedRoot='sub/1/' ContainingRoot='{_workingDir}{s}' ScmRepositoryUrl='ssh://github.com/sub-1'", + $@"'{_workingDir}{s}sub{s}3{s}' SourceControl='git' RevisionId='3333333333333333333333333333333333333333' NestedRoot='sub/3/' ContainingRoot='{_workingDir}{s}' ScmRepositoryUrl='https://github.com/sub-3'", + $@"'{_workingDir}{s}sub{s}6{s}' SourceControl='git' RevisionId='6666666666666666666666666666666666666666' NestedRoot='sub/6/' ContainingRoot='{_workingDir}{s}' ScmRepositoryUrl='https://github.com/sub-6'", }, items.Select(TestUtilities.InspectSourceRoot)); - AssertEx.Equal(new[] { Resources.RepositoryHasNoCommit }, warnings.Select(TestUtilities.InspectDiagnostic)); + AssertEx.Equal(new[] + { + Resources.RepositoryHasNoCommit, + string.Format(Resources.SourceCodeWontBeAvailableViaSourceLink, string.Format(Resources.InvalidSubmoduleUrl, "sub4", "https:///")) + }, warnings.Select(TestUtilities.InspectDiagnostic)); ; } [Fact] @@ -483,9 +501,12 @@ public void GetSourceRoots_RepoWithCommitsWithSubmodules() { var repo = CreateRepository( commitSha: "0000000000000000000000000000000000000000", + config: CreateConfig( + ("submodule.1.url", "http://github.com/sub-1"), + ("submodule.2.url", "http://github.com/sub-2")), submodules: ImmutableArray.Create( CreateSubmodule("1", "sub/1", "http://1.com", headCommitSha: null), - CreateSubmodule("1", "sub/2", "http://2.com", "2222222222222222222222222222222222222222"))); + CreateSubmodule("2", "sub/2", "http://2.com", "2222222222222222222222222222222222222222"))); var warnings = new List<(string, object?[])>(); var items = GitOperations.GetSourceRoots(repo, remoteName: null, (message, args) => warnings.Add((message, args))); @@ -493,7 +514,7 @@ public void GetSourceRoots_RepoWithCommitsWithSubmodules() 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://2.com/'", + $@"'{_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")) }, @@ -501,7 +522,7 @@ public void GetSourceRoots_RepoWithCommitsWithSubmodules() } [Fact] - public void GetSourceRoots_RelativeSubmodulePaths() + public void GetSourceRoots_RelativeSubmodulePath() { using var temp = new TempRoot(); @@ -518,8 +539,10 @@ public void GetSourceRoots_RelativeSubmodulePaths() var repo = CreateRepository( workingDir: repoDir.Path, commitSha: "0000000000000000000000000000000000000000", + config: CreateConfig( + ("submodule.1.url", "../1")), submodules: ImmutableArray.Create( - CreateSubmodule("1", "sub/1", "../1", "1111111111111111111111111111111111111111", containingRepositoryWorkingDir: repoDir.Path))); + CreateSubmodule("1", "sub/1", "---", "1111111111111111111111111111111111111111", containingRepositoryWorkingDir: repoDir.Path))); var warnings = new List<(string, object?[])>(); var items = GitOperations.GetSourceRoots(repo, remoteName: null, (message, args) => warnings.Add((message, args))); @@ -532,32 +555,8 @@ public void GetSourceRoots_RelativeSubmodulePaths() Assert.Empty(warnings); } - - [Fact] - public void GetSourceRoots_InvalidSubmoduleUrl() - { - var repo = CreateRepository( - commitSha: "0000000000000000000000000000000000000000", - submodules: ImmutableArray.Create( - CreateSubmodule("1", "sub/1", "http:///", "1111111111111111111111111111111111111111"), - CreateSubmodule("3", "sub/3", "http://3.com", "3333333333333333333333333333333333333333"))); - - var warnings = new List<(string, object?[])>(); - var items = GitOperations.GetSourceRoots(repo, remoteName: null, (message, args) => warnings.Add((message, args))); - - AssertEx.Equal(new[] - { - $@"'{s_root}{s}' SourceControl='git' RevisionId='0000000000000000000000000000000000000000'", - $@"'{s_root}{s}sub{s}3{s}' SourceControl='git' RevisionId='3333333333333333333333333333333333333333' NestedRoot='sub/3/' ContainingRoot='{s_root}{s}' ScmRepositoryUrl='http://3.com/'", - }, items.Select(TestUtilities.InspectSourceRoot)); - - AssertEx.Equal(new[] - { - string.Format(Resources.SourceCodeWontBeAvailableViaSourceLink, string.Format(Resources.InvalidSubmoduleUrl, "1", "http:///")), - }, warnings.Select(TestUtilities.InspectDiagnostic)); - } - - private static GitOperations.DirectoryNode CreateNode(string name, string? submoduleWorkingDirectory, List? children = null) + + private static GitOperations.DirectoryNode CreateNode(string name, string? submoduleWorkingDirectory, List? children = null) => new GitOperations.DirectoryNode(name, children ?? new List()) { Matcher = (submoduleWorkingDirectory != null) ? new Lazy(() => diff --git a/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs b/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs index 8d611bb4..442a8dcc 100644 --- a/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs +++ b/src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs @@ -307,8 +307,8 @@ public void Submodules_Errors() 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"))), - // The URL of submodule 'S4' is missing or invalid: ' ' - string.Format(Resources.InvalidSubmoduleUrl, "S4", " "), + // 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"))), // Access to the path 'sub6\.git' is denied diff --git a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs index 9bab50f5..c2fbfeb5 100644 --- a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs +++ b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs @@ -261,11 +261,7 @@ void reportDiagnostic(string diagnostic) continue; } - if (NullableString.IsNullOrWhiteSpace(url)) - { - reportDiagnostic(string.Format(Resources.InvalidSubmoduleUrl, name, url)); - continue; - } + // Ignore unspecified URL - Source Link doesn't use it. string fullPath; try diff --git a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitSubmodule.cs b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitSubmodule.cs index cba708d7..a953773e 100644 --- a/src/Microsoft.Build.Tasks.Git/GitDataReader/GitSubmodule.cs +++ b/src/Microsoft.Build.Tasks.Git/GitDataReader/GitSubmodule.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Diagnostics.CodeAnalysis; namespace Microsoft.Build.Tasks.Git { @@ -21,21 +20,20 @@ internal readonly struct GitSubmodule public string WorkingDirectoryFullPath { get; } /// - /// An absolute URL or a relative path (if it starts with `./` or `../`) to the default remote of the containing repository. + /// An absolute URL or a relative path (if it starts with `./` or `../`) to the origin remote of the containing repository. /// - public string Url { get; } + public string? Url { get; } /// /// Head tip commit SHA. Null, if there is no commit. /// public string? HeadCommitSha { get; } - internal GitSubmodule(string name, string workingDirectoryRelativePath, string workingDirectoryFullPath, string url, string? headCommitSha) + internal GitSubmodule(string name, string workingDirectoryRelativePath, string workingDirectoryFullPath, string? url, string? headCommitSha) { NullableDebug.Assert(name != null); NullableDebug.Assert(workingDirectoryRelativePath != null); NullableDebug.Assert(workingDirectoryFullPath != null); - NullableDebug.Assert(url != null); Name = name; WorkingDirectoryRelativePath = workingDirectoryRelativePath; diff --git a/src/Microsoft.Build.Tasks.Git/GitOperations.cs b/src/Microsoft.Build.Tasks.Git/GitOperations.cs index e21948a4..2d029de4 100644 --- a/src/Microsoft.Build.Tasks.Git/GitOperations.cs +++ b/src/Microsoft.Build.Tasks.Git/GitOperations.cs @@ -18,6 +18,7 @@ internal static class GitOperations private const string SourceControlName = "git"; private const string RemoteSectionName = "remote"; + private const string SubmoduleSectionName = "submodule"; private const string RemoteOriginName = "origin"; private const string UrlSectionName = "url"; private const string UrlVariableName = "url"; @@ -275,12 +276,22 @@ public static ITaskItem[] GetSourceRoots(GitRepository repository, string? remot continue; } - // https://git-scm.com/docs/git-submodule - var submoduleUri = NormalizeUrl(repository, submodule.Url); + // submodule..url specifies where to find the submodule. + // This variable is calculated based on the entry in .gitmodules by git submodule init and will be present for initialized submodules. + // Uninitialized modules don't have source that should be considered during the build. + // Relative URLs are relative to the repository directory. + // See https://git-scm.com/docs/gitsubmodules. + var submoduleConfigUrl = repository.Config.GetVariableValue(SubmoduleSectionName, submodule.Name, UrlVariableName); + if (submoduleConfigUrl == null) + { + continue; + } + + var submoduleUri = NormalizeUrl(repository, submoduleConfigUrl); if (submoduleUri == null) { logWarning(Resources.SourceCodeWontBeAvailableViaSourceLink, - new[] { string.Format(Resources.InvalidSubmoduleUrl, submodule.Name, submodule.Url) }); + new[] { string.Format(Resources.InvalidSubmoduleUrl, submodule.Name, submoduleConfigUrl) }); continue; } @@ -289,7 +300,7 @@ public static ITaskItem[] GetSourceRoots(GitRepository repository, string? remot if (submoduleUrl == null) { logWarning(Resources.SourceCodeWontBeAvailableViaSourceLink, - new[] { string.Format(Resources.InvalidSubmoduleUrl, submodule.Name, submodule.Url) }); + new[] { string.Format(Resources.InvalidSubmoduleUrl, submodule.Name, submoduleConfigUrl) }); continue; }