From 2acefa3bd23458bc1aaabfd7b8def252c3bfa73e Mon Sep 17 00:00:00 2001
From: Simone Basso <bassosimone@gmail.com>
Date: Wed, 25 Jan 2023 17:20:33 +0100
Subject: [PATCH] feat: rewrite windows build rules in Go (#1050)

This diff rewrites Windows build rules in Go.

See https://github.com/ooni/probe/issues/2401.
---
 CLI/check-mingw-w64-version            |  37 ------
 CLI/go-build-windows                   |  55 ---------
 Makefile                               |  11 +-
 internal/cmd/buildtool/builddeps.go    |   2 +-
 internal/cmd/buildtool/main.go         |   1 +
 internal/cmd/buildtool/windows.go      | 123 +++++++++++++++++++
 internal/cmd/buildtool/windows_test.go | 156 +++++++++++++++++++++++++
 internal/logx/logx.go                  |   2 +
 internal/logx/logx_test.go             |   5 +
 9 files changed, 290 insertions(+), 102 deletions(-)
 delete mode 100755 CLI/check-mingw-w64-version
 delete mode 100755 CLI/go-build-windows
 create mode 100644 internal/cmd/buildtool/windows.go
 create mode 100644 internal/cmd/buildtool/windows_test.go

diff --git a/CLI/check-mingw-w64-version b/CLI/check-mingw-w64-version
deleted file mode 100755
index 37687145bf..0000000000
--- a/CLI/check-mingw-w64-version
+++ /dev/null
@@ -1,37 +0,0 @@
-#!/bin/bash
-
-set -euo pipefail
-
-EXPECTED_MINGW_W64_VERSION=${EXPECTED_MINGW_W64_VERSION:-12.2.0} # Allow overriding
-
-printf "checking for x86_64-w64-mingw32-gcc... "
-command -v x86_64-w64-mingw32-gcc || {
-	echo "not found"
-	exit 1
-}
-
-printf "checking for i686-w64-mingw32-gcc... "
-command -v i686-w64-mingw32-gcc || {
-	echo "not found"
-	exit 1
-}
-
-exitcode=0
-
-printf "checking for x86_64-w64-mingw32-gcc version... "
-__version_amd64=$(x86_64-w64-mingw32-gcc --version | sed -n 1p | awk '{print $3}')
-echo $__version_amd64
-[[ "$EXPECTED_MINGW_W64_VERSION" == "$__version_amd64" ]] || {
-	echo "fatal: x86_64-w64-mingw32-gcc version must be $EXPECTED_MINGW_W64_VERSION instead of $__version_amd64"
-	exitcode=1
-}
-
-printf "checking for i686-w64-mingw32-gcc version... "
-__version_386=$(i686-w64-mingw32-gcc --version | sed -n 1p | awk '{print $3}')
-echo $__version_386
-[[ "$EXPECTED_MINGW_W64_VERSION" == "$__version_386" ]] || {
-	echo "fatal: i686-w64-mingw32-gcc version must be $EXPECTED_MINGW_W64_VERSION instead of $__version_386"
-	exitcode=1
-}
-
-exit $exitcode
diff --git a/CLI/go-build-windows b/CLI/go-build-windows
deleted file mode 100755
index 09f63ab2d0..0000000000
--- a/CLI/go-build-windows
+++ /dev/null
@@ -1,55 +0,0 @@
-#!/bin/bash
-
-set -euo pipefail
-
-if [[ $# -ne 2 ]]; then
-	echo "" 1>&2
-	echo "Compiler for a Go PACKAGE producing windows/GOARCH binaries." 1>&2
-	echo "" 1>&2
-	echo "usage: $0 GOARCH PACKAGE" 1>&2
-	echo "" 1>&2
-	echo "GOARCH must be one of: 386, amd64." 1>&2
-	echo "" 1>&2
-	echo "Features:" 1>&2
-	echo "" 1>&2
-	echo "* automatically sets -tags=ooni_psiphon_config when possible;" 1>&2
-	echo "" 1>&2
-	echo "* if GOLANG_EXTRA_FLAGS is set, pass it to the Go compiler." 1>&2
-	echo "" 1>&2
-	echo "Example:" 1>&2
-	echo "" 1>&2
-	echo "  ./CLI/go-build-windows amd64 ./internal/cmd/miniooni" 1>&2
-	echo "" 1>&2
-	exit 1
-fi
-
-GOOS=windows
-GOARCH=$1
-PACKAGE=$2
-
-if [[ -f ./internal/engine/psiphon-config.json.age &&
-	-f ./internal/engine/psiphon-config.key ]]; then
-	OONI_PSIPHON_TAGS=ooni_psiphon_config
-else
-	OONI_PSIPHON_TAGS=""
-fi
-
-PRODUCT=$(basename $PACKAGE)
-
-if [[ $GOARCH == "amd64" ]]; then
-	CC=x86_64-w64-mingw32-gcc
-elif [[ $GOARCH == "386" ]]; then
-	CC=i686-w64-mingw32-gcc
-else
-	echo "FATAL: unsupported GOARCH: $GOARCH" 1>&2
-	exit 1
-fi
-
-set -x
-export CC=$CC
-export CGO_ENABLED=1
-export GOOS=$GOOS
-export GOARCH=$GOARCH
-go build -tags=$OONI_PSIPHON_TAGS -ldflags="-s -w" \
-	-o ./CLI/$PRODUCT-$GOOS-$GOARCH.exe ${GOLANG_EXTRA_FLAGS:-} \
-	$PACKAGE
diff --git a/Makefile b/Makefile
index c5b8dd1a39..4227309f90 100644
--- a/Makefile
+++ b/Makefile
@@ -144,11 +144,8 @@ CLI/ooniprobe: maybe/copypsiphon search/for/go
 #help: The `make CLI/windows` command builds the ooniprobe and miniooni
 #help: command line clients for windows/386 and windows/amd64.
 .PHONY: CLI/windows
-CLI/windows: search/for/go search/for/mingw-w64 maybe/copypsiphon
-	./CLI/go-build-windows 386 ./internal/cmd/miniooni
-	./CLI/go-build-windows 386 ./cmd/ooniprobe
-	./CLI/go-build-windows amd64 ./internal/cmd/miniooni
-	./CLI/go-build-windows amd64 ./cmd/ooniprobe
+CLI/windows:
+	go run ./internal/cmd/buildtool windows
 
 #help:
 #help: The `make MOBILE/android` command builds the oonimkall library for Android.
@@ -184,10 +181,6 @@ search/for/java:
 	@printf "checking for java... "
 	@command -v java || { echo "not found"; exit 1; }
 
-.PHONY: search/for/mingw-w64
-search/for/mingw-w64:
-	./CLI/check-mingw-w64-version
-
 .PHONY: search/for/xcode
 search/for/xcode:
 	./MOBILE/ios/check-xcode-version
diff --git a/internal/cmd/buildtool/builddeps.go b/internal/cmd/buildtool/builddeps.go
index 9d7a538f07..7cccf682a7 100644
--- a/internal/cmd/buildtool/builddeps.go
+++ b/internal/cmd/buildtool/builddeps.go
@@ -39,5 +39,5 @@ func (*buildDeps) PsiphonMaybeCopyConfigFiles() {
 
 // WindowsMingwCheck implements buildtoolmodel.Dependencies
 func (*buildDeps) WindowsMingwCheck() {
-	//windowsMingwCheck() /* TODO(bassosimone) */
+	windowsMingwCheck()
 }
diff --git a/internal/cmd/buildtool/main.go b/internal/cmd/buildtool/main.go
index dc25b82378..1871769e50 100644
--- a/internal/cmd/buildtool/main.go
+++ b/internal/cmd/buildtool/main.go
@@ -17,6 +17,7 @@ func main() {
 		Short: "Tool for building ooniprobe, miniooni, etc.",
 	}
 	root.AddCommand(darwinSubcommand())
+	root.AddCommand(windowsSubcommand())
 	logHandler := logx.NewHandlerWithDefaultSettings()
 	logHandler.Emoji = true
 	log.Log = &log.Logger{Level: log.InfoLevel, Handler: logHandler}
diff --git a/internal/cmd/buildtool/windows.go b/internal/cmd/buildtool/windows.go
new file mode 100644
index 0000000000..58fc4c58c3
--- /dev/null
+++ b/internal/cmd/buildtool/windows.go
@@ -0,0 +1,123 @@
+package main
+
+//
+// Windows build
+//
+
+import (
+	"errors"
+	"os"
+	"strings"
+
+	"github.com/apex/log"
+	"github.com/ooni/probe-cli/v3/internal/cmd/buildtool/internal/buildtoolmodel"
+	"github.com/ooni/probe-cli/v3/internal/must"
+	"github.com/ooni/probe-cli/v3/internal/runtimex"
+	"github.com/ooni/probe-cli/v3/internal/shellx"
+	"github.com/spf13/cobra"
+)
+
+// windowsSubcommand returns the windows sucommand.
+func windowsSubcommand() *cobra.Command {
+	return &cobra.Command{
+		Use:   "windows",
+		Short: "Builds ooniprobe and miniooni for windows",
+		Run: func(cmd *cobra.Command, args []string) {
+			windowsBuildAll(&buildDeps{})
+		},
+		Args: cobra.NoArgs,
+	}
+}
+
+// windowsBuildAll is the main function of the windows subcommand.
+func windowsBuildAll(deps buildtoolmodel.Dependencies) {
+	deps.PsiphonMaybeCopyConfigFiles()
+	deps.GolangCheck()
+	deps.WindowsMingwCheck()
+	archs := []string{"386", "amd64"}
+	products := []*product{productMiniooni, productOoniprobe}
+	for _, arch := range archs {
+		for _, product := range products {
+			windowsBuildPackage(deps, arch, product)
+		}
+	}
+}
+
+// windowsBuildPackage builds the given package for windows
+// compiling for the specified architecture.
+func windowsBuildPackage(deps buildtoolmodel.Dependencies, goarch string, product *product) {
+	log.Infof("building %s for windows/%s", product.Pkg, goarch)
+
+	argv := runtimex.Try1(shellx.NewArgv("go", "build"))
+	if deps.PsiphonFilesExist() {
+		argv.Append("-tags", "ooni_psiphon_config")
+	}
+
+	argv.Append("-ldflags", "-s -w")
+	argv.Append("-o", product.DestinationPath("windows", goarch))
+	argv.Append(product.Pkg)
+
+	envp := &shellx.Envp{}
+	switch goarch {
+	case "amd64":
+		envp.Append("CC", windowsMingwAmd64Compiler)
+	case "386":
+		envp.Append("CC", windowsMingw386Compiler)
+	default:
+		panic(errors.New("unsupported windows goarch"))
+	}
+
+	envp.Append("CGO_ENABLED", "1")
+	envp.Append("GOARCH", goarch)
+	envp.Append("GOOS", "windows")
+
+	config := &shellx.Config{
+		Logger: log.Log,
+		Flags:  shellx.FlagShowStdoutStderr,
+	}
+
+	runtimex.Try0(shellx.RunEx(config, argv, envp))
+}
+
+// windowsMingwExpectedVersion is the expected version of mingw-w64,
+// which may be overriden by setting the EXPECTED_MINGW_W64_VERSION
+// environment variable before starting the build.
+var windowsMingwExpectedVersion = "12.2.0"
+
+// windowsMingwEnvironmentVariable is the name of the environment variable
+// that overrides the expected mingw version.
+const windowsMingwEnvironmentVariable = "EXPECTED_MINGW_W64_VERSION"
+
+// windowsMingwAmd64Compiler is the amd64 compiler.
+const windowsMingwAmd64Compiler = "x86_64-w64-mingw32-gcc"
+
+// windowsMingw386Compiler is the 386 compiler.
+const windowsMingw386Compiler = "i686-w64-mingw32-gcc"
+
+// windowsMingwCheck checks we're using the correct mingw version.
+func windowsMingwCheck() {
+	windowsMingwCheckFor(windowsMingwAmd64Compiler)
+	windowsMingwCheckFor(windowsMingw386Compiler)
+}
+
+// windowsMingwCheckFor implements mingwCheck for the given compiler.
+func windowsMingwCheckFor(compiler string) {
+	expected := windowsMingwExpectedVersionGetter()
+	firstLine := string(must.FirstLineBytes(must.RunOutputQuiet(compiler, "--version")))
+	v := strings.Split(firstLine, " ")
+	runtimex.Assert(len(v) == 3, "expected to see exactly three tokens")
+	if got := v[2]; got != expected {
+		log.Fatalf("expected mingw %s but got %s", expected, got)
+	}
+	log.Infof("using %s %s", compiler, expected)
+}
+
+// windowsMingwEexpectedVersionGetter returns the correct expected mingw version.
+func windowsMingwExpectedVersionGetter() string {
+	value := os.Getenv(windowsMingwEnvironmentVariable)
+	if value == "" {
+		return windowsMingwExpectedVersion
+	}
+	log.Infof("mingw version overriden using %s", windowsMingwEnvironmentVariable)
+	return value
+}
diff --git a/internal/cmd/buildtool/windows_test.go b/internal/cmd/buildtool/windows_test.go
new file mode 100644
index 0000000000..f11e6ac45d
--- /dev/null
+++ b/internal/cmd/buildtool/windows_test.go
@@ -0,0 +1,156 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/cmd/buildtool/internal/buildtooltest"
+	"github.com/ooni/probe-cli/v3/internal/shellx/shellxtesting"
+)
+
+func TestWindowsBuildAll(t *testing.T) {
+
+	// testspec specifies a test case for this test
+	type testspec struct {
+		// name is the name of the test case
+		name string
+
+		// hasPsiphon indicates whether we should build with psiphon config
+		hasPsiphon bool
+
+		// expectations contains the commands we expect to see
+		expect []buildtooltest.ExecExpectations
+	}
+
+	var testcases = []testspec{{
+		name:       "build where we have the psiphon config",
+		hasPsiphon: true,
+		expect: []buildtooltest.ExecExpectations{{
+			Env: []string{
+				"CC=i686-w64-mingw32-gcc",
+				"CGO_ENABLED=1",
+				"GOARCH=386",
+				"GOOS=windows",
+			},
+			Argv: []string{
+				"go", "build", "-tags", "ooni_psiphon_config",
+				"-ldflags", "-s -w", "-o", "CLI/miniooni-windows-386.exe",
+				"./internal/cmd/miniooni",
+			},
+		}, {
+			Env: []string{
+				"CC=i686-w64-mingw32-gcc",
+				"CGO_ENABLED=1",
+				"GOARCH=386",
+				"GOOS=windows",
+			},
+			Argv: []string{
+				"go", "build", "-tags", "ooni_psiphon_config",
+				"-ldflags", "-s -w", "-o", "CLI/ooniprobe-windows-386.exe",
+				"./cmd/ooniprobe",
+			},
+		}, {
+			Env: []string{
+				"CC=x86_64-w64-mingw32-gcc",
+				"CGO_ENABLED=1",
+				"GOARCH=amd64",
+				"GOOS=windows",
+			},
+			Argv: []string{
+				"go", "build", "-tags", "ooni_psiphon_config",
+				"-ldflags", "-s -w", "-o", "CLI/miniooni-windows-amd64.exe",
+				"./internal/cmd/miniooni",
+			},
+		}, {
+			Env: []string{
+				"CC=x86_64-w64-mingw32-gcc",
+				"CGO_ENABLED=1",
+				"GOARCH=amd64",
+				"GOOS=windows",
+			},
+			Argv: []string{
+				"go", "build", "-tags", "ooni_psiphon_config",
+				"-ldflags", "-s -w", "-o", "CLI/ooniprobe-windows-amd64.exe",
+				"./cmd/ooniprobe",
+			},
+		}},
+	}, {
+		name:       "build where we don't have the psiphon config",
+		hasPsiphon: false,
+		expect: []buildtooltest.ExecExpectations{{
+			Env: []string{
+				"CC=i686-w64-mingw32-gcc",
+				"CGO_ENABLED=1",
+				"GOARCH=386",
+				"GOOS=windows",
+			},
+			Argv: []string{
+				"go", "build", "-ldflags", "-s -w", "-o", "CLI/miniooni-windows-386.exe",
+				"./internal/cmd/miniooni",
+			},
+		}, {
+			Env: []string{
+				"CC=i686-w64-mingw32-gcc",
+				"CGO_ENABLED=1",
+				"GOARCH=386",
+				"GOOS=windows",
+			},
+			Argv: []string{
+				"go", "build", "-ldflags", "-s -w", "-o", "CLI/ooniprobe-windows-386.exe",
+				"./cmd/ooniprobe",
+			},
+		}, {
+			Env: []string{
+				"CC=x86_64-w64-mingw32-gcc",
+				"CGO_ENABLED=1",
+				"GOARCH=amd64",
+				"GOOS=windows",
+			},
+			Argv: []string{
+				"go", "build", "-ldflags", "-s -w", "-o", "CLI/miniooni-windows-amd64.exe",
+				"./internal/cmd/miniooni",
+			},
+		}, {
+			Env: []string{
+				"CC=x86_64-w64-mingw32-gcc",
+				"CGO_ENABLED=1",
+				"GOARCH=amd64",
+				"GOOS=windows",
+			},
+			Argv: []string{
+				"go", "build", "-ldflags", "-s -w", "-o", "CLI/ooniprobe-windows-amd64.exe",
+				"./cmd/ooniprobe",
+			},
+		}},
+	}}
+
+	for _, testcase := range testcases {
+		t.Run(testcase.name, func(t *testing.T) {
+
+			cc := &buildtooltest.SimpleCommandCollector{}
+
+			deps := &buildtooltest.DependenciesCallCounter{
+				HasPsiphon: testcase.hasPsiphon,
+			}
+
+			shellxtesting.WithCustomLibrary(cc, func() {
+				windowsBuildAll(deps)
+			})
+
+			expectCalls := map[string]int{
+				buildtooltest.TagGolangCheck:                 1,
+				buildtooltest.TagPsiphonMaybeCopyConfigFiles: 1,
+				buildtooltest.TagPsiphonFilesExist:           4,
+				buildtooltest.TagWindowsMingwCheck:           1,
+			}
+
+			if diff := cmp.Diff(expectCalls, deps.Counter); diff != "" {
+				t.Fatal(diff)
+			}
+
+			if err := buildtooltest.CheckManyCommands(cc.Commands, testcase.expect); err != nil {
+				t.Fatal(err)
+			}
+		})
+	}
+}
diff --git a/internal/logx/logx.go b/internal/logx/logx.go
index c4123db309..27c265892f 100644
--- a/internal/logx/logx.go
+++ b/internal/logx/logx.go
@@ -48,6 +48,8 @@ func (h *Handler) HandleLog(e *log.Entry) (err error) {
 			level = "  "
 		case log.WarnLevel:
 			level = "🔥"
+		case log.FatalLevel:
+			level = "🚨"
 		default:
 			// keep the original string
 		}
diff --git a/internal/logx/logx_test.go b/internal/logx/logx_test.go
index 3f7d532cc5..97e6163f1d 100644
--- a/internal/logx/logx_test.go
+++ b/internal/logx/logx_test.go
@@ -80,6 +80,11 @@ func TestLogHandlerHandleLog(t *testing.T) {
 		Emoji:          true,
 		Level:          log.WarnLevel,
 		ExpectSeverity: "🔥",
+	}, {
+		Name:           "fatal level with emoji",
+		Emoji:          true,
+		Level:          log.FatalLevel,
+		ExpectSeverity: "🚨",
 	}}
 
 	for _, cnf := range configs {