build: warn if git operation fails

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>

build/build.go

@@ -595,10 +595,6 @@ func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt Op
 		so.FrontendAttrs["attest:provenance"] = "mode=min,inline-only=true"
 	}
 
-	for k, v := range getGitAttributes(ctx, opt.Inputs.ContextPath, opt.Inputs.DockerfilePath) {
-		so.FrontendAttrs[k] = v
-	}
-
 	// set platforms
 	if len(opt.Platforms) != 0 {
 		pp := make([]string, len(opt.Platforms))
@@ -853,6 +849,10 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s
 	for k, opt := range opt {
 		multiDriver := len(m[k]) > 1
 		hasMobyDriver := false
+		gitattrs, err := getGitAttributes(ctx, opt.Inputs.ContextPath, opt.Inputs.DockerfilePath)
+		if err != nil {
+			logrus.Warn(err)
+		}
 		for i, np := range m[k] {
 			node := nodes[np.driverIndex]
 			if node.Driver.IsMobyDriver() {
@@ -862,6 +862,9 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s
 			so, release, err := toSolveOpt(ctx, node, multiDriver, opt, np.bopts, configDir, w, func(name string) (io.WriteCloser, func(), error) {
 				return docker.LoadImage(ctx, name, w)
 			})
+			for k, v := range gitattrs {
+				so.FrontendAttrs[k] = v
+			}
 			if err != nil {
 				return nil, err
 			}

build/git.go

@@ -3,18 +3,19 @@ package build
 import (
 	"context"
 	"os"
+	"path"
 	"path/filepath"
 	"strconv"
 	"strings"
 
 	"github.com/docker/buildx/util/gitutil"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
-	"github.com/sirupsen/logrus"
+	"github.com/pkg/errors"
 )
 
 const DockerfileLabel = "com.docker.image.source.entrypoint"
 
-func getGitAttributes(ctx context.Context, contextPath string, dockerfilePath string) (res map[string]string) {
+func getGitAttributes(ctx context.Context, contextPath string, dockerfilePath string) (res map[string]string, _ error) {
 	res = make(map[string]string)
 	if contextPath == "" {
 		return
@@ -50,30 +51,45 @@ func getGitAttributes(ctx context.Context, contextPath string, dockerfilePath st
 
 	gitc, err := gitutil.New(gitutil.WithContext(ctx), gitutil.WithWorkingDir(wd))
 	if err != nil {
-		logrus.Warnf("Failed to initialize git: %v", err)
+		if st, err := os.Stat(path.Join(wd, ".git")); err == nil && st.IsDir() {
+			return res, errors.New("No git was found in the system. Current commit information was not captured by the build.")
+		}
 		return
 	}
 
 	if !gitc.IsInsideWorkTree() {
-		logrus.Warnf("Unable to determine git information")
+		return res, errors.New("Not inside a git repository")
-		return
 	}
 
-	var resRevision, resSource, resDockerfilePath string
+	if sha, err := gitc.FullCommit(); err != nil {
-
+		return res, errors.Wrapf(err, "failed to get git commit")
-	if sha, err := gitc.FullCommit(); err == nil && sha != "" {
+	} else if sha != "" {
-		resRevision = sha
 		if gitc.IsDirty() {
-			resRevision += "-dirty"
+			sha += "-dirty"
+		}
+		if setGitLabels {
+			res["label:"+specs.AnnotationRevision] = sha
+		}
+		if setGitInfo {
+			res["vcs:revision"] = sha
 		}
 	}
 
-	if rurl, err := gitc.RemoteURL(); err == nil && rurl != "" {
+	if rurl, err := gitc.RemoteURL(); err != nil {
-		resSource = rurl
+		return res, errors.Wrapf(err, "failed to get git remote url")
+	} else if rurl != "" {
+		if setGitLabels {
+			res["label:"+specs.AnnotationSource] = rurl
+		}
+		if setGitInfo {
+			res["vcs:source"] = rurl
+		}
 	}
 
 	if setGitLabels {
-		if root, err := gitc.RootDir(); err == nil && root != "" {
+		if root, err := gitc.RootDir(); err != nil {
+			return res, errors.Wrapf(err, "failed to get git root dir")
+		} else if root != "" {
 			if dockerfilePath == "" {
 				dockerfilePath = filepath.Join(wd, "Dockerfile")
 			}
@@ -83,32 +99,10 @@ func getGitAttributes(ctx context.Context, contextPath string, dockerfilePath st
 			}
 			dockerfilePath, _ = filepath.Rel(root, dockerfilePath)
 			if !strings.HasPrefix(dockerfilePath, "..") {
-				resDockerfilePath = dockerfilePath
+				res["label:"+DockerfileLabel] = dockerfilePath
 			}
 		}
 	}
 
-	if resSource != "" {
-		if setGitLabels {
-			res["label:"+specs.AnnotationSource] = resSource
-		}
-		if setGitInfo {
-			res["vcs:source"] = resSource
-		}
-	}
-	if resRevision != "" {
-		if setGitLabels {
-			res["label:"+specs.AnnotationRevision] = resRevision
-		}
-		if setGitInfo {
-			res["vcs:revision"] = resRevision
-		}
-	}
-	if resDockerfilePath != "" {
-		if setGitLabels {
-			res["label:"+DockerfileLabel] = resDockerfilePath
-		}
-	}
-
 	return
 }

build/git_test.go

@@ -3,6 +3,7 @@ package build
 import (
 	"context"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -10,24 +11,42 @@ import (
 	"github.com/docker/buildx/util/gitutil"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func setupTest(tb testing.TB) *gitutil.Git {
+func setupTest(tb testing.TB) {
-	c, err := gitutil.New()
-	assert.NoError(tb, err)
 	gitutil.Mktmp(tb)
+
+	c, err := gitutil.New()
+	require.NoError(tb, err)
 	gitutil.GitInit(c, tb)
+
 	df := []byte("FROM alpine:latest\n")
 	assert.NoError(tb, os.WriteFile("Dockerfile", df, 0644))
+
 	gitutil.GitAdd(c, tb, "Dockerfile")
 	gitutil.GitCommit(c, tb, "initial commit")
-	return c
+	gitutil.GitSetRemote(c, tb, "git@github.com:docker/buildx.git")
+}
+
+func TestGetGitAttributesNotGitRepo(t *testing.T) {
+	_, err := getGitAttributes(context.Background(), t.TempDir(), "Dockerfile")
+	assert.Error(t, err)
+}
+
+func TestGetGitAttributesBadGitRepo(t *testing.T) {
+	tmp := t.TempDir()
+	require.NoError(t, os.MkdirAll(path.Join(tmp, ".git"), 0755))
+
+	_, err := getGitAttributes(context.Background(), tmp, "Dockerfile")
+	assert.Error(t, err)
 }
 
 func TestGetGitAttributesNoContext(t *testing.T) {
-	_ = setupTest(t)
+	setupTest(t)
 
-	gitattrs := getGitAttributes(context.Background(), "", "Dockerfile")
+	gitattrs, err := getGitAttributes(context.Background(), "", "Dockerfile")
+	assert.NoError(t, err)
 	assert.Empty(t, gitattrs)
 }
 
@@ -44,6 +63,7 @@ func TestGetGitAttributes(t *testing.T) {
 			envGitInfo:   "",
 			expected: []string{
 				"vcs:revision",
+				"vcs:source",
 			},
 		},
 		{
@@ -58,6 +78,7 @@ func TestGetGitAttributes(t *testing.T) {
 			envGitInfo:   "true",
 			expected: []string{
 				"vcs:revision",
+				"vcs:source",
 			},
 		},
 		{
@@ -67,6 +88,7 @@ func TestGetGitAttributes(t *testing.T) {
 			expected: []string{
 				"label:" + DockerfileLabel,
 				"label:" + specs.AnnotationRevision,
+				"label:" + specs.AnnotationSource,
 			},
 		},
 		{
@@ -76,66 +98,58 @@ func TestGetGitAttributes(t *testing.T) {
 			expected: []string{
 				"label:" + DockerfileLabel,
 				"label:" + specs.AnnotationRevision,
+				"label:" + specs.AnnotationSource,
 				"vcs:revision",
+				"vcs:source",
 			},
 		},
 	}
 	for _, tt := range cases {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			_ = setupTest(t)
+			setupTest(t)
 			if tt.envGitLabels != "" {
 				t.Setenv("BUILDX_GIT_LABELS", tt.envGitLabels)
 			}
 			if tt.envGitInfo != "" {
 				t.Setenv("BUILDX_GIT_INFO", tt.envGitInfo)
 			}
-			gitattrs := getGitAttributes(context.Background(), ".", "Dockerfile")
+			gitattrs, err := getGitAttributes(context.Background(), ".", "Dockerfile")
+			require.NoError(t, err)
 			for _, e := range tt.expected {
 				assert.Contains(t, gitattrs, e)
 				assert.NotEmpty(t, gitattrs[e])
 				if e == "label:"+DockerfileLabel {
 					assert.Equal(t, "Dockerfile", gitattrs[e])
+				} else if e == "label:"+specs.AnnotationSource || e == "vcs:source" {
+					assert.Equal(t, "git@github.com:docker/buildx.git", gitattrs[e])
 				}
 			}
 		})
 	}
 }
 
-func TestGetGitAttributesWithRemote(t *testing.T) {
-	c := setupTest(t)
-	gitutil.GitSetRemote(c, t, "git@github.com:docker/buildx.git")
-
-	t.Setenv("BUILDX_GIT_LABELS", "true")
-	gitattrs := getGitAttributes(context.Background(), ".", "Dockerfile")
-	assert.Equal(t, 5, len(gitattrs))
-	assert.Contains(t, gitattrs, "label:"+DockerfileLabel)
-	assert.Equal(t, "Dockerfile", gitattrs["label:"+DockerfileLabel])
-	assert.Contains(t, gitattrs, "label:"+specs.AnnotationRevision)
-	assert.NotEmpty(t, gitattrs["label:"+specs.AnnotationRevision])
-	assert.Contains(t, gitattrs, "label:"+specs.AnnotationSource)
-	assert.Equal(t, "git@github.com:docker/buildx.git", gitattrs["label:"+specs.AnnotationSource])
-	assert.Contains(t, gitattrs, "vcs:revision")
-	assert.NotEmpty(t, gitattrs["vcs:revision"])
-	assert.Contains(t, gitattrs, "vcs:source")
-	assert.Equal(t, "git@github.com:docker/buildx.git", gitattrs["vcs:source"])
-}
-
 func TestGetGitAttributesDirty(t *testing.T) {
-	_ = setupTest(t)
+	setupTest(t)
 
 	// make a change to test dirty flag
 	df := []byte("FROM alpine:edge\n")
-	assert.NoError(t, os.Mkdir("dir", 0755))
+	require.NoError(t, os.Mkdir("dir", 0755))
-	assert.NoError(t, os.WriteFile(filepath.Join("dir", "Dockerfile"), df, 0644))
+	require.NoError(t, os.WriteFile(filepath.Join("dir", "Dockerfile"), df, 0644))
 
 	t.Setenv("BUILDX_GIT_LABELS", "true")
-	gitattrs := getGitAttributes(context.Background(), ".", "Dockerfile")
+	gitattrs, _ := getGitAttributes(context.Background(), ".", "Dockerfile")
-	assert.Equal(t, 3, len(gitattrs))
+	assert.Equal(t, 5, len(gitattrs))
+
 	assert.Contains(t, gitattrs, "label:"+DockerfileLabel)
 	assert.Equal(t, "Dockerfile", gitattrs["label:"+DockerfileLabel])
+	assert.Contains(t, gitattrs, "label:"+specs.AnnotationSource)
+	assert.Equal(t, "git@github.com:docker/buildx.git", gitattrs["label:"+specs.AnnotationSource])
 	assert.Contains(t, gitattrs, "label:"+specs.AnnotationRevision)
 	assert.True(t, strings.HasSuffix(gitattrs["label:"+specs.AnnotationRevision], "-dirty"))
+
+	assert.Contains(t, gitattrs, "vcs:source")
+	assert.Equal(t, "git@github.com:docker/buildx.git", gitattrs["vcs:source"])
 	assert.Contains(t, gitattrs, "vcs:revision")
 	assert.True(t, strings.HasSuffix(gitattrs["vcs:revision"], "-dirty"))
 }

util/gitutil/gitutil_test.go

@@ -3,13 +3,12 @@ package gitutil
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestGit(t *testing.T) {
 	c, err := New()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	out, err := c.run("status")
 	require.NoError(t, err)
@@ -22,10 +21,10 @@ func TestGit(t *testing.T) {
 }
 
 func TestGitFullCommit(t *testing.T) {
+	Mktmp(t)
 	c, err := New()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
-	Mktmp(t)
 	GitInit(c, t)
 	GitCommit(c, t, "bar")
 
@@ -35,10 +34,10 @@ func TestGitFullCommit(t *testing.T) {
 }
 
 func TestGitShortCommit(t *testing.T) {
+	Mktmp(t)
 	c, err := New()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
-	Mktmp(t)
 	GitInit(c, t)
 	GitCommit(c, t, "bar")
 
@@ -48,10 +47,10 @@ func TestGitShortCommit(t *testing.T) {
 }
 
 func TestGitTagsPointsAt(t *testing.T) {
+	Mktmp(t)
 	c, err := New()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
-	Mktmp(t)
 	GitInit(c, t)
 	GitCommit(c, t, "bar")
 	GitTag(c, t, "v0.8.0")
@@ -64,10 +63,10 @@ func TestGitTagsPointsAt(t *testing.T) {
 }
 
 func TestGitDescribeTags(t *testing.T) {
+	Mktmp(t)
 	c, err := New()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
-	Mktmp(t)
 	GitInit(c, t)
 	GitCommit(c, t, "bar")
 	GitTag(c, t, "v0.8.0")