Skip to content

Commit 9200883

Browse files
tonistiigitonistiigi
authored
Merge pull request moby#3596 from crazy-max/fix-git-context
git context: fix support for empty git ref with subdir
2 parents 0ccfe62 + 6a2287e commit 9200883

File tree

3 files changed

+241
-101
lines changed

3 files changed

+241
-101
lines changed

‎source/gitidentifier_test.go

Lines changed: 110 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,27 +7,114 @@ import (
77
)
88

99
func TestNewGitIdentifier(t *testing.T) {
10-
gi, err := NewGitIdentifier("ssh://[email protected]:2222/root/my/really/weird/path/foo.git")
11-
require.Nil(t, err)
12-
require.Equal(t, "ssh://[email protected]:2222/root/my/really/weird/path/foo.git", gi.Remote)
13-
require.Equal(t, "", gi.Ref)
14-
require.Equal(t, "", gi.Subdir)
15-
16-
gi, err = NewGitIdentifier("ssh://[email protected]:2222/root/my/really/weird/path/foo.git#main")
17-
require.Nil(t, err)
18-
require.Equal(t, "ssh://[email protected]:2222/root/my/really/weird/path/foo.git", gi.Remote)
19-
require.Equal(t, "main", gi.Ref)
20-
require.Equal(t, "", gi.Subdir)
21-
22-
gi, err = NewGitIdentifier("[email protected]:moby/buildkit.git")
23-
require.Nil(t, err)
24-
require.Equal(t, "[email protected]:moby/buildkit.git", gi.Remote)
25-
require.Equal(t, "", gi.Ref)
26-
require.Equal(t, "", gi.Subdir)
27-
28-
gi, err = NewGitIdentifier("github.com/moby/buildkit.git#main")
29-
require.Nil(t, err)
30-
require.Equal(t, "https://github.com/moby/buildkit.git", gi.Remote)
31-
require.Equal(t, "main", gi.Ref)
32-
require.Equal(t, "", gi.Subdir)
10+
tests := []struct {
11+
url string
12+
expected GitIdentifier
13+
}{
14+
{
15+
url: "ssh://[email protected]:2222/root/my/really/weird/path/foo.git",
16+
expected: GitIdentifier{
17+
Remote: "ssh://[email protected]:2222/root/my/really/weird/path/foo.git",
18+
},
19+
},
20+
{
21+
url: "ssh://[email protected]:2222/root/my/really/weird/path/foo.git#main",
22+
expected: GitIdentifier{
23+
Remote: "ssh://[email protected]:2222/root/my/really/weird/path/foo.git",
24+
Ref: "main",
25+
},
26+
},
27+
{
28+
url: "[email protected]:moby/buildkit.git",
29+
expected: GitIdentifier{
30+
Remote: "[email protected]:moby/buildkit.git",
31+
},
32+
},
33+
{
34+
url: "github.com/moby/buildkit.git#main",
35+
expected: GitIdentifier{
36+
Remote: "https://github.com/moby/buildkit.git",
37+
Ref: "main",
38+
},
39+
},
40+
{
41+
url: "git://github.com/user/repo.git",
42+
expected: GitIdentifier{
43+
Remote: "git://github.com/user/repo.git",
44+
},
45+
},
46+
{
47+
url: "git://github.com/user/repo.git#mybranch:mydir/mysubdir/",
48+
expected: GitIdentifier{
49+
Remote: "git://github.com/user/repo.git",
50+
Ref: "mybranch",
51+
Subdir: "mydir/mysubdir/",
52+
},
53+
},
54+
{
55+
url: "git://github.com/user/repo.git#:mydir/mysubdir/",
56+
expected: GitIdentifier{
57+
Remote: "git://github.com/user/repo.git",
58+
Subdir: "mydir/mysubdir/",
59+
},
60+
},
61+
{
62+
url: "https://github.com/user/repo.git",
63+
expected: GitIdentifier{
64+
Remote: "https://github.com/user/repo.git",
65+
},
66+
},
67+
{
68+
url: "https://github.com/user/repo.git#mybranch:mydir/mysubdir/",
69+
expected: GitIdentifier{
70+
Remote: "https://github.com/user/repo.git",
71+
Ref: "mybranch",
72+
Subdir: "mydir/mysubdir/",
73+
},
74+
},
75+
{
76+
url: "[email protected]:user/repo.git",
77+
expected: GitIdentifier{
78+
Remote: "[email protected]:user/repo.git",
79+
},
80+
},
81+
{
82+
url: "[email protected]:user/repo.git#mybranch:mydir/mysubdir/",
83+
expected: GitIdentifier{
84+
Remote: "[email protected]:user/repo.git",
85+
Ref: "mybranch",
86+
Subdir: "mydir/mysubdir/",
87+
},
88+
},
89+
{
90+
url: "ssh://github.com/user/repo.git",
91+
expected: GitIdentifier{
92+
Remote: "ssh://github.com/user/repo.git",
93+
},
94+
},
95+
{
96+
url: "ssh://github.com/user/repo.git#mybranch:mydir/mysubdir/",
97+
expected: GitIdentifier{
98+
Remote: "ssh://github.com/user/repo.git",
99+
Ref: "mybranch",
100+
Subdir: "mydir/mysubdir/",
101+
},
102+
},
103+
{
104+
url: "ssh://foo%[email protected]/user/repo.git#mybranch:mydir/mysubdir/",
105+
expected: GitIdentifier{
106+
Remote: "ssh://foo%[email protected]/user/repo.git",
107+
Ref: "mybranch",
108+
Subdir: "mydir/mysubdir/",
109+
},
110+
},
111+
}
112+
for _, tt := range tests {
113+
tt := tt
114+
t.Run(tt.url, func(t *testing.T) {
115+
gi, err := NewGitIdentifier(tt.url)
116+
require.NoError(t, err)
117+
require.Equal(t, tt.expected, *gi)
118+
})
119+
}
33120
}

