From e5d26a8d40922770b5f1f2f236946c1c5c86d56b Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Thu, 20 Jun 2024 00:46:30 +0200
Subject: [PATCH] build: allow BuildKit to be used on Windows daemons that
 advertise it

Commit 6fef143dbcf5fdcb436d05cf8e67d56c24bc4e6e switched the CLI to use
BuildKit by default, but as part of that removed the use of the
BuildkitVersion field as returned by Ping.

Some follow-up changes in commits e38e6c51ffb55405321038dfcf3a477f and
e7a8748b939fbf7798352cff1e0b87ef updated the logic for detecting whether
BuildKit should be used or the legacy builder, but hard-coded using the
legacy builder for Windows daemons.

While Windows / WCOW does not yet support BuildKit by default, there is
work in progress to implement it, so we should not hard-code the assumption
that a Windows daemon cannot support BuildKit.

On the daemon-side, [moby@7b153b9] (Docker v23.0) changed the default as
advertised by the daemon to be BuildKit for Linux daemons. That change
still hardcoded BuildKit to be unsupported for Windows daemons (and does
not yet allow overriding the config), but this may change for future
versions of the daemon, or test-builds.

This patch:

- Re-introduces checks for the BuildkitVersion field in the "Ping" response.
- If the Ping response from the daemon advertises that it supports BuildKit,
  the CLI will now use BuildKit as builder.
- If we didn't get a Ping response, or the Ping response did NOT advertise
  that the daemon supported BuildKit, we continue to use the current
  defaults (BuildKit for Linux daemons, and the legacy builder for Windows)
- Handling of the DOCKER_BUILDKIT environment variable is unchanged; for
  CLI.BuildKitEnabled, DOCKER_BUILDKIT always takes precedence, and for
  processBuilder the value is taken into account, but will print a warning
  when BuildKit is disabled and a Linux daemon is used. For Windows daemons,
  no warning is printed.

[moby@7b153b9]: https://github.com/moby/moby/commit/7b153b9e28b53779215de209736310f9266b3f2f

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 cli/command/cli.go    | 15 ++++++++++++---
 cmd/docker/builder.go | 20 +++++++++++++++-----
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/cli/command/cli.go b/cli/command/cli.go
index c63cd69fce..d8c3460efa 100644
--- a/cli/command/cli.go
+++ b/cli/command/cli.go
@@ -184,9 +184,18 @@ func (cli *DockerCli) BuildKitEnabled() (bool, error) {
 	if _, ok := aliasMap["builder"]; ok {
 		return true, nil
 	}
-	// otherwise, assume BuildKit is enabled but
-	// not if wcow reported from server side
-	return cli.ServerInfo().OSType != "windows", nil
+
+	si := cli.ServerInfo()
+	if si.BuildkitVersion == types.BuilderBuildKit {
+		// The daemon advertised BuildKit as the preferred builder; this may
+		// be either a Linux daemon or a Windows daemon with experimental
+		// BuildKit support enabled.
+		return true, nil
+	}
+
+	// otherwise, assume BuildKit is enabled for Linux, but disabled for
+	// Windows / WCOW, which does not yet support BuildKit by default.
+	return si.OSType != "windows", nil
 }
 
 // HooksEnabled returns whether plugin hooks are enabled.
diff --git a/cmd/docker/builder.go b/cmd/docker/builder.go
index 828af27869..ecb73264e0 100644
--- a/cmd/docker/builder.go
+++ b/cmd/docker/builder.go
@@ -9,6 +9,7 @@ import (
 
 	pluginmanager "github.com/docker/cli/cli-plugins/manager"
 	"github.com/docker/cli/cli/command"
+	"github.com/docker/docker/api/types"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
@@ -74,14 +75,23 @@ func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []st
 		return args, osargs, nil, nil
 	}
 
-	// wcow build command must use the legacy builder
-	// if not opt-in through a builder component
-	if !useBuilder && dockerCli.ServerInfo().OSType == "windows" {
-		return args, osargs, nil, nil
+	if !useBuilder {
+		// Builder is not explicitly configured as an alias for buildx.
+		// Detect whether we should use BuildKit, or fallback to the
+		// legacy builder.
+		if si := dockerCli.ServerInfo(); si.BuildkitVersion != types.BuilderBuildKit && si.OSType == "windows" {
+			// The daemon didn't advertise BuildKit as the preferred builder,
+			// so use the legacy builder, which is still the default for
+			// Windows / WCOW.
+			return args, osargs, nil, nil
+		}
 	}
 
 	if buildKitDisabled {
-		// display warning if not wcow and continue
+		// When using a Linux daemon, print a warning that the legacy builder
+		// is deprecated. For Windows / WCOW, BuildKit is still experimental,
+		// so we don't print this warning, even if the daemon advertised that
+		// it supports BuildKit.
 		if dockerCli.ServerInfo().OSType != "windows" {
 			_, _ = fmt.Fprintf(dockerCli.Err(), "%s\n\n", buildkitDisabledWarning)
 		}