Skip to content

Commit 0df34b7

Browse files
AkihiroSudaAkihiroSuda
committed
git url: support CSV form
CSV parameters can be now specified after "##" in a git URL. e.g., https://github.com/user/repo.git##tag=mytag,branch=main,commit=cafebab,subdir=/dir tag, branch, and commit are exclusive. The follow-up commit will allow specifying tag and commit together. Fix issue 4905, but "source" in the original proposal was renamed to "subdir". The documents will be added in https://github.com/docker/docs/blob/main/content/manuals/build/concepts/context.md#url-fragments Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
1 parent 37daea9 commit 0df34b7

5 files changed

Lines changed: 215 additions & 76 deletions

File tree

‎frontend/dockerfile/dockerfile2llb/convert.go‎

Lines changed: 81 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,7 +1464,11 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
14641464
if len(cfg.params.SourcePaths) != 1 {
14651465
return errors.New("checksum can't be specified for multiple sources")
14661466
}
1467-
if !isHTTPSource(cfg.params.SourcePaths[0]) {
1467+
ok, err := isHTTPSource(cfg.params.SourcePaths[0])
1468+
if err != nil {
1469+
return err
1470+
}
1471+
if !ok {
14681472
return errors.New("checksum can't be specified for non-HTTP(S) sources")
14691473
}
14701474
}
@@ -1523,77 +1527,83 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
15231527
} else {
15241528
a = a.Copy(st, "/", dest, opts...)
15251529
}
1526-
} else if isHTTPSource(src) {
1527-
if !cfg.isAddCommand {
1528-
return errors.New("source can't be a URL for COPY")
1530+
} else {
1531+
isHTTPSourceV, err := isHTTPSource(src)
1532+
if err != nil {
1533+
return err
15291534
}
1535+
if isHTTPSourceV {
1536+
if !cfg.isAddCommand {
1537+
return errors.New("source can't be a URL for COPY")
1538+
}
15301539

1531-
// Resources from remote URLs are not decompressed.
1532-
// https://docs.docker.com/engine/reference/builder/#add
1533-
//
1534-
// Note: mixing up remote archives and local archives in a single ADD instruction
1535-
// would result in undefined behavior: https://github.com/moby/buildkit/pull/387#discussion_r189494717
1536-
u, err := url.Parse(src)
1537-
f := "__unnamed__"
1538-
if err == nil {
1539-
if base := path.Base(u.Path); base != "." && base != "/" {
1540-
f = base
1540+
// Resources from remote URLs are not decompressed.
1541+
// https://docs.docker.com/engine/reference/builder/#add
1542+
//
1543+
// Note: mixing up remote archives and local archives in a single ADD instruction
1544+
// would result in undefined behavior: https://github.com/moby/buildkit/pull/387#discussion_r189494717
1545+
u, err := url.Parse(src)
1546+
f := "__unnamed__"
1547+
if err == nil {
1548+
if base := path.Base(u.Path); base != "." && base != "/" {
1549+
f = base
1550+
}
15411551
}
1542-
}
15431552

1544-
st := llb.HTTP(src, llb.Filename(f), llb.WithCustomName(pgName), llb.Checksum(cfg.checksum), dfCmd(cfg.params))
1553+
st := llb.HTTP(src, llb.Filename(f), llb.WithCustomName(pgName), llb.Checksum(cfg.checksum), dfCmd(cfg.params))
15451554

1546-
opts := append([]llb.CopyOption{&llb.CopyInfo{
1547-
Mode: chopt,
1548-
CreateDestPath: true,
1549-
}}, copyOpt...)
1555+
opts := append([]llb.CopyOption{&llb.CopyInfo{
1556+
Mode: chopt,
1557+
CreateDestPath: true,
1558+
}}, copyOpt...)
15501559

1551-
if a == nil {
1552-
a = llb.Copy(st, f, dest, opts...)
1553-
} else {
1554-
a = a.Copy(st, f, dest, opts...)
1555-
}
1556-
} else {
1557-
validateCopySourcePath(src, &cfg)
1558-
var patterns []string
1559-
if cfg.parents {
1560-
// detect optional pivot point
1561-
parent, pattern, ok := strings.Cut(src, "/./")
1562-
if !ok {
1563-
pattern = src
1564-
src = "/"
1560+
if a == nil {
1561+
a = llb.Copy(st, f, dest, opts...)
15651562
} else {
1566-
src = parent
1563+
a = a.Copy(st, f, dest, opts...)
15671564
}
1565+
} else {
1566+
validateCopySourcePath(src, &cfg)
1567+
var patterns []string
1568+
if cfg.parents {
1569+
// detect optional pivot point
1570+
parent, pattern, ok := strings.Cut(src, "/./")
1571+
if !ok {
1572+
pattern = src
1573+
src = "/"
1574+
} else {
1575+
src = parent
1576+
}
1577+
1578+
pattern, err = system.NormalizePath("/", pattern, d.platform.OS, false)
1579+
if err != nil {
1580+
return errors.Wrap(err, "removing drive letter")
1581+
}
15681582

1569-
pattern, err = system.NormalizePath("/", pattern, d.platform.OS, false)
1583+
patterns = []string{strings.TrimPrefix(pattern, "/")}
1584+
}
1585+
1586+
src, err = system.NormalizePath("/", src, d.platform.OS, false)
15701587
if err != nil {
15711588
return errors.Wrap(err, "removing drive letter")
15721589
}
15731590

1574-
patterns = []string{strings.TrimPrefix(pattern, "/")}
1575-
}
1576-
1577-
src, err = system.NormalizePath("/", src, d.platform.OS, false)
1578-
if err != nil {
1579-
return errors.Wrap(err, "removing drive letter")
1580-
}
1581-
1582-
opts := append([]llb.CopyOption{&llb.CopyInfo{
1583-
Mode: chopt,
1584-
FollowSymlinks: true,
1585-
CopyDirContentsOnly: true,
1586-
IncludePatterns: patterns,
1587-
AttemptUnpack: cfg.isAddCommand,
1588-
CreateDestPath: true,
1589-
AllowWildcard: true,
1590-
AllowEmptyWildcard: true,
1591-
}}, copyOpt...)
1592-
1593-
if a == nil {
1594-
a = llb.Copy(cfg.source, src, dest, opts...)
1595-
} else {
1596-
a = a.Copy(cfg.source, src, dest, opts...)
1591+
opts := append([]llb.CopyOption{&llb.CopyInfo{
1592+
Mode: chopt,
1593+
FollowSymlinks: true,
1594+
CopyDirContentsOnly: true,
1595+
IncludePatterns: patterns,
1596+
AttemptUnpack: cfg.isAddCommand,
1597+
CreateDestPath: true,
1598+
AllowWildcard: true,
1599+
AllowEmptyWildcard: true,
1600+
}}, copyOpt...)
1601+
1602+
if a == nil {
1603+
a = llb.Copy(cfg.source, src, dest, opts...)
1604+
} else {
1605+
a = a.Copy(cfg.source, src, dest, opts...)
1606+
}
15971607
}
15981608
}
15991609
}
@@ -2255,15 +2265,22 @@ func commonImageNames() []string {
22552265
return out
22562266
}
22572267

2258-
func isHTTPSource(src string) bool {
2268+
func isHTTPSource(src string) (bool, error) {
22592269
if !strings.HasPrefix(src, "http://") && !strings.HasPrefix(src, "https://") {
2260-
return false
2270+
return false, nil
22612271
}
22622272
// https://github.com/ORG/REPO.git is a git source, not an http source
2263-
if gitRef, gitErr := gitutil.ParseGitRef(src); gitRef != nil && gitErr == nil {
2264-
return false
2273+
gitRef, gitErr := gitutil.ParseGitRef(src)
2274+
var eiuf *gitutil.ErrInvalidURLFragemnt
2275+
if errors.As(gitErr, &eiuf) {
2276+
// this is a git source, and it has an invalid URL fragment
2277+
// (e.g., both tag and branch are specified)
2278+
return false, gitErr
2279+
}
2280+
if gitRef != nil && gitErr == nil {
2281+
return false, nil
22652282
}
2266-
return true
2283+
return true, nil
22672284
}
22682285

22692286
func isEnabledForStage(stage string, value string) bool {

‎util/gitutil/git_ref.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,14 @@ func ParseGitRef(ref string) (*GitRef, error) {
6161
return nil, cerrdefs.ErrInvalidArgument
6262
} else if strings.HasPrefix(ref, "github.com/") {
6363
res.IndistinguishableFromLocal = true // Deprecated
64-
remote = fromURL(&url.URL{
64+
remote, err = fromURL(&url.URL{
6565
Scheme: "https",
6666
Host: "github.com",
6767
Path: strings.TrimPrefix(ref, "github.com/"),
6868
})
69+
if err != nil {
70+
return nil, err
71+
}
6972
} else {
7073
remote, err = ParseURL(ref)
7174
if errors.Is(err, ErrUnknownProtocol) {

‎util/gitutil/git_ref_test.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,15 @@ func TestParseGitRef(t *testing.T) {
138138
ref: ".git",
139139
expected: nil,
140140
},
141+
{
142+
ref: "https://example.com/foo.git##branch=release/1.2,subdir=/subdir",
143+
expected: &GitRef{
144+
Remote: "https://example.com/foo.git",
145+
ShortName: "foo",
146+
Commit: "release/1.2",
147+
SubDir: "/subdir",
148+
},
149+
},
141150
}
142151
for _, tt := range cases {
143152
tt := tt

‎util/gitutil/git_url.go‎

Lines changed: 77 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/moby/buildkit/util/sshutil"
99
"github.com/pkg/errors"
10+
"github.com/tonistiigi/go-csvvalue"
1011
)
1112

1213
const (
@@ -66,12 +67,59 @@ type GitURLFragment struct {
6667

6768
// splitGitFragment splits a git URL fragment into its respective git
6869
// reference and subdirectory components.
69-
func splitGitFragment(fragment string) *GitURLFragment {
70+
func splitGitFragment(fragment string) (*GitURLFragment, error) {
71+
if strings.HasPrefix(fragment, "#") {
72+
// Double-hash in the unparsed URL.
73+
// e.g., https://github.com/user/repo.git##tag=tag,subdir=/dir
74+
return splitGitFragmentCSVForm(fragment)
75+
}
76+
// Single-hash in the unparsed URL.
77+
// e.g., https://github.com/user/repo.git#branch_or_tag_or_commit:dir
7078
if fragment == "" {
71-
return nil
79+
return nil, nil
7280
}
7381
ref, subdir, _ := strings.Cut(fragment, ":")
74-
return &GitURLFragment{Ref: ref, Subdir: subdir}
82+
return &GitURLFragment{Ref: ref, Subdir: subdir}, nil
83+
}
84+
85+
func splitGitFragmentCSVForm(fragment string) (*GitURLFragment, error) {
86+
fragment = strings.TrimPrefix(fragment, "#")
87+
if fragment == "" {
88+
return nil, nil
89+
}
90+
fields, err := csvvalue.Fields(fragment, nil)
91+
if err != nil {
92+
return nil, errors.Wrapf(err, "failed to parse CSV %q", fragment)
93+
}
94+
95+
res := &GitURLFragment{}
96+
refs := make(map[string]string)
97+
for _, field := range fields {
98+
key, value, ok := strings.Cut(field, "=")
99+
if !ok {
100+
return nil, errors.Errorf("invalid field '%s' must be a key=value pair", field)
101+
}
102+
key = strings.ToLower(key)
103+
switch key {
104+
case "tag", "branch", "commit":
105+
refs[key] = value
106+
case "subdir":
107+
res.Subdir = value
108+
default:
109+
return nil, errors.Errorf("unexpected key '%s' in '%s' (supported keys: tag, branch, commit, subdir)", key, field)
110+
}
111+
}
112+
if len(refs) > 0 {
113+
if len(refs) > 1 {
114+
// TODO: allow specifying tag and commit together https://github.com/moby/buildkit/issues/5871
115+
return nil, errors.New("tag, branch, and commit are exclusive")
116+
}
117+
for _, v := range refs {
118+
res.Ref = v
119+
break
120+
}
121+
}
122+
return res, nil
75123
}
76124

77125
// ParseURL parses a BuildKit-style Git URL (that may contain additional
@@ -86,11 +134,11 @@ func ParseURL(remote string) (*GitURL, error) {
86134
if err != nil {
87135
return nil, err
88136
}
89-
return fromURL(url), nil
137+
return fromURL(url)
90138
}
91139

92140
if url, err := sshutil.ParseSCPStyleURL(remote); err == nil {
93-
return fromSCPStyleURL(url), nil
141+
return fromSCPStyleURL(url)
94142
}
95143

96144
return nil, ErrUnknownProtocol
@@ -105,28 +153,46 @@ func IsGitTransport(remote string) bool {
105153
return sshutil.IsImplicitSSHTransport(remote)
106154
}
107155

108-
func fromURL(url *url.URL) *GitURL {
156+
// ErrInvalidURLFragemnt is returned for an invalid URL fragment
157+
type ErrInvalidURLFragemnt struct {
158+
error
159+
}
160+
161+
func fromURL(url *url.URL) (*GitURL, error) {
109162
withoutFragment := *url
110163
withoutFragment.Fragment = ""
111-
return &GitURL{
164+
fragment, err := splitGitFragment(url.Fragment)
165+
if err != nil {
166+
return nil, &ErrInvalidURLFragemnt{
167+
error: errors.Wrapf(err, "failed to parse URL fragment %q", url.Fragment),
168+
}
169+
}
170+
gitURL := &GitURL{
112171
Scheme: url.Scheme,
113172
User: url.User,
114173
Host: url.Host,
115174
Path: url.Path,
116-
Fragment: splitGitFragment(url.Fragment),
175+
Fragment: fragment,
117176
Remote: withoutFragment.String(),
118177
}
178+
return gitURL, nil
119179
}
120180

121-
func fromSCPStyleURL(url *sshutil.SCPStyleURL) *GitURL {
181+
func fromSCPStyleURL(url *sshutil.SCPStyleURL) (*GitURL, error) {
122182
withoutFragment := *url
123183
withoutFragment.Fragment = ""
184+
fragment, err := splitGitFragment(url.Fragment)
185+
if err != nil {
186+
return nil, &ErrInvalidURLFragemnt{
187+
error: errors.Wrapf(err, "failed to parse URL fragment %q", url.Fragment),
188+
}
189+
}
124190
return &GitURL{
125191
Scheme: SSHProtocol,
126192
User: url.User,
127193
Host: url.Host,
128194
Path: url.Path,
129-
Fragment: splitGitFragment(url.Fragment),
195+
Fragment: fragment,
130196
Remote: withoutFragment.String(),
131-
}
197+
}, nil
132198
}

0 commit comments

Comments
 (0)