From 1060328a9606939962cbb4aaaff5add1c2b213d0 Mon Sep 17 00:00:00 2001
From: CrazyMax <1951866+crazy-max@users.noreply.github.com>
Date: Tue, 8 Oct 2024 13:38:13 +0200
Subject: [PATCH] build: fix build details link in experimental mode

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
---
 cmd/buildx/main.go                       | 11 +++++-
 controller/errdefs/build.go              | 18 ++++++++-
 controller/errdefs/errdefs.pb.go         | 23 ++++++++----
 controller/errdefs/errdefs.proto         |  1 +
 controller/errdefs/errdefs_vtproto.pb.go | 47 ++++++++++++++++++++++++
 controller/local/controller.go           |  8 +++-
 controller/remote/server.go              |  8 +++-
 tests/build.go                           |  5 ---
 util/desktop/desktop.go                  |  1 -
 9 files changed, 104 insertions(+), 18 deletions(-)

diff --git a/cmd/buildx/main.go b/cmd/buildx/main.go
index 62762f4c..98c654d7 100644
--- a/cmd/buildx/main.go
+++ b/cmd/buildx/main.go
@@ -6,6 +6,7 @@ import (
 	"os"
 
 	"github.com/docker/buildx/commands"
+	controllererrors "github.com/docker/buildx/controller/errdefs"
 	"github.com/docker/buildx/util/desktop"
 	"github.com/docker/buildx/version"
 	"github.com/docker/cli/cli"
@@ -16,6 +17,7 @@ import (
 	cliflags "github.com/docker/cli/cli/flags"
 	"github.com/moby/buildkit/solver/errdefs"
 	"github.com/moby/buildkit/util/stack"
+	"github.com/pkg/errors"
 	"go.opentelemetry.io/otel"
 
 	//nolint:staticcheck // vendored dependencies may still use this
@@ -109,8 +111,15 @@ func main() {
 	} else {
 		fmt.Fprintf(cmd.Err(), "ERROR: %v\n", err)
 	}
-	if ebr, ok := err.(*desktop.ErrorWithBuildRef); ok {
+
+	var ebr *desktop.ErrorWithBuildRef
+	if errors.As(err, &ebr) {
 		ebr.Print(cmd.Err())
+	} else {
+		var be *controllererrors.BuildError
+		if errors.As(err, &be) {
+			be.PrintBuildDetails(cmd.Err())
+		}
 	}
 
 	os.Exit(1)
diff --git a/controller/errdefs/build.go b/controller/errdefs/build.go
index ffa5f6da..471adbac 100644
--- a/controller/errdefs/build.go
+++ b/controller/errdefs/build.go
@@ -1,7 +1,10 @@
 package errdefs
 
 import (
+	"io"
+
 	"github.com/containerd/typeurl/v2"
+	"github.com/docker/buildx/util/desktop"
 	"github.com/moby/buildkit/util/grpcerrors"
 )
 
@@ -22,11 +25,22 @@ func (e *BuildError) ToProto() grpcerrors.TypedErrorProto {
 	return e.Build
 }
 
-func WrapBuild(err error, ref string) error {
+func (e *BuildError) PrintBuildDetails(w io.Writer) error {
+	if e.BuildRef == "" {
+		return nil
+	}
+	ebr := &desktop.ErrorWithBuildRef{
+		Ref: e.BuildRef,
+		Err: e.error,
+	}
+	return ebr.Print(w)
+}
+
+func WrapBuild(err error, ref string, buildRef string) error {
 	if err == nil {
 		return nil
 	}
-	return &BuildError{Build: &Build{Ref: ref}, error: err}
+	return &BuildError{Build: &Build{Ref: ref, BuildRef: buildRef}, error: err}
 }
 
 func (b *Build) WrapError(err error) error {
diff --git a/controller/errdefs/errdefs.pb.go b/controller/errdefs/errdefs.pb.go
index ead4df64..40505a99 100644
--- a/controller/errdefs/errdefs.pb.go
+++ b/controller/errdefs/errdefs.pb.go
@@ -25,7 +25,8 @@ type Build struct {
 	sizeCache     protoimpl.SizeCache
 	unknownFields protoimpl.UnknownFields
 
-	Ref string `protobuf:"bytes,1,opt,name=Ref,proto3" json:"Ref,omitempty"`
+	Ref      string `protobuf:"bytes,1,opt,name=Ref,proto3" json:"Ref,omitempty"`
+	BuildRef string `protobuf:"bytes,2,opt,name=BuildRef,proto3" json:"BuildRef,omitempty"`
 }
 
 func (x *Build) Reset() {
@@ -67,6 +68,13 @@ func (x *Build) GetRef() string {
 	return ""
 }
 
+func (x *Build) GetBuildRef() string {
+	if x != nil {
+		return x.BuildRef
+	}
+	return ""
+}
+
 var File_github_com_docker_buildx_controller_errdefs_errdefs_proto protoreflect.FileDescriptor
 
 var file_github_com_docker_buildx_controller_errdefs_errdefs_proto_rawDesc = []byte{
@@ -75,12 +83,13 @@ var file_github_com_docker_buildx_controller_errdefs_errdefs_proto_rawDesc = []b
 	0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2f, 0x65, 0x72, 0x72, 0x64, 0x65, 0x66, 0x73, 0x2f, 0x65, 0x72,
 	0x72, 0x64, 0x65, 0x66, 0x73, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x15, 0x64, 0x6f, 0x63,
 	0x6b, 0x65, 0x72, 0x2e, 0x62, 0x75, 0x69, 0x6c, 0x64, 0x78, 0x2e, 0x65, 0x72, 0x72, 0x64, 0x65,
-	0x66, 0x73, 0x22, 0x19, 0x0a, 0x05, 0x42, 0x75, 0x69, 0x6c, 0x64, 0x12, 0x10, 0x0a, 0x03, 0x52,
-	0x65, 0x66, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x52, 0x65, 0x66, 0x42, 0x2d, 0x5a,
-	0x2b, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x64, 0x6f, 0x63, 0x6b,
-	0x65, 0x72, 0x2f, 0x62, 0x75, 0x69, 0x6c, 0x64, 0x78, 0x2f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f,
-	0x6c, 0x6c, 0x65, 0x72, 0x2f, 0x65, 0x72, 0x72, 0x64, 0x65, 0x66, 0x73, 0x62, 0x06, 0x70, 0x72,
-	0x6f, 0x74, 0x6f, 0x33,
+	0x66, 0x73, 0x22, 0x35, 0x0a, 0x05, 0x42, 0x75, 0x69, 0x6c, 0x64, 0x12, 0x10, 0x0a, 0x03, 0x52,
+	0x65, 0x66, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x52, 0x65, 0x66, 0x12, 0x1a, 0x0a,
+	0x08, 0x42, 0x75, 0x69, 0x6c, 0x64, 0x52, 0x65, 0x66, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52,
+	0x08, 0x42, 0x75, 0x69, 0x6c, 0x64, 0x52, 0x65, 0x66, 0x42, 0x2d, 0x5a, 0x2b, 0x67, 0x69, 0x74,
+	0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x64, 0x6f, 0x63, 0x6b, 0x65, 0x72, 0x2f, 0x62,
+	0x75, 0x69, 0x6c, 0x64, 0x78, 0x2f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72,
+	0x2f, 0x65, 0x72, 0x72, 0x64, 0x65, 0x66, 0x73, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33,
 }
 
 var (
diff --git a/controller/errdefs/errdefs.proto b/controller/errdefs/errdefs.proto
index f99b5953..ded837e9 100644
--- a/controller/errdefs/errdefs.proto
+++ b/controller/errdefs/errdefs.proto
@@ -6,4 +6,5 @@ option go_package = "github.com/docker/buildx/controller/errdefs";
 
 message Build {
   string Ref = 1;
+  string BuildRef = 2;
 }
diff --git a/controller/errdefs/errdefs_vtproto.pb.go b/controller/errdefs/errdefs_vtproto.pb.go
index 0b256532..cab6582e 100644
--- a/controller/errdefs/errdefs_vtproto.pb.go
+++ b/controller/errdefs/errdefs_vtproto.pb.go
@@ -25,6 +25,7 @@ func (m *Build) CloneVT() *Build {
 	}
 	r := new(Build)
 	r.Ref = m.Ref
+	r.BuildRef = m.BuildRef
 	if len(m.unknownFields) > 0 {
 		r.unknownFields = make([]byte, len(m.unknownFields))
 		copy(r.unknownFields, m.unknownFields)
@@ -45,6 +46,9 @@ func (this *Build) EqualVT(that *Build) bool {
 	if this.Ref != that.Ref {
 		return false
 	}
+	if this.BuildRef != that.BuildRef {
+		return false
+	}
 	return string(this.unknownFields) == string(that.unknownFields)
 }
 
@@ -85,6 +89,13 @@ func (m *Build) MarshalToSizedBufferVT(dAtA []byte) (int, error) {
 		i -= len(m.unknownFields)
 		copy(dAtA[i:], m.unknownFields)
 	}
+	if len(m.BuildRef) > 0 {
+		i -= len(m.BuildRef)
+		copy(dAtA[i:], m.BuildRef)
+		i = protohelpers.EncodeVarint(dAtA, i, uint64(len(m.BuildRef)))
+		i--
+		dAtA[i] = 0x12
+	}
 	if len(m.Ref) > 0 {
 		i -= len(m.Ref)
 		copy(dAtA[i:], m.Ref)
@@ -105,6 +116,10 @@ func (m *Build) SizeVT() (n int) {
 	if l > 0 {
 		n += 1 + l + protohelpers.SizeOfVarint(uint64(l))
 	}
+	l = len(m.BuildRef)
+	if l > 0 {
+		n += 1 + l + protohelpers.SizeOfVarint(uint64(l))
+	}
 	n += len(m.unknownFields)
 	return n
 }
@@ -170,6 +185,38 @@ func (m *Build) UnmarshalVT(dAtA []byte) error {
 			}
 			m.Ref = string(dAtA[iNdEx:postIndex])
 			iNdEx = postIndex
+		case 2:
+			if wireType != 2 {
+				return fmt.Errorf("proto: wrong wireType = %d for field BuildRef", wireType)
+			}
+			var stringLen uint64
+			for shift := uint(0); ; shift += 7 {
+				if shift >= 64 {
+					return protohelpers.ErrIntOverflow
+				}
+				if iNdEx >= l {
+					return io.ErrUnexpectedEOF
+				}
+				b := dAtA[iNdEx]
+				iNdEx++
+				stringLen |= uint64(b&0x7F) << shift
+				if b < 0x80 {
+					break
+				}
+			}
+			intStringLen := int(stringLen)
+			if intStringLen < 0 {
+				return protohelpers.ErrInvalidLength
+			}
+			postIndex := iNdEx + intStringLen
+			if postIndex < 0 {
+				return protohelpers.ErrInvalidLength
+			}
+			if postIndex > l {
+				return io.ErrUnexpectedEOF
+			}
+			m.BuildRef = string(dAtA[iNdEx:postIndex])
+			iNdEx = postIndex
 		default:
 			iNdEx = preIndex
 			skippy, err := protohelpers.Skip(dAtA[iNdEx:])
diff --git a/controller/local/controller.go b/controller/local/controller.go
index 48e7e00d..68c5a809 100644
--- a/controller/local/controller.go
+++ b/controller/local/controller.go
@@ -11,6 +11,7 @@ import (
 	controllererrors "github.com/docker/buildx/controller/errdefs"
 	controllerapi "github.com/docker/buildx/controller/pb"
 	"github.com/docker/buildx/controller/processes"
+	"github.com/docker/buildx/util/desktop"
 	"github.com/docker/buildx/util/ioset"
 	"github.com/docker/buildx/util/progress"
 	"github.com/docker/cli/cli/command"
@@ -56,7 +57,12 @@ func (b *localController) Build(ctx context.Context, options *controllerapi.Buil
 			buildOptions: options,
 		}
 		if buildErr != nil {
-			buildErr = controllererrors.WrapBuild(buildErr, b.ref)
+			var buildRef string
+			var ebr *desktop.ErrorWithBuildRef
+			if errors.As(buildErr, &ebr) {
+				buildRef = ebr.Ref
+			}
+			buildErr = controllererrors.WrapBuild(buildErr, b.ref, buildRef)
 		}
 	}
 	if buildErr != nil {
diff --git a/controller/remote/server.go b/controller/remote/server.go
index b0b82bef..f586571b 100644
--- a/controller/remote/server.go
+++ b/controller/remote/server.go
@@ -11,6 +11,7 @@ import (
 	controllererrors "github.com/docker/buildx/controller/errdefs"
 	"github.com/docker/buildx/controller/pb"
 	"github.com/docker/buildx/controller/processes"
+	"github.com/docker/buildx/util/desktop"
 	"github.com/docker/buildx/util/ioset"
 	"github.com/docker/buildx/util/progress"
 	"github.com/docker/buildx/version"
@@ -210,7 +211,12 @@ func (m *Server) Build(ctx context.Context, req *pb.BuildRequest) (*pb.BuildResp
 			s.buildOptions = req.Options
 			m.session[ref] = s
 			if buildErr != nil {
-				buildErr = controllererrors.WrapBuild(buildErr, ref)
+				var buildRef string
+				var ebr *desktop.ErrorWithBuildRef
+				if errors.As(buildErr, &ebr) {
+					buildRef = ebr.Ref
+				}
+				buildErr = controllererrors.WrapBuild(buildErr, ref, buildRef)
 			}
 		}
 	} else {
diff --git a/tests/build.go b/tests/build.go
index beb8fbe4..046689dd 100644
--- a/tests/build.go
+++ b/tests/build.go
@@ -493,11 +493,6 @@ RUN echo foo > /bar`)
 	require.NoError(t, err, string(out))
 	require.True(t, buildDetailsPattern.MatchString(string(out)), fmt.Sprintf("expected build details link in output, got %q", out))
 
-	if isExperimental() {
-		// FIXME: https://github.com/docker/buildx/issues/2382
-		t.Skip("build details link not displayed in experimental mode when build fails: https://github.com/docker/buildx/issues/2382")
-	}
-
 	// build erroneous dockerfile
 	dockerfile = []byte(`FROM busybox:latest
 RUN exit 1`)
diff --git a/util/desktop/desktop.go b/util/desktop/desktop.go
index 7fa4ee6d..563b8f2f 100644
--- a/util/desktop/desktop.go
+++ b/util/desktop/desktop.go
@@ -65,7 +65,6 @@ func hyperlink(url string) string {
 type ErrorWithBuildRef struct {
 	Ref string
 	Err error
-	Msg string
 }
 
 func (e *ErrorWithBuildRef) Error() string {