‎util/gitutil/git_ref.go

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -73,22 +73,12 @@ func ParseGitRef(ref string) (*GitRef, error) {
7373
}
7474
}
7575

76-
refSplitBySharp := strings.SplitN(ref, "#", 2)
77-
res.Remote = refSplitBySharp[0]
76+
var fragment string
77+
res.Remote, fragment, _ = strings.Cut(ref, "#")
7878
if len(res.Remote) == 0 {
7979
return res, errdefs.ErrInvalidArgument
8080
}
81-
82-
if len(refSplitBySharp) > 1 {
83-
refSplitBySharpSplitByColon := strings.SplitN(refSplitBySharp[1], ":", 2)
84-
res.Commit = refSplitBySharpSplitByColon[0]
85-
if len(res.Commit) == 0 {
86-
return res, errdefs.ErrInvalidArgument
87-
}
88-
if len(refSplitBySharpSplitByColon) > 1 {
89-
res.SubDir = refSplitBySharpSplitByColon[1]
90-
}
91-
}
81+
res.Commit, res.SubDir, _ = strings.Cut(fragment, ":")
9282
repoSplitBySlash := strings.Split(res.Remote, "/")
9383
res.ShortName = strings.TrimSuffix(repoSplitBySlash[len(repoSplitBySlash)-1], ".git")
9484
return res, nil

‎util/gitutil/git_ref_test.go

Lines changed: 128 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -6,72 +6,135 @@ import (
66
)
77

