diff --git a/.github/workflows/goversion.yml b/.github/workflows/goversion.yml new file mode 100644 index 0000000000..9e82ba24b7 --- /dev/null +++ b/.github/workflows/goversion.yml @@ -0,0 +1,28 @@ +# Ensures that ./internal/cmd/buildtool works with different go versions + +name: goversion +on: + push: + branches: + - "release/**" + - "fullbuild" + tags: + - "v*" + schedule: + - cron: "17 7 * * *" + +jobs: + build_with_unexpected_go_version: + strategy: + matrix: + goversion: ["1.17", "1.18", "1.19", "1.21"] + system: [ubuntu-latest, windows-latest, macos-latest] + runs-on: "${{ matrix.system }}" + steps: + - uses: actions/checkout@v3 + + - uses: actions/setup-go@v4 + with: + go-version: "${{ matrix.goversion }}" + + - run: go run ./internal/cmd/buildtool generic miniooni diff --git a/internal/cmd/buildtool/android.go b/internal/cmd/buildtool/android.go index 174aa6d7a4..d0b9cb4d74 100644 --- a/internal/cmd/buildtool/android.go +++ b/internal/cmd/buildtool/android.go @@ -78,7 +78,6 @@ func androidSubcommand() *cobra.Command { // androidBuildGomobile invokes the gomobile build. func androidBuildGomobile(deps buildtoolmodel.Dependencies) { deps.PsiphonMaybeCopyConfigFiles() - deps.GolangCheck() androidHome := deps.AndroidSDKCheck() ndkDir := deps.AndroidNDKCheck(androidHome) @@ -146,7 +145,6 @@ func androidNDKCheck(androidHome string) string { // androidBuildCLIAll builds all products in CLI mode for Android func androidBuildCLIAll(deps buildtoolmodel.Dependencies) { deps.PsiphonMaybeCopyConfigFiles() - deps.GolangCheck() androidHome := deps.AndroidSDKCheck() ndkDir := deps.AndroidNDKCheck(androidHome) @@ -177,7 +175,7 @@ func androidBuildCLIProductArch( log.Infof("building %s for android/%s", product.Pkg, ooniArch) - argv := runtimex.Try1(shellx.NewArgv("go", "build")) + argv := runtimex.Try1(shellx.NewArgv(deps.GolangBinary(), "build")) if deps.PsiphonFilesExist() { argv.Append("-tags", "ooni_psiphon_config,ooni_libtor") } else { diff --git a/internal/cmd/buildtool/android_test.go b/internal/cmd/buildtool/android_test.go index a053bf3d8f..69ce3cbb1a 100644 --- a/internal/cmd/buildtool/android_test.go +++ b/internal/cmd/buildtool/android_test.go @@ -121,7 +121,7 @@ func TestAndroidBuildGomobile(t *testing.T) { buildtooltest.TagGOPATH: 2, buildtooltest.TagAndroidNDKCheck: 1, buildtooltest.TagAndroidSDKCheck: 1, - buildtooltest.TagGolangCheck: 1, + buildtooltest.TagGolangBinary: 1, buildtooltest.TagPsiphonMaybeCopyConfigFiles: 1, buildtooltest.TagPsiphonFilesExist: 1, } @@ -406,7 +406,7 @@ func TestAndroidBuildCLIAll(t *testing.T) { expectCalls := map[string]int{ buildtooltest.TagAndroidNDKCheck: 1, buildtooltest.TagAndroidSDKCheck: 1, - buildtooltest.TagGolangCheck: 1, + buildtooltest.TagGolangBinary: 1, buildtooltest.TagPsiphonMaybeCopyConfigFiles: 1, buildtooltest.TagPsiphonFilesExist: 8, } diff --git a/internal/cmd/buildtool/builddeps.go b/internal/cmd/buildtool/builddeps.go index cc4bececfb..f7dfb4a9cd 100644 --- a/internal/cmd/buildtool/builddeps.go +++ b/internal/cmd/buildtool/builddeps.go @@ -38,9 +38,9 @@ func (*buildDeps) GOPATH() string { return golangGOPATH() } -// GolangCheck implements buildtoolmodel.Dependencies -func (*buildDeps) GolangCheck() { - golangCheck("GOVERSION") +// GolangBinary implements buildtoolmodel.Dependencies +func (*buildDeps) GolangBinary() string { + return golangBinary() } // LinuxReadGOVERSION implements buildtoolmodel.Dependencies diff --git a/internal/cmd/buildtool/darwin.go b/internal/cmd/buildtool/darwin.go index e9dfd4a4a3..fa8af860bb 100644 --- a/internal/cmd/buildtool/darwin.go +++ b/internal/cmd/buildtool/darwin.go @@ -27,7 +27,6 @@ func darwinSubcommand() *cobra.Command { // darwinBuildAll builds all the packages for darwin. func darwinBuildAll(deps buildtoolmodel.Dependencies) { deps.PsiphonMaybeCopyConfigFiles() - deps.GolangCheck() archs := []string{"amd64", "arm64"} products := []*product{productMiniooni, productOoniprobe} for _, arch := range archs { @@ -41,7 +40,7 @@ func darwinBuildAll(deps buildtoolmodel.Dependencies) { func darwinBuildPackage(deps buildtoolmodel.Dependencies, goarch string, product *product) { log.Infof("building %s for darwin/%s", product.Pkg, goarch) - argv := runtimex.Try1(shellx.NewArgv("go", "build")) + argv := runtimex.Try1(shellx.NewArgv(deps.GolangBinary(), "build")) if deps.PsiphonFilesExist() { argv.Append("-tags", "ooni_psiphon_config") } diff --git a/internal/cmd/buildtool/darwin_test.go b/internal/cmd/buildtool/darwin_test.go index 3c7dc42fa4..b618067be5 100644 --- a/internal/cmd/buildtool/darwin_test.go +++ b/internal/cmd/buildtool/darwin_test.go @@ -130,7 +130,7 @@ func TestDarwinBuildAll(t *testing.T) { }) expectCalls := map[string]int{ - buildtooltest.TagGolangCheck: 1, + buildtooltest.TagGolangBinary: 1, buildtooltest.TagPsiphonMaybeCopyConfigFiles: 1, buildtooltest.TagPsiphonFilesExist: 4, } diff --git a/internal/cmd/buildtool/generic.go b/internal/cmd/buildtool/generic.go index 081fa69be1..df66bd541b 100644 --- a/internal/cmd/buildtool/generic.go +++ b/internal/cmd/buildtool/generic.go @@ -48,14 +48,15 @@ func genericSubcommand() *cobra.Command { return cmd } +// TODO(bassosimone): golangBinary() MUST be a method of the buildtoolmodel.Dependencies + // genericBuildPackage is the generic function for building a package. func genericBuildPackage(deps buildtoolmodel.Dependencies, product *product) { deps.PsiphonMaybeCopyConfigFiles() - deps.GolangCheck() log.Infof("building %s for %s/%s", product.Pkg, runtime.GOOS, runtime.GOARCH) - argv := runtimex.Try1(shellx.NewArgv("go", "build")) + argv := runtimex.Try1(shellx.NewArgv(golangBinary(), "build")) if deps.PsiphonFilesExist() { argv.Append("-tags", "ooni_psiphon_config") } @@ -67,7 +68,6 @@ func genericBuildPackage(deps buildtoolmodel.Dependencies, product *product) { // genericBuildLibrary is the generic function for building a library. func genericBuildLibrary(deps buildtoolmodel.Dependencies, product *product) { - deps.GolangCheck() os := deps.GOOS() log.Infof("building %s for %s/%s", product.Pkg, os, runtime.GOARCH) @@ -76,7 +76,7 @@ func genericBuildLibrary(deps buildtoolmodel.Dependencies, product *product) { // packages paths are separated by forward slashes! library := runtimex.Try1(generateLibrary(path.Base(product.Pkg), os)) - argv := runtimex.Try1(shellx.NewArgv("go", "build")) + argv := runtimex.Try1(shellx.NewArgv(golangBinary(), "build")) argv.Append("-buildmode", "c-shared") argv.Append("-o", library) argv.Append(product.Pkg) diff --git a/internal/cmd/buildtool/generic_test.go b/internal/cmd/buildtool/generic_test.go index f41ff7bd63..f689718ce6 100644 --- a/internal/cmd/buildtool/generic_test.go +++ b/internal/cmd/buildtool/generic_test.go @@ -83,7 +83,7 @@ func TestGenericBuildPackage(t *testing.T) { }) expectCalls := map[string]int{ - buildtooltest.TagGolangCheck: 1, + buildtooltest.TagGolangBinary: 1, buildtooltest.TagPsiphonMaybeCopyConfigFiles: 1, buildtooltest.TagPsiphonFilesExist: 1, } @@ -164,8 +164,8 @@ func TestCheckGenericBuildLibrary(t *testing.T) { }) expectCalls := map[string]int{ - buildtooltest.TagGolangCheck: 1, - buildtooltest.TagGOOS: 1, + buildtooltest.TagGolangBinary: 1, + buildtooltest.TagGOOS: 1, } if diff := cmp.Diff(expectCalls, deps.Counter); diff != "" { diff --git a/internal/cmd/buildtool/golang.go b/internal/cmd/buildtool/golang.go index e086534708..1e8c1cfd11 100644 --- a/internal/cmd/buildtool/golang.go +++ b/internal/cmd/buildtool/golang.go @@ -5,23 +5,73 @@ package main // import ( + "fmt" + "path/filepath" "strings" + "sync" "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/must" "github.com/ooni/probe-cli/v3/internal/runtimex" ) -// golangCheck checks whether the "go" binary is the correct version -func golangCheck(filename string) { +// golangCheckCorrectVersion returns true if the version of Go is correct. +func golangCheckCorrectVersion(filename string) bool { + // read the version of go that we would like to use expected := string(must.FirstLineBytes(must.ReadFile(filename))) + + // read the version of go that we're using firstline := string(must.FirstLineBytes(must.RunOutput(log.Log, "go", "version"))) vec := strings.Split(firstline, " ") runtimex.Assert(len(vec) == 4, "expected four tokens") - if got := vec[2]; got != "go"+expected { - log.Fatalf("expected go%s but got %s", expected, got) + + // make sure they're equal + return vec[2] == "go"+expected +} + +// golangInstall installs and returns the path to the correct version of Go. +func golangInstall(filename string) string { + // read the version of Go we would like to use + expected := string(must.FirstLineBytes(must.ReadFile(filename))) + + // install the downloaded script + packageName := fmt.Sprintf("golang.org/dl/go%s@latest", expected) + must.Run(log.Log, "go", "install", "-v", packageName) + + // run the downloader script + gobinary := filepath.Join( + string(must.FirstLineBytes(must.RunOutput(log.Log, "go", "env", "GOPATH"))), + "bin", + fmt.Sprintf("go%s", expected), + ) + must.Run(log.Log, gobinary, "download") + + // if all is good, then we have the right gobinary + return gobinary +} + +// golangBinaryWithoutCache returns the path to the correct golang binary to use. +func golangBinaryWithoutCache() string { + if !golangCheckCorrectVersion("GOVERSION") { + return golangInstall("GOVERSION") + } + return "go" +} + +// golangCachedBinary is the cached golang binary. +var golangCachedBinary string + +// golangCacheMu synchronizes accesses to [golangCachedBinary]. +var golangCacheMu sync.Mutex + +// golangBinary returns the path to the correct golang binary to use. +func golangBinary() string { + defer golangCacheMu.Unlock() + golangCacheMu.Lock() + if golangCachedBinary == "" { + golangCachedBinary = golangBinaryWithoutCache() } - log.Infof("using go%s", expected) + return golangCachedBinary } // golangGOPATH returns the GOPATH value. diff --git a/internal/cmd/buildtool/golang_test.go b/internal/cmd/buildtool/golang_test.go index 5f64e678f0..566358b7ab 100644 --- a/internal/cmd/buildtool/golang_test.go +++ b/internal/cmd/buildtool/golang_test.go @@ -1,11 +1,13 @@ package main import ( - "path/filepath" "testing" + + "github.com/ooni/probe-cli/v3/internal/must" ) -func TestGolangCheck(t *testing.T) { - // make sure the code does not panic when it runs - golangCheck(filepath.Join("..", "..", "..", "GOVERSION")) +func TestGolangBinary(t *testing.T) { + // make sure the code does not panic when it runs and returns a valid binary + value := golangBinary() + must.RunQuiet(value, "version") } diff --git a/internal/cmd/buildtool/gomobile.go b/internal/cmd/buildtool/gomobile.go index f868c7fad4..61354e25d1 100644 --- a/internal/cmd/buildtool/gomobile.go +++ b/internal/cmd/buildtool/gomobile.go @@ -35,16 +35,16 @@ type gomobileConfig struct { // gomobileBuild runs a build based on gomobile. func gomobileBuild(config *gomobileConfig) { // Undoes the effects of go-getting golang.org/x/mobile/cmd/gomobile - defer must.Run(log.Log, "go", "mod", "tidy") + defer must.Run(log.Log, config.deps.GolangBinary(), "mod", "tidy") - must.Run(log.Log, "go", "install", "golang.org/x/mobile/cmd/gomobile@latest") + must.Run(log.Log, config.deps.GolangBinary(), "install", "golang.org/x/mobile/cmd/gomobile@latest") gopath := config.deps.GOPATH() gomobile := filepath.Join(gopath, "bin", "gomobile") must.Run(log.Log, gomobile, "init") // Adding gomobile to go.mod as documented by golang.org/wiki/Mobile - must.Run(log.Log, "go", "get", "-d", "golang.org/x/mobile/cmd/gomobile") + must.Run(log.Log, config.deps.GolangBinary(), "get", "-d", "golang.org/x/mobile/cmd/gomobile") argv := runtimex.Try1(shellx.NewArgv(gomobile, "bind")) argv.Append("-target", config.target) diff --git a/internal/cmd/buildtool/internal/buildtoolmodel/buildtoolmodel.go b/internal/cmd/buildtool/internal/buildtoolmodel/buildtoolmodel.go index bac8c6856a..137a397c1b 100644 --- a/internal/cmd/buildtool/internal/buildtoolmodel/buildtoolmodel.go +++ b/internal/cmd/buildtool/internal/buildtoolmodel/buildtoolmodel.go @@ -24,9 +24,8 @@ type Dependencies interface { // GOPATH returns the current GOPATH. GOPATH() string - // GolangCheck ensures we have the correct - // version of golang as the "go" binary. - GolangCheck() + // GolangBinary returns the golang binary to use. + GolangBinary() string // LinuxWriteDockerfile writes the dockerfile for linux. LinuxWriteDockerfile(filename string, content []byte, mode fs.FileMode) diff --git a/internal/cmd/buildtool/internal/buildtooltest/buildtooltest.go b/internal/cmd/buildtool/internal/buildtooltest/buildtooltest.go index e9081489f4..d5c492aa0a 100644 --- a/internal/cmd/buildtool/internal/buildtooltest/buildtooltest.go +++ b/internal/cmd/buildtool/internal/buildtooltest/buildtooltest.go @@ -139,7 +139,7 @@ const ( TagAndroidNDKCheck = "androidNDK" TagAndroidSDKCheck = "androidSDK" TagGOPATH = "GOPATH" - TagGolangCheck = "golangCheck" + TagGolangBinary = "golangBinary" TagLinuxReadGOVERSION = "linuxReadGOVERSION" TagLinuxWriteDockerfile = "linuxWriteDockerfile" TagMustChdir = "mustChdir" @@ -188,8 +188,9 @@ func (cc *DependenciesCallCounter) GOPATH() string { } // golangCheck implements buildtoolmodel.Dependencies -func (cc *DependenciesCallCounter) GolangCheck() { - cc.increment(TagGolangCheck) +func (cc *DependenciesCallCounter) GolangBinary() string { + cc.increment(TagGolangBinary) + return "go" } // linuxReadGOVERSION implements buildtoolmodel.Dependencies diff --git a/internal/cmd/buildtool/internal/buildtooltest/buildtooltest_test.go b/internal/cmd/buildtool/internal/buildtooltest/buildtooltest_test.go index fd8dfb90a4..866a85e9d4 100644 --- a/internal/cmd/buildtool/internal/buildtooltest/buildtooltest_test.go +++ b/internal/cmd/buildtool/internal/buildtooltest/buildtooltest_test.go @@ -203,10 +203,13 @@ func TestDependenciesCallCounter(t *testing.T) { t.Run("GolangCheck", func(t *testing.T) { cc := &DependenciesCallCounter{} - cc.GolangCheck() - if cc.Counter[TagGolangCheck] != 1 { + value := cc.GolangBinary() + if cc.Counter[TagGolangBinary] != 1 { t.Fatal("did not increment") } + if value != "go" { + t.Fatal("expected 'go', got", value) + } }) t.Run("LinuxReadGOVERSION", func(t *testing.T) { diff --git a/internal/cmd/buildtool/ios.go b/internal/cmd/buildtool/ios.go index 49cb8cbb54..e725e1c36a 100644 --- a/internal/cmd/buildtool/ios.go +++ b/internal/cmd/buildtool/ios.go @@ -53,7 +53,6 @@ func iosSubcommand() *cobra.Command { // iosBuildGomobile invokes the gomobile build. func iosBuildGomobile(deps buildtoolmodel.Dependencies) { deps.PsiphonMaybeCopyConfigFiles() - deps.GolangCheck() config := &gomobileConfig{ deps: deps, diff --git a/internal/cmd/buildtool/ios_test.go b/internal/cmd/buildtool/ios_test.go index 1377704d09..931f44c40e 100644 --- a/internal/cmd/buildtool/ios_test.go +++ b/internal/cmd/buildtool/ios_test.go @@ -107,7 +107,7 @@ func TestIOSBuildGomobile(t *testing.T) { expectCalls := map[string]int{ buildtooltest.TagGOPATH: 1, - buildtooltest.TagGolangCheck: 1, + buildtooltest.TagGolangBinary: 1, buildtooltest.TagPsiphonMaybeCopyConfigFiles: 1, buildtooltest.TagPsiphonFilesExist: 1, } diff --git a/internal/cmd/buildtool/linuxstatic.go b/internal/cmd/buildtool/linuxstatic.go index d04cfe99f1..c98f0b5e8d 100644 --- a/internal/cmd/buildtool/linuxstatic.go +++ b/internal/cmd/buildtool/linuxstatic.go @@ -47,7 +47,6 @@ func (b *linuxStaticBuilder) main(*cobra.Command, []string) { // linuxStaticBuildAll builds all the packages on a linux-static environment. func linuxStaticBuilAll(deps buildtoolmodel.Dependencies, goarch string, goarm int64) { deps.PsiphonMaybeCopyConfigFiles() - deps.GolangCheck() // TODO(bassosimone): I am running the container with the right userID but // apparently this is not enough to make git happy--why? @@ -73,7 +72,7 @@ func linuxStaticBuildPackage( ooniArch := linuxStaticBuildOONIArch(goarch, goarm) - argv := runtimex.Try1(shellx.NewArgv("go", "build")) + argv := runtimex.Try1(shellx.NewArgv(deps.GolangBinary(), "build")) if deps.PsiphonFilesExist() { argv.Append("-tags", "ooni_psiphon_config") } diff --git a/internal/cmd/buildtool/linuxstatic_test.go b/internal/cmd/buildtool/linuxstatic_test.go index 58bfbc2d08..0b18194ff2 100644 --- a/internal/cmd/buildtool/linuxstatic_test.go +++ b/internal/cmd/buildtool/linuxstatic_test.go @@ -198,7 +198,7 @@ func TestLinuxStaticBuildAll(t *testing.T) { }) expectCalls := map[string]int{ - buildtooltest.TagGolangCheck: 1, + buildtooltest.TagGolangBinary: 1, buildtooltest.TagPsiphonMaybeCopyConfigFiles: 1, buildtooltest.TagPsiphonFilesExist: 2, } diff --git a/internal/cmd/buildtool/oohelperd.go b/internal/cmd/buildtool/oohelperd.go index 12d9110e85..d6384f71b2 100644 --- a/internal/cmd/buildtool/oohelperd.go +++ b/internal/cmd/buildtool/oohelperd.go @@ -42,8 +42,6 @@ func oohelperdSubcommand() *cobra.Command { // oohelperdBuildAndMaybeDeploy builds oohelperd for linux/amd64 and // possibly deploys the build to the 0.th.ooni.org server. func oohelperdBuildAndMaybeDeploy(deps buildtoolmodel.Dependencies, deploy bool) { - deps.GolangCheck() - log.Info("building oohelperd for linux/amd64") oohelperdBinary := filepath.Join("CLI", "oohelperd-linux-amd64") @@ -52,7 +50,7 @@ func oohelperdBuildAndMaybeDeploy(deps buildtoolmodel.Dependencies, deploy bool) envp.Append("GOOS", "linux") envp.Append("GOARCH", "amd64") - argv := runtimex.Try1(shellx.NewArgv("go", "build")) + argv := runtimex.Try1(shellx.NewArgv(deps.GolangBinary(), "build")) argv.Append("-o", oohelperdBinary) argv.Append("-tags", "netgo") argv.Append("-ldflags", "-s -w -extldflags -static") diff --git a/internal/cmd/buildtool/oohelperd_test.go b/internal/cmd/buildtool/oohelperd_test.go index 900d51d2ee..5c89fafa10 100644 --- a/internal/cmd/buildtool/oohelperd_test.go +++ b/internal/cmd/buildtool/oohelperd_test.go @@ -79,7 +79,7 @@ func TestOohelperdBuildAndMaybeDeploy(t *testing.T) { }) expectCalls := map[string]int{ - buildtooltest.TagGolangCheck: 1, + buildtooltest.TagGolangBinary: 1, } if diff := cmp.Diff(expectCalls, deps.Counter); diff != "" { diff --git a/internal/cmd/buildtool/windows.go b/internal/cmd/buildtool/windows.go index db6f0bca59..2550fb2eae 100644 --- a/internal/cmd/buildtool/windows.go +++ b/internal/cmd/buildtool/windows.go @@ -32,7 +32,6 @@ func windowsSubcommand() *cobra.Command { // 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} @@ -48,7 +47,7 @@ func windowsBuildAll(deps buildtoolmodel.Dependencies) { 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")) + argv := runtimex.Try1(shellx.NewArgv(deps.GolangBinary(), "build")) if deps.PsiphonFilesExist() { argv.Append("-tags", "ooni_psiphon_config") } diff --git a/internal/cmd/buildtool/windows_test.go b/internal/cmd/buildtool/windows_test.go index f11e6ac45d..fb40ac3be1 100644 --- a/internal/cmd/buildtool/windows_test.go +++ b/internal/cmd/buildtool/windows_test.go @@ -138,7 +138,7 @@ func TestWindowsBuildAll(t *testing.T) { }) expectCalls := map[string]int{ - buildtooltest.TagGolangCheck: 1, + buildtooltest.TagGolangBinary: 1, buildtooltest.TagPsiphonMaybeCopyConfigFiles: 1, buildtooltest.TagPsiphonFilesExist: 4, buildtooltest.TagWindowsMingwCheck: 1,