dockerfile: treat ADD source as git URL when --keep-git-dir is set

frontend/dockerfile/dfgitutil/git_ref.go

@@ -68,7 +68,9 @@ type GitRef struct {
 }
 
 // ParseGitRef parses a git ref.
-func ParseGitRef(ref string) (*GitRef, bool, error) {
+// An HTTP(S) URL is a valid git ref when it has the ".git" suffix,
+// or when `knownGit` is true (such as when `--keep-git-dir` is passed).
+func ParseGitRef(ref string, knownGit bool) (*GitRef, bool, error) {
 	res := &GitRef{}
 
 	var (
@@ -104,9 +106,8 @@ func ParseGitRef(ref string) (*GitRef, bool, error) {
 		}
 
 		switch remote.Scheme {
-		// An HTTP(S) URL is considered to be a valid git ref only when it has the ".git[...]" suffix.
 		case gitutil.HTTPProtocol, gitutil.HTTPSProtocol:
-			if !strings.HasSuffix(remote.Path, ".git") {
+			if !knownGit && !strings.HasSuffix(remote.Path, ".git") {
 				return nil, false, errors.WithStack(cerrdefs.ErrInvalidArgument)
 			}
 		}
@@ -244,7 +245,7 @@ func (gf *GitRef) loadQuery(query url.Values) error {
 // FragmentFormat returns a simplified git URL in fragment format.
 // If the URL cannot be parsed, the original string is returned with false.
 func FragmentFormat(remote string, withSubdir bool) (string, bool) {
-	gitRef, _, err := ParseGitRef(remote)
+	gitRef, _, err := ParseGitRef(remote, false)
 	if err != nil || gitRef == nil {
 		return remote, false
 	}

frontend/dockerfile/dfgitutil/git_ref_test.go

@@ -10,6 +10,7 @@ import (
 func TestParseGitRef(t *testing.T) {
 	cases := []struct {
 		ref      string
+		knownGit bool
 		expected *GitRef
 		err      string
 	}{
@@ -268,10 +269,60 @@ func TestParseGitRef(t *testing.T) {
 			ref: "https://github.com/moby/buildkit.git?invalid=123",
 			err: "unexpected query \"invalid\"",
 		},
+		{
+			// HTTP(S) URLs do not need the ".git" suffix when the ref is known to be a git URL,
+			// e.g., for git forges that do not support the suffix, such as sourcehut.
+			ref:      "https://git.sr.ht/~foo/bar",
+			knownGit: true,
+			expected: &GitRef{
+				Remote:    "https://git.sr.ht/~foo/bar",
+				ShortName: "bar",
+			},
+		},
+		{
+			ref:      "https://git.sr.ht/~foo/bar#main",
+			knownGit: true,
+			expected: &GitRef{
+				Remote:    "https://git.sr.ht/~foo/bar",
+				ShortName: "bar",
+				Ref:       "main",
+			},
+		},
+		{
+			ref:      "https://github.com/moby/buildkit",
+			knownGit: true,
+			expected: &GitRef{
+				Remote:    "https://github.com/moby/buildkit",
+				ShortName: "buildkit",
+			},
+		},
+		{
+			ref:      "https://github.com/moby/buildkit.git",
+			knownGit: true,
+			expected: &GitRef{
+				Remote:    "https://github.com/moby/buildkit.git",
+				ShortName: "buildkit",
+			},
+		},
+		{
+			ref:      "http://example.com/foo?tag=v1.0.0",
+			knownGit: true,
+			expected: &GitRef{
+				Remote:         "http://example.com/foo",
+				ShortName:      "foo",
+				Ref:            "refs/tags/v1.0.0",
+				UnencryptedTCP: true,
+			},
+		},
+		{
+			ref:      "./.git",
+			knownGit: true,
+			expected: nil,
+		},
 	}
 	for i, tt := range cases {
 		t.Run(fmt.Sprintf("case%d", i+1), func(t *testing.T) {
-			got, _, err := ParseGitRef(tt.ref)
+			got, _, err := ParseGitRef(tt.ref, tt.knownGit)
 			if tt.expected == nil {
 				require.Nil(t, got)
 				require.Error(t, err)

frontend/dockerfile/dockerfile2llb/convert_copy.go

@@ -84,14 +84,17 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
 		}
 	}
 
+	// An explicit --keep-git-dir flag identifies the source as a git URL
+	knownGit := cfg.keepGitDir != nil
+
 	if cfg.checksum != "" {
 		if !cfg.isAddCommand {
 			return errors.New("checksum can't be specified for COPY")
 		}
 		if len(cfg.params.SourcePaths) != 1 {
 			return errors.New("checksum can't be specified for multiple sources")
 		}
-		if !isHTTPSource(cfg.params.SourcePaths[0]) && !isGitSource(cfg.params.SourcePaths[0]) {
+		if src := cfg.params.SourcePaths[0]; !isHTTPSource(src, knownGit) && !isGitSource(src, knownGit) {
 			return errors.New("checksum requires HTTP(S) or Git sources")
 		}
 	}
@@ -126,7 +129,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
 
 	for _, src := range cfg.params.SourcePaths {
 		commitMessage.WriteString(" " + src)
-		gitRef, isGit, gitRefErr := dfgitutil.ParseGitRef(src)
+		gitRef, isGit, gitRefErr := dfgitutil.ParseGitRef(src, knownGit)
 		if gitRefErr != nil && isGit {
 			return gitRefErr
 		}
@@ -181,7 +184,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
 			} else {
 				a = a.Copy(st, "/", dest, opts...)
 			}
-		} else if isHTTPSource(src) {
+		} else if isHTTPSource(src, knownGit) {
 			if !cfg.isAddCommand {
 				return errors.New("source can't be a URL for COPY")
 			}
@@ -351,16 +354,16 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
 	return commitToHistory(&d.image, commitMessage.String(), true, &d.state, d.epoch)
 }
 
-func isHTTPSource(src string) bool {
+func isHTTPSource(src string, knownGit bool) bool {
 	if !strings.HasPrefix(src, "http://") && !strings.HasPrefix(src, "https://") {
 		return false
 	}
-	return !isGitSource(src)
+	return !isGitSource(src, knownGit)
 }
 
-func isGitSource(src string) bool {
+func isGitSource(src string, knownGit bool) bool {
 	// https://github.com/ORG/REPO.git is a git source, not an http source
-	if gitRef, isGit, _ := dfgitutil.ParseGitRef(src); gitRef != nil && isGit {
+	if gitRef, isGit, _ := dfgitutil.ParseGitRef(src, knownGit); gitRef != nil && isGit {
 		return true
 	}
 	return false

frontend/dockerfile/dockerfile2llb/convert_copy_test.go

@@ -0,0 +1,58 @@
+package dockerfile2llb
+
+import (
+	"testing"
+
+	"github.com/moby/buildkit/solver/pb"
+	"github.com/moby/buildkit/util/appcontext"
+	"github.com/stretchr/testify/require"
+)
+
+func sourceIdentifiers(t *testing.T, df string) []string {
+	t.Helper()
+	res, err := Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{})
+	require.NoError(t, err)
+	def, err := res.State.Marshal(appcontext.Context())
+	require.NoError(t, err)
+	var ids []string
+	for _, dt := range def.Def {
+		var op pb.Op
+		require.NoError(t, op.Unmarshal(dt))
+		if src := op.GetSource(); src != nil {
+			ids = append(ids, src.Identifier)
+		}
+	}
+	return ids
+}
+
+func TestAddKeepGitDirNonSuffixedURL(t *testing.T) {
+	t.Parallel()
+	// with --keep-git-dir, a non-".git" URL must become a git source
+	ids := sourceIdentifiers(t, `
+FROM scratch
+ADD --keep-git-dir=true https://git.sr.ht/~foo/bar#main /dst
+`)
+	require.Contains(t, ids, "git://git.sr.ht/~foo/bar#main")
+	require.Len(t, ids, 1)
+
+	// same with --keep-git-dir=false
+	ids = sourceIdentifiers(t, `
+FROM scratch
+ADD --keep-git-dir=false https://git.sr.ht/~foo/bar#main /dst
+`)
+	require.Contains(t, ids, "git://git.sr.ht/~foo/bar#main")
+
+	// without the flag, a non-".git" URL stays an HTTP source
+	ids = sourceIdentifiers(t, `
+FROM scratch
+ADD https://git.sr.ht/~foo/bar /dst
+`)
+	require.Contains(t, ids, "https://git.sr.ht/~foo/bar")
+
+	// without the flag, a ".git" URL is still a git source
+	ids = sourceIdentifiers(t, `
+FROM scratch
+ADD https://github.com/moby/buildkit.git#master /dst
+`)
+	require.Contains(t, ids, "git://github.com/moby/buildkit.git#master")
+}

frontend/dockerfile/dockerfile2llb/epoch.go

@@ -177,7 +177,10 @@ func sourceDateEpochAddSource(cmd *instructions.AddCommand, env *llb.EnvList, sh
 		return nil, err
 	}
 
-	if isHTTPSource(src) {
+	// An explicit --keep-git-dir flag identifies the source as a git URL
+	knownGit := cmd.KeepGitDir != nil
+
+	if cmd.KeepGitDir == nil && isHTTPSource(src, knownGit) {
 		var checksum digest.Digest
 		if cmd.Checksum != "" {
 			expandedChecksum, _, err := shlex.ProcessWord(cmd.Checksum, env)
@@ -193,7 +196,7 @@ func sourceDateEpochAddSource(cmd *instructions.AddCommand, env *llb.EnvList, sh
 		return &st, nil
 	}
 
-	gitRef, isGit, gitRefErr := dfgitutil.ParseGitRef(src)
+	gitRef, isGit, gitRefErr := dfgitutil.ParseGitRef(src, knownGit)
 	if gitRefErr != nil && isGit {
 		return nil, gitRefErr
 	}

frontend/dockerfile/dockerfile_addgit_test.go

@@ -78,6 +78,7 @@ func testAddGit(t *testing.T, sb integration.Sandbox, format string) {
 	gitCommands = append(gitCommands, makeCommit("v0.0.2")...)
 	gitCommands = append(gitCommands, makeCommit("v0.0.3")...)
 	gitCommands = append(gitCommands, "git update-server-info")
+	gitCommands = append(gitCommands, "ln -s .git/ bla")
 	err = runShell(gitDir, gitCommands...)
 	require.NoError(t, err)
 
@@ -220,6 +221,39 @@ RUN [ ! -d /nogitdir/.git ]
 	}, nil)
 	require.NoError(t, err)
 
+	// Additional test: ADD from a git URL without the ".git" suffix.
+	// --keep-git-dir flag marks the URL as a git URL regardless of the suffix.
+	dockerfileNoSuffix, err := applyTemplate(`
+	FROM alpine
+	ARG REPO="{{.ServerURL}}/bla"
+	ADD ${REPO} /nogitanything
+	RUN [ ! -f /nogitanything/foo ]
+	RUN [ ! -d /nogitanything/.git ]
+	ADD --keep-git-dir=true ${REPO}#v0.0.2 /gitdir
+	RUN [ -f /gitdir/foo ]
+	RUN [ "$(cat /gitdir/foo)" = "foo of v0.0.2" ]
+	RUN [ -d /gitdir/.git ]
+	ADD --keep-git-dir=false ${REPO}#v0.0.3 /nogitdir
+	RUN [ -f /nogitdir/foo ]
+	RUN [ "$(cat /nogitdir/foo)" = "foo of v0.0.3" ]
+	RUN [ ! -d /nogitdir/.git ]
+	`, map[string]string{
+		"ServerURL": serverURL,
+	})
+	require.NoError(t, err)
+
+	dirNoSuffix := integration.Tmpdir(t,
+		fstest.CreateFile("Dockerfile", []byte(dockerfileNoSuffix), 0600),
+	)
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dirNoSuffix,
+			dockerui.DefaultLocalNameContext:    dirNoSuffix,
+		},
+	}, nil)
+	require.NoError(t, err)
+
 	// checksum does not match
 	dockerfile5, err := applyTemplate(`
 	FROM alpine

frontend/dockerfile/docs/reference.md

@@ -1601,6 +1601,17 @@ FROM alpine
 ADD --keep-git-dir=true https://github.com/moby/buildkit.git#v0.10.1 /buildkit
 ```
 
+Passing the `--keep-git-dir` flag explicitly (whether `true` or `false`) also
+marks `<src>` as the URL of a Git repository, even when the URL doesn't end
+with the `.git` suffix. This is useful for Git forges such as Azure DevOps
+or SourceHut that don't support the `.git` suffix in clone URLs.
+
+```dockerfile
+# syntax=docker/dockerfile:1
+FROM alpine
+ADD --keep-git-dir=false https://git.sr.ht/~foo/bar#main /bar
+```
+
 ### ADD --checksum
 
 ```dockerfile

frontend/dockerui/context.go

@@ -291,7 +291,7 @@ func sourceOpFromState(ctx context.Context, st *llb.State, opts ...llb.Constrain
 }
 
 func DetectGitContext(ref string, keepGit *bool, opts ...llb.GitOption) (*llb.State, bool, error) {
-	g, isGit, err := dfgitutil.ParseGitRef(ref)
+	g, isGit, err := dfgitutil.ParseGitRef(ref, keepGit != nil)
 	if err != nil {
 		return nil, isGit, err
 	}