Merge pull request #3730 from crazy-max/history-mv-buildname

history: move BuildName to util/history

commands/history/inspect.go

@@ -24,6 +24,7 @@ import (
 	"github.com/docker/buildx/util/cobrautil/completion"
 	"github.com/docker/buildx/util/confutil"
 	"github.com/docker/buildx/util/desktop"
+	historyutil "github.com/docker/buildx/util/history"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/formatter"
 	"github.com/docker/cli/cli/debug"
@@ -243,7 +244,7 @@ workers0:
 	}
 	delete(attrs, "filename")
 
-	out.Name = BuildName(rec.FrontendAttrs, st)
+	out.Name = historyutil.BuildName(rec.FrontendAttrs, st)
 	out.Ref = rec.Ref
 
 	out.Context = context

commands/history/ls.go

@@ -15,6 +15,7 @@ import (
 	"github.com/docker/buildx/util/confutil"
 	"github.com/docker/buildx/util/desktop"
 	"github.com/docker/buildx/util/gitutil"
+	historyutil "github.com/docker/buildx/util/history"
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/formatter"
@@ -85,7 +86,7 @@ func runLs(ctx context.Context, dockerCli command.Cli, opts lsOptions) error {
 
 	for i, rec := range out {
 		st, _ := ls.ReadRef(rec.node.Builder, rec.node.Name, rec.Ref)
-		rec.name = BuildName(rec.FrontendAttrs, st)
+		rec.name = historyutil.BuildName(rec.FrontendAttrs, st)
 		out[i] = rec
 	}
 

commands/history/utils.go

@@ -6,7 +6,6 @@ import (
 	"encoding/csv"
 	"fmt"
 	"io"
-	"path/filepath"
 	"slices"
 	"strconv"
 	"strings"
@@ -14,8 +13,6 @@ import (
 	"time"
 
 	"github.com/docker/buildx/builder"
-	"github.com/docker/buildx/localstate"
-	"github.com/docker/buildx/util/urlutil"
 	"github.com/docker/cli/cli/command"
 	controlapi "github.com/moby/buildkit/api/services/control"
 	"github.com/moby/buildkit/frontend/dockerfile/dfgitutil"
@@ -25,90 +22,6 @@ import (
 
 const recordsLimit = 50
 
-func BuildName(fattrs map[string]string, ls *localstate.State) string {
-	if v, ok := fattrs["build-arg:BUILDKIT_BUILD_NAME"]; ok && v != "" {
-		return v
-	}
-
-	var res string
-
-	var target, contextPath, dockerfilePath, vcsSource string
-	if v, ok := fattrs["target"]; ok {
-		target = v
-	}
-	if v, ok := fattrs["context"]; ok {
-		contextPath = filepath.ToSlash(v)
-	} else if v, ok := fattrs["vcs:localdir:context"]; ok && v != "." {
-		contextPath = filepath.ToSlash(v)
-	}
-	if v, ok := fattrs["vcs:source"]; ok {
-		vcsSource = v
-	} else if v, ok := fattrs["input:context"]; ok {
-		if _, ok, _ := dfgitutil.ParseGitRef(v); ok {
-			vcsSource = v
-		}
-	}
-	if v, ok := fattrs["filename"]; ok && v != "Dockerfile" {
-		dockerfilePath = filepath.ToSlash(v)
-	}
-	if v, ok := fattrs["vcs:localdir:dockerfile"]; ok && v != "." {
-		dockerfilePath = filepath.ToSlash(filepath.Join(v, dockerfilePath))
-	}
-
-	var localPath string
-	if ls != nil && !urlutil.IsRemoteURL(ls.LocalPath) {
-		if ls.LocalPath != "" && ls.LocalPath != "-" {
-			localPath = filepath.ToSlash(ls.LocalPath)
-		}
-		if ls.DockerfilePath != "" && ls.DockerfilePath != "-" && ls.DockerfilePath != "Dockerfile" {
-			dockerfilePath = filepath.ToSlash(ls.DockerfilePath)
-		}
-	}
-
-	// remove default dockerfile name
-	const defaultFilename = "/Dockerfile"
-	hasDefaultFileName := strings.HasSuffix(dockerfilePath, defaultFilename) || dockerfilePath == ""
-	dockerfilePath = strings.TrimSuffix(dockerfilePath, defaultFilename)
-
-	// dockerfile is a subpath of context
-	if strings.HasPrefix(dockerfilePath, localPath) && len(dockerfilePath) > len(localPath) {
-		res = dockerfilePath[strings.LastIndex(localPath, "/")+1:]
-	} else {
-		// Otherwise, use basename
-		bpath := localPath
-		if len(dockerfilePath) > 0 {
-			bpath = dockerfilePath
-		}
-		if len(bpath) > 0 {
-			lidx := strings.LastIndex(bpath, "/")
-			res = bpath[lidx+1:]
-			if !hasDefaultFileName {
-				if lidx != -1 {
-					res = filepath.ToSlash(filepath.Join(filepath.Base(bpath[:lidx]), res))
-				} else {
-					res = filepath.ToSlash(filepath.Join(filepath.Base(bpath), res))
-				}
-			}
-		}
-	}
-
-	if len(contextPath) > 0 {
-		res = contextPath
-	}
-	if len(target) > 0 {
-		if len(res) > 0 {
-			res = res + " (" + target + ")"
-		} else {
-			res = target
-		}
-	}
-	if res == "" && vcsSource != "" {
-		u, _ := dfgitutil.FragmentFormat(vcsSource)
-		return u
-	}
-	return res
-}
-
 func trimBeginning(s string, n int) string {
 	if len(s) <= n {
 		return s

util/history/name.go

@@ -0,0 +1,91 @@
+package history
+
+import (
+	"path/filepath"
+	"strings"
+
+	"github.com/docker/buildx/localstate"
+	"github.com/docker/buildx/util/urlutil"
+	"github.com/moby/buildkit/frontend/dockerfile/dfgitutil"
+)
+
+func BuildName(fattrs map[string]string, ls *localstate.State) string {
+	if v, ok := fattrs["build-arg:BUILDKIT_BUILD_NAME"]; ok && v != "" {
+		return v
+	}
+
+	var res string
+
+	var target, contextPath, dockerfilePath, vcsSource string
+	if v, ok := fattrs["target"]; ok {
+		target = v
+	}
+	if v, ok := fattrs["context"]; ok {
+		contextPath = filepath.ToSlash(v)
+	} else if v, ok := fattrs["vcs:localdir:context"]; ok && v != "." {
+		contextPath = filepath.ToSlash(v)
+	}
+	if v, ok := fattrs["vcs:source"]; ok {
+		vcsSource = v
+	} else if v, ok := fattrs["input:context"]; ok {
+		if _, ok, _ := dfgitutil.ParseGitRef(v); ok {
+			vcsSource = v
+		}
+	}
+	if v, ok := fattrs["filename"]; ok && v != "Dockerfile" {
+		dockerfilePath = filepath.ToSlash(v)
+	}
+	if v, ok := fattrs["vcs:localdir:dockerfile"]; ok && v != "." {
+		dockerfilePath = filepath.ToSlash(filepath.Join(v, dockerfilePath))
+	}
+
+	var localPath string
+	if ls != nil && !urlutil.IsRemoteURL(ls.LocalPath) {
+		if ls.LocalPath != "" && ls.LocalPath != "-" {
+			localPath = filepath.ToSlash(ls.LocalPath)
+		}
+		if ls.DockerfilePath != "" && ls.DockerfilePath != "-" && ls.DockerfilePath != "Dockerfile" {
+			dockerfilePath = filepath.ToSlash(ls.DockerfilePath)
+		}
+	}
+
+	const defaultFilename = "/Dockerfile"
+	hasDefaultFileName := strings.HasSuffix(dockerfilePath, defaultFilename) || dockerfilePath == ""
+	dockerfilePath = strings.TrimSuffix(dockerfilePath, defaultFilename)
+
+	if strings.HasPrefix(dockerfilePath, localPath) && len(dockerfilePath) > len(localPath) {
+		res = dockerfilePath[strings.LastIndex(localPath, "/")+1:]
+	} else {
+		bpath := localPath
+		if len(dockerfilePath) > 0 {
+			bpath = dockerfilePath
+		}
+		if len(bpath) > 0 {
+			lidx := strings.LastIndex(bpath, "/")
+			res = bpath[lidx+1:]
+			if !hasDefaultFileName {
+				if lidx != -1 {
+					res = filepath.ToSlash(filepath.Join(filepath.Base(bpath[:lidx]), res))
+				} else {
+					res = filepath.ToSlash(filepath.Join(filepath.Base(bpath), res))
+				}
+			}
+		}
+	}
+
+	if len(contextPath) > 0 {
+		res = contextPath
+	}
+	if len(target) > 0 {
+		if len(res) > 0 {
+			res = res + " (" + target + ")"
+		} else {
+			res = target
+		}
+	}
+	if res == "" && vcsSource != "" {
+		u, _ := dfgitutil.FragmentFormat(vcsSource)
+		return u
+	}
+	return res
+}

util/history/name_test.go

@@ -0,0 +1,110 @@
+package history
+
+import (
+	"testing"
+
+	"github.com/docker/buildx/localstate"
+	"github.com/stretchr/testify/require"
+)
+
+func TestBuildName(t *testing.T) {
+	tests := []struct {
+		name  string
+		attrs map[string]string
+		state *localstate.State
+		want  string
+	}{
+		{
+			name: "empty",
+			want: "",
+		},
+		{
+			name: "override",
+			attrs: map[string]string{
+				"build-arg:BUILDKIT_BUILD_NAME": "foobar",
+			},
+			want: "foobar",
+		},
+		{
+			name: "local dockerfile path",
+			state: &localstate.State{
+				LocalPath:      "/tmp/project",
+				DockerfilePath: "/tmp/project/deploy/Dockerfile.release",
+			},
+			want: "project/deploy/Dockerfile.release",
+		},
+		{
+			name: "local default dockerfile",
+			state: &localstate.State{
+				LocalPath:      "/tmp/project",
+				DockerfilePath: "/tmp/project/Dockerfile",
+			},
+			want: "project",
+		},
+		{
+			name: "git query input",
+			attrs: map[string]string{
+				"input:context": "https://github.com/docker/buildx.git?ref=main",
+			},
+			want: "https://github.com/docker/buildx.git#main",
+		},
+		{
+			name: "vcs source",
+			attrs: map[string]string{
+				"vcs:source": "https://github.com/docker/buildx.git?ref=main",
+			},
+			want: "https://github.com/docker/buildx.git#main",
+		},
+		{
+			name: "vcs local context fallback",
+			attrs: map[string]string{
+				"vcs:localdir:context": "subdir",
+			},
+			want: "subdir",
+		},
+		{
+			name: "dockerfile attrs",
+			attrs: map[string]string{
+				"filename":                "Dockerfile.release",
+				"vcs:localdir:dockerfile": "deploy",
+			},
+			want: "deploy/Dockerfile.release",
+		},
+		{
+			name: "target only",
+			attrs: map[string]string{
+				"target": "release",
+			},
+			want: "release",
+		},
+		{
+			name: "context overrides local name",
+			attrs: map[string]string{
+				"context": "subdir",
+				"target":  "release",
+			},
+			state: &localstate.State{
+				LocalPath:      "/tmp/project",
+				DockerfilePath: "/tmp/project/deploy/Dockerfile.release",
+			},
+			want: "subdir (release)",
+		},
+		{
+			name: "remote local path ignored",
+			attrs: map[string]string{
+				"filename":                "Dockerfile.release",
+				"vcs:localdir:dockerfile": "deploy",
+			},
+			state: &localstate.State{
+				LocalPath:      "https://github.com/docker/buildx.git",
+				DockerfilePath: "/tmp/project/ignored/Dockerfile",
+			},
+			want: "deploy/Dockerfile.release",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.want, BuildName(tt.attrs, tt.state))
+		})
+	}
+}