Skip to content

Commit

Permalink
go/tools/bazel_testing: provide a way to configure C toolchain in tes…
Browse files Browse the repository at this point in the history
…ts (#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
  • Loading branch information
Jay Conrod committed May 27, 2020
1 parent 92e2397 commit d41df07
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 35 deletions.
46 changes: 12 additions & 34 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -250,14 +241,17 @@ 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"
- "--incompatible_enable_cc_toolchain_resolution"
# 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:
- "--"
- "..."
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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"
Expand Down
14 changes: 13 additions & 1 deletion go/tools/bazel_testing/bazel_testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
6 changes: 6 additions & 0 deletions go/tools/builders/stdlib.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions proto/compiler.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit d41df07

Please sign in to comment.