Skip to content
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.

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

Use submodule.name.url to determine the URL of a submodule #587

Merged
merged 5 commits into from Mar 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 8 additions & 4 deletions docs/README.md
Expand Up @@ -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
```

Expand Down
9 changes: 7 additions & 2 deletions src/Microsoft.Build.Tasks.Git.UnitTests/GitDataTests.cs
Expand Up @@ -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_*");
Expand All @@ -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");
Expand Down
91 changes: 45 additions & 46 deletions src/Microsoft.Build.Tasks.Git.UnitTests/GitOperationsTests.cs
Expand Up @@ -9,6 +9,8 @@

namespace Microsoft.Build.Tasks.Git.UnitTests
{
using static KeyValuePairUtils;

public class GitOperationsTests
{
private static readonly bool IsUnix = Path.DirectorySeparatorChar == '/';
Expand Down Expand Up @@ -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<GitVariableName, ImmutableArray<string>>(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<GitVariableName, ImmutableArray<string>>(CreateVariableName(v.Name), ImmutableArray.CreateRange(v.Values)))));
variables.Select(v => KVP(CreateVariableName(v.Name), ImmutableArray.CreateRange(v.Values)))));

[Fact]
public void GetRepositoryUrl_NoRemotes()
Expand Down Expand Up @@ -179,8 +181,8 @@ public void GetRepositoryUrl_InsteadOf()
{
var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[]
{
new KeyValuePair<GitVariableName, ImmutableArray<string>>(new GitVariableName("remote", "origin", "url"), ImmutableArray.Create("http://?")),
new KeyValuePair<GitVariableName, ImmutableArray<string>>(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?[])>();
Expand All @@ -206,7 +208,7 @@ public void GetRepositoryUrl_Local(bool useFileUrl)

var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[]
{
new KeyValuePair<GitVariableName, ImmutableArray<string>>(new GitVariableName("remote", "origin", "url"),
KVP(new GitVariableName("remote", "origin", "url"),
ImmutableArray.Create(useFileUrl ? new Uri(mainWorkingDir.Path).AbsolutePath : mainWorkingDir.Path)),
})));

Expand All @@ -231,7 +233,7 @@ public void GetRepositoryUrl_Local_CustomRemoteName()

var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[]
{
new KeyValuePair<GitVariableName, ImmutableArray<string>>(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?[])>();
Expand All @@ -253,7 +255,7 @@ public void GetRepositoryUrl_Local_NoRemoteOriginUrl()

var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[]
{
new KeyValuePair<GitVariableName, ImmutableArray<string>>(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?[])>();
Expand Down Expand Up @@ -281,7 +283,7 @@ public void GetRepositoryUrl_Local_NoWorkingDirectory()

var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[]
{
new KeyValuePair<GitVariableName, ImmutableArray<string>>(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?[])>();
Expand All @@ -302,7 +304,7 @@ public void GetRepositoryUrl_Local_BadRepo()

var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[]
{
new KeyValuePair<GitVariableName, ImmutableArray<string>>(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?[])>();
Expand All @@ -322,7 +324,7 @@ public void GetRepositoryUrl_LocalRecursion()

var repo = CreateRepository(config: new GitConfig(ImmutableDictionary.CreateRange(new[]
{
new KeyValuePair<GitVariableName, ImmutableArray<string>>(new GitVariableName("remote", "origin", "url"),
KVP(new GitVariableName("remote", "origin", "url"),
ImmutableArray.Create(mainWorkingDir.Path)),
})));

Expand Down Expand Up @@ -461,47 +463,66 @@ 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]
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)));

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")) },
warnings.Select(TestUtilities.InspectDiagnostic));
}

[Fact]
public void GetSourceRoots_RelativeSubmodulePaths()
public void GetSourceRoots_RelativeSubmodulePath()
{
using var temp = new TempRoot();

Expand All @@ -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)));
Expand All @@ -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<GitOperations.DirectoryNode>? children = null)

private static GitOperations.DirectoryNode CreateNode(string name, string? submoduleWorkingDirectory, List<GitOperations.DirectoryNode>? children = null)
=> new GitOperations.DirectoryNode(name, children ?? new List<GitOperations.DirectoryNode>())
{
Matcher = (submoduleWorkingDirectory != null) ? new Lazy<GitIgnore.Matcher?>(() =>
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Build.Tasks.Git.UnitTests/GitRepositoryTests.cs
Expand Up @@ -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
Expand Down
6 changes: 1 addition & 5 deletions src/Microsoft.Build.Tasks.Git/GitDataReader/GitRepository.cs
Expand Up @@ -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
Expand Down
8 changes: 3 additions & 5 deletions 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
{
Expand All @@ -21,21 +20,20 @@ namespace Microsoft.Build.Tasks.Git
public string WorkingDirectoryFullPath { get; }

/// <summary>
/// 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.
/// </summary>
public string Url { get; }
public string? Url { get; }

/// <summary>
/// Head tip commit SHA. Null, if there is no commit.
/// </summary>
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;
Expand Down