From eddc81ff04172606678c198cff7fe9db71aeb5e3 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Wed, 27 May 2020 17:10:44 -0400 Subject: [PATCH] go/tools/bazel_testing: provide a way to configure C toolchain in tests (#2509) go_bazel_tests that require cgo have been failing on Windows since the test environment hasn't been passing additional flags needed to configure the C toolchain on Windows. This CL adds GO_BAZEL_TEST_BAZELFLAGS, an environment variable to be set with --test_env, which provides a list of additional flags to Bazel. Fixes #2507 --- .bazelci/presubmit.yml | 46 +++++++------------------ go/tools/bazel_testing/bazel_testing.go | 14 +++++++- go/tools/builders/stdlib.go | 6 ++++ proto/compiler.bzl | 5 +++ 4 files changed, 36 insertions(+), 35 deletions(-) diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 53be88da71..d6911a921a 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -50,7 +50,7 @@ platforms: # gcc or clang. The Go SDK does not support MSVC. - "--cpu=x64_windows" - "--compiler=mingw-gcc" - - '--action_env=PATH=PATH=C:\tools\msys64\usr\bin;C:\tools\msys64\bin;C:\tools\msys64\mingw64\bin;C:\python3\Scripts\;C:\python3;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\ProgramData\GooGet;C:\Program Files\Google\Compute Engine\metadata_scripts;C:\Program Files (x86)\Google\Cloud SDK\google-cloud-sdk\bin;C:\Program Files\Google\Compute Engine\sysprep;C:\ProgramData\chocolatey\bin;C:\Program Files\Git\cmd;C:\tools\msys64\usr\bin;c:\openjdk\bin;C:\Program Files (x86)\Windows Kits\8.1\Windows Performance Toolkit\;C:\Program Files\CMake\bin;c:\ninja;c:\bazel;c:\buildkite' + - '--action_env=PATH=C:\tools\msys64\usr\bin;C:\tools\msys64\bin;C:\tools\msys64\mingw64\bin;C:\python3\Scripts\;C:\python3;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\ProgramData\GooGet;C:\Program Files\Google\Compute Engine\metadata_scripts;C:\Program Files (x86)\Google\Cloud SDK\google-cloud-sdk\bin;C:\Program Files\Google\Compute Engine\sysprep;C:\ProgramData\chocolatey\bin;C:\Program Files\Git\cmd;C:\tools\msys64\usr\bin;c:\openjdk\bin;C:\Program Files (x86)\Windows Kits\8.1\Windows Performance Toolkit\;C:\Program Files\CMake\bin;c:\ninja;c:\bazel;c:\buildkite' # NOTE(bazelbuild/bazel#10529): bazel doesn't register the mingw toolchain automatically. # We also need the host and target platforms to have the mingw constraint value. - "--extra_toolchains=@local_config_cc//:cc-toolchain-x64_windows_mingw" @@ -67,6 +67,7 @@ platforms: # TODO(#1789): go_path tests that require symlinks fail. These should # be skipped automatically. # TODO(#1790): Tests that require data should use bazel.Runfile. + # TODO(#2516): Tests that require protoc fail when protoc is built with mingw-gcc. - "--" - "..." - "-@com_github_golang_protobuf//ptypes:go_default_library_gen" @@ -156,9 +157,7 @@ platforms: - "-//tests/core/cgo:dylib_test" - "-//tests/core/cgo:versioned_dylib_client" - "-//tests/core/cgo:versioned_dylib_test" - - "-//tests/core/coverage:coverage_test" - - "-//tests/core/cross:cross_test" - - "-//tests/core/go_binary:go_default_test" + - "-//tests/core/cross:proto_test" - "-//tests/core/go_embed_data:go_default_test" - "-//tests/core/go_embed_data:go_default_library" - "-//tests/core/go_embed_data:unpack" @@ -186,18 +185,10 @@ platforms: - "-//tests/core/go_plugin:go_plugin" - "-//tests/core/go_plugin:go_default_test" - "-//tests/core/go_plugin:plugin" - - "-//tests/core/go_proto_library:bar_go_proto" - - "-//tests/core/go_proto_library:bar_proto" - - "-//tests/core/go_proto_library:embed_go_proto" - - "-//tests/core/go_proto_library:embed_test" - - "-//tests/core/go_proto_library:foo_go_proto" - - "-//tests/core/go_proto_library:foo_proto" + - "-//tests/core/go_proto_library:all" - "-//tests/core/go_proto_library_importmap:foo_go_proto" - "-//tests/core/go_proto_library_importmap:foo_proto" - "-//tests/core/go_proto_library_importmap:importmap_test" - - "-//tests/core/go_proto_library:transitive_go_proto" - - "-//tests/core/go_proto_library:transitive_test" - - "-//tests/core/go_proto_library:wrap_lib" - "-//tests/core/go_test:data_test" - "-//tests/core/go_test:pwd_test" - "-//tests/core/race:race_test" @@ -250,7 +241,7 @@ platforms: # gcc or clang. The Go SDK does not support MSVC. - "--cpu=x64_windows" - "--compiler=mingw-gcc" - - '--action_env=PATH=PATH=C:\tools\msys64\usr\bin;C:\tools\msys64\bin;C:\tools\msys64\mingw64\bin;C:\python3\Scripts\;C:\python3;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\ProgramData\GooGet;C:\Program Files\Google\Compute Engine\metadata_scripts;C:\Program Files (x86)\Google\Cloud SDK\google-cloud-sdk\bin;C:\Program Files\Google\Compute Engine\sysprep;C:\ProgramData\chocolatey\bin;C:\Program Files\Git\cmd;C:\tools\msys64\usr\bin;c:\openjdk\bin;C:\Program Files (x86)\Windows Kits\8.1\Windows Performance Toolkit\;C:\Program Files\CMake\bin;c:\ninja;c:\bazel;c:\buildkite' + - '--action_env=PATH=C:\tools\msys64\usr\bin;C:\tools\msys64\bin;C:\tools\msys64\mingw64\bin;C:\python3\Scripts\;C:\python3;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\ProgramData\GooGet;C:\Program Files\Google\Compute Engine\metadata_scripts;C:\Program Files (x86)\Google\Cloud SDK\google-cloud-sdk\bin;C:\Program Files\Google\Compute Engine\sysprep;C:\ProgramData\chocolatey\bin;C:\Program Files\Git\cmd;C:\tools\msys64\usr\bin;c:\openjdk\bin;C:\Program Files (x86)\Windows Kits\8.1\Windows Performance Toolkit\;C:\Program Files\CMake\bin;c:\ninja;c:\bazel;c:\buildkite' - "--extra_toolchains=@local_config_cc//:cc-toolchain-x64_windows_mingw" - "--host_platform=@io_bazel_rules_go//go/toolchain:windows_amd64_cgo" - "--platforms=@io_bazel_rules_go//go/toolchain:windows_amd64_cgo" @@ -258,6 +249,9 @@ platforms: # On Windows CI, bazel (bazelisk) needs %LocalAppData% to find the cache directory. # We invoke bazel in tests, so the tests need this, too. - "--test_env=LOCALAPPDATA" + # go_bazel_test runs bazel in a test workspace. It needs the same flags as above. + - "--test_env=GO_BAZEL_TEST_BAZELFLAGS=--cpu=x64_windows --compiler=mingw-gcc --extra_toolchains=@local_config_cc//:cc-toolchain-x64_windows_mingw --action_env=PATH --host_platform=@io_bazel_rules_go//go/toolchain:windows_amd64_cgo --incompatible_enable_cc_toolchain_resolution" + - "--test_env=PATH" test_targets: - "--" - "..." @@ -273,25 +267,13 @@ platforms: - "-@org_golang_x_tools//go/packages/packagestest:go_default_test" - "-@test_chdir_remote//sub:go_default_test" - "-//tests:buildifier_test" # requires bash - - "-//tests/core/cgo/objc:objc_test" - - "-//tests/core/cgo:race_test" - - "-//tests/core/cgo:opts_test" - - "-//tests/core/cgo:opts" - "-//tests/core/cgo:dylib_test" - "-//tests/core/cgo:dylib_client" - "-//tests/core/cgo:versioned_dylib_test" - "-//tests/core/cgo:versioned_dylib_client" - - "-//tests/core/cgo:cc_libs_test" - - "-//tests/core/cgo:pure" - - "-//tests/core/cgo:cc_srcs" - - "-//tests/core/cgo:cc_deps" - - "-//tests/core/cgo:c_srcs" - - "-//tests/core/cgo:bar_dep" - - "-//tests/core/cgo:tag_test" # TODO(#2031): FindBinary does not work - "-//tests/core/coverage:coverage_test" - - "-//tests/core/cross:cross_test" + - "-//tests/core/cross:proto_test" - "-//tests/core/go_binary:go_default_test" - - "-//tests/core/go_binary:stamp_test" - "-//tests/core/go_embed_data:go_default_test" - "-//tests/core/go_embed_data:go_default_test" - "-//tests/core/go_embed_data:go_default_library" @@ -312,20 +294,17 @@ platforms: - "-//tests/core/go_plugin:go_plugin" - "-//tests/core/go_plugin:go_default_test" - "-//tests/core/go_plugin:plugin" - - "-//tests/core/go_proto_library:embed_test" + - "-//tests/core/go_proto_library:all" - "-//tests/core/go_proto_library_importmap:importmap_test" - - "-//tests/core/go_proto_library:transitive_test" - "-//tests/core/go_test:data_test" - "-//tests/core/go_test:pwd_test" - - "-//tests/core/nogo/config:config_test" - "-//tests/core/nogo/coverage:coverage_test" - - "-//tests/core/nogo/vet:vet_test" - - "-//tests/core/race:race_test" - "-//tests/core/stdlib:buildid_test" - "-//tests/examples/executable_name:executable_name" - - "-//tests/integration/gazelle:gazelle_test" + - "-//tests/integration/gazelle:gazelle_test" # exceeds command line length limit - "-//tests/integration/googleapis:color_service_test" - "-//tests/integration/reproducibility:reproducibility_test" + - "-//tests/legacy/cgo_pthread_flag:go_default_test" # fails without error, passes locally. Problem with CI msys2? - "-//tests/legacy/examples/cgo/example_command:example_command_test" - "-//tests/legacy/examples/cgo/example_command:example_command_script" - "-//tests/legacy/examples/cgo/example_command:example_command" @@ -335,7 +314,6 @@ platforms: - "-//tests/legacy/examples/cgo/cc_dependency:version" - "-//tests/legacy/examples/cgo/cc_dependency:c_version_so" - "-//tests/legacy/examples/cgo/cc_dependency:c_version_orig" - - "-//tests/legacy/examples/cgo:sub" - "-//tests/legacy/examples/proto/gogo:gogo_test" - "-//tests/legacy/examples/proto:proto_pure_test" - "-//tests/legacy/examples/proto:proto_test" diff --git a/go/tools/bazel_testing/bazel_testing.go b/go/tools/bazel_testing/bazel_testing.go index 58868ee38d..ba6c769920 100644 --- a/go/tools/bazel_testing/bazel_testing.go +++ b/go/tools/bazel_testing/bazel_testing.go @@ -301,11 +301,23 @@ func setupWorkspace(args Args, files []string) (dir string, cleanup func() error } cleanups = append(cleanups, func() error { return os.RemoveAll(execDir) }) - // Extract test files for the main workspace. + // Create the workspace directory. mainDir := filepath.Join(execDir, "main") if err := os.MkdirAll(mainDir, 0777); err != nil { return "", cleanup, err } + + // Create a .bazelrc file if GO_BAZEL_TEST_BAZELFLAGS is set. + // The test can override this with its own .bazelrc or with flags in commands. + if flags := os.Getenv("GO_BAZEL_TEST_BAZELFLAGS"); flags != "" { + bazelrcPath := filepath.Join(mainDir, ".bazelrc") + content := "build " + flags + if err := ioutil.WriteFile(bazelrcPath, []byte(content), 0666); err != nil { + return "", cleanup, err + } + } + + // Extract test files for the main workspace. if err := extractTxtar(mainDir, args.Main); err != nil { return "", cleanup, fmt.Errorf("building main workspace: %v", err) } diff --git a/go/tools/builders/stdlib.go b/go/tools/builders/stdlib.go index b962256358..fd21cc3b89 100644 --- a/go/tools/builders/stdlib.go +++ b/go/tools/builders/stdlib.go @@ -44,6 +44,12 @@ func stdlib(args []string) error { } output := abs(*out) + // Fail fast if cgo is required but a toolchain is not configured. + if os.Getenv("CGO_ENABLED") == "1" && filepath.Base(os.Getenv("CC")) == "vc_installation_error.bat" { + return fmt.Errorf(`cgo is required, but a C toolchain has not been configured. +You may need to use the flags --cpu=x64_windows --compiler=mingw-gcc.`) + } + // Link in the bare minimum needed to the new GOROOT if err := replicate(goroot, output, replicatePaths("src", "pkg/tool", "pkg/include")); err != nil { return err diff --git a/proto/compiler.bzl b/proto/compiler.bzl index ae7c7bbd95..1544ea214d 100644 --- a/proto/compiler.bzl +++ b/proto/compiler.bzl @@ -100,6 +100,11 @@ def go_proto_compile(go, compiler, protos, imports, importpath): executable = compiler.go_protoc, arguments = [args], env = go.env, + # We may need the shell environment (potentially augmented with --action_env) + # to invoke protoc on Windows. If protoc was built with mingw, it probably needs + # .dll files in non-default locations that must be in PATH. The target configuration + # may not have a C compiler, so we have no idea what PATH should be. + use_default_shell_env = "PATH" not in go.env, ) return go_srcs