88
func TestParseGitRef(t *testing.T) {
9-
cases := map[string]*GitRef{
10-
"https://example.com/": nil,
11-
"https://example.com/foo": nil,
12-
"https://example.com/foo.git": {
13-
Remote: "https://example.com/foo.git",
14-
ShortName: "foo",
15-
},
16-
"https://example.com/foo.git#deadbeef": {
17-
Remote: "https://example.com/foo.git",
18-
ShortName: "foo",
19-
Commit: "deadbeef",
20-
},
21-
"https://example.com/foo.git#release/1.2": {
22-
Remote: "https://example.com/foo.git",
23-
ShortName: "foo",
24-
Commit: "release/1.2",
25-
},
26-
"https://example.com/foo.git/": nil,
27-
"https://example.com/foo.git.bar": nil,
28-
"git://example.com/foo": {
29-
Remote: "git://example.com/foo",
30-
ShortName: "foo",
31-
UnencryptedTCP: true,
32-
},
33-
"github.com/moby/buildkit": {
34-
Remote: "github.com/moby/buildkit", ShortName: "buildkit",
35-
IndistinguishableFromLocal: true,
36-
},
37-
"https://github.com/moby/buildkit": nil,
38-
"https://github.com/moby/buildkit.git": {
39-
Remote: "https://github.com/moby/buildkit.git",
40-
ShortName: "buildkit",
41-
},
42-
"[email protected]:moby/buildkit": {
43-
Remote: "[email protected]:moby/buildkit",
44-
ShortName: "buildkit",
45-
},
46-
"[email protected]:moby/buildkit.git": {
47-
Remote: "[email protected]:moby/buildkit.git",
48-
ShortName: "buildkit",
49-
},
50-
"[email protected]:atlassianlabs/atlassian-docker.git": {
51-
Remote: "[email protected]:atlassianlabs/atlassian-docker.git",
52-
ShortName: "atlassian-docker",
53-
},
54-
"https://github.com/foo/bar.git#baz/qux:quux/quuz": {
55-
Remote: "https://github.com/foo/bar.git",
56-
ShortName: "bar",
57-
Commit: "baz/qux",
58-
SubDir: "quux/quuz",
59-
},
60-
"http://github.com/docker/docker.git:#branch": nil,
9+
cases := []struct {
10+
ref string
11+
expected *GitRef
12+
}{
13+
{
14+
ref: "https://example.com/",
15+
expected: nil,
16+
},
17+
{
18+
ref: "https://example.com/foo",
19+
expected: nil,
20+
},
21+
{
22+
ref: "https://example.com/foo.git",
23+
expected: &GitRef{
24+
Remote: "https://example.com/foo.git",
25+
ShortName: "foo",
26+
},
27+
},
28+
{
29+
ref: "https://example.com/foo.git#deadbeef",
30+
expected: &GitRef{
31+
Remote: "https://example.com/foo.git",
32+
ShortName: "foo",
33+
Commit: "deadbeef",
34+
},
35+
},
36+
{
37+
ref: "https://example.com/foo.git#release/1.2",
38+
expected: &GitRef{
39+
Remote: "https://example.com/foo.git",
40+
ShortName: "foo",
41+
Commit: "release/1.2",
42+
},
43+
},
44+
{
45+
ref: "https://example.com/foo.git/",
46+
expected: nil,
47+
},
48+
{
49+
ref: "https://example.com/foo.git.bar",
50+
expected: nil,
51+
},
52+
{
53+
ref: "git://example.com/foo",
54+
expected: &GitRef{
55+
Remote: "git://example.com/foo",
56+
ShortName: "foo",
57+
UnencryptedTCP: true,
58+
},
59+
},
60+
{
61+
ref: "github.com/moby/buildkit",
62+
expected: &GitRef{
63+
Remote: "github.com/moby/buildkit",
64+
ShortName: "buildkit",
65+
IndistinguishableFromLocal: true,
66+
},
67+
},
68+
{
69+
ref: "https://github.com/moby/buildkit",
70+
expected: nil,
71+
},
72+
{
73+
ref: "https://github.com/moby/buildkit.git",
74+
expected: &GitRef{
75+
Remote: "https://github.com/moby/buildkit.git",
76+
ShortName: "buildkit",
77+
},
78+
},
79+
{
80+
ref: "[email protected]:moby/buildkit",
81+
expected: &GitRef{
82+
Remote: "[email protected]:moby/buildkit",
83+
ShortName: "buildkit",
84+
},
85+
},
86+
{
87+
ref: "[email protected]:moby/buildkit.git",
88+
expected: &GitRef{
89+
Remote: "[email protected]:moby/buildkit.git",
90+
ShortName: "buildkit",
91+
},
92+
},
93+
{
94+
ref: "[email protected]:atlassianlabs/atlassian-docker.git",
95+
expected: &GitRef{
96+
Remote: "[email protected]:atlassianlabs/atlassian-docker.git",
97+
ShortName: "atlassian-docker",
98+
},
99+
},
100+
{
101+
ref: "https://github.com/foo/bar.git#baz/qux:quux/quuz",
102+
expected: &GitRef{
103+
Remote: "https://github.com/foo/bar.git",
104+
ShortName: "bar",
105+
Commit: "baz/qux",
106+
SubDir: "quux/quuz",
107+
},
108+
},
109+
{
110+
ref: "http://github.com/docker/docker.git:#branch",
111+
expected: nil,
112+
},
113+
{
114+
ref: "https://github.com/docker/docker.git#:myfolder",
115+
expected: &GitRef{
116+
Remote: "https://github.com/docker/docker.git",
117+
ShortName: "docker",
118+
SubDir: "myfolder",
119+
},
120+
},
61121
}
62-
for ref, expected := range cases {
63-
got, err := ParseGitRef(ref)
64-
if expected == nil {
65-
if err == nil {
66-
t.Errorf("expected an error for ParseGitRef(%q)", ref)
67-
}
68-
} else {
69-
if err != nil {
70-
t.Errorf("got an unexpected error: ParseGitRef(%q): %v", ref, err)
71-
}
72-
if !reflect.DeepEqual(got, expected) {
73-
t.Errorf("expected ParseGitRef(%q) to return %#v, got %#v", ref, expected, got)
122+
for _, tt := range cases {
123+
tt := tt
124+
t.Run(tt.ref, func(t *testing.T) {
125+
got, err := ParseGitRef(tt.ref)
126+
if tt.expected == nil {
127+
if err == nil {
128+
t.Errorf("expected an error for ParseGitRef(%q)", tt.ref)
129+
}
130+
} else {
131+
if err != nil {
132+
t.Errorf("got an unexpected error: ParseGitRef(%q): %v", tt.ref, err)
133+
}
134+
if !reflect.DeepEqual(got, tt.expected) {
135+
t.Errorf("expected ParseGitRef(%q) to return %#v, got %#v", tt.ref, tt.expected, got)
136+
}
74137
}
75-
}
138+
})
76139
}
77140
}

0 commit comments

Comments
 (0)