From d540b4c088b31e92947914faed88e63520c1eb76 Mon Sep 17 00:00:00 2001 From: Yijie Ma Date: Mon, 21 Aug 2023 16:20:55 -0700 Subject: [PATCH] [Windows] Make resolver_component_tests_runner_invoker run with Bazel on Windows (#34107) Local Bazel invocation succeeds: ``` C:\Users\yijiem\projects\grpc>bazel --output_base=C:\bazel2 test --dynamic_mode=off --verbose_failures //test/cpp/naming:resolver_component_tests_runner_invoker@poller=epoll1 INFO: Analyzed target //test/cpp/naming:resolver_component_tests_runner_invoker@poller=epoll1 (0 packages loaded, 0 targets configured). INFO: Found 1 test target... Target //test/cpp/naming:resolver_component_tests_runner_invoker@poller=epoll1 up-to-date: bazel-bin/test/cpp/naming/resolver_component_tests_runner_invoker@poller=epoll1.exe INFO: Elapsed time: 199.262s, Critical Path: 193.48s INFO: 2 processes: 1 internal, 1 local. INFO: Build completed successfully, 2 total actions //test/cpp/naming:resolver_component_tests_runner_invoker@poller=epoll1 PASSED in 193.4s Executed 1 out of 1 test: 1 test passes. ``` The local invocation of RBE failed with linker error `LINK : error LNK2001: unresolved external symbol mainCRTStartup`, but that does not limited to this target: https://gist.github.com/yijiem/2c6cbd9a31209a6de8fd711afbf2b479. --- .../resolver_component_tests_defs.include | 2 +- test/cpp/naming/BUILD | 5 ++ .../generate_resolver_component_tests.bzl | 4 +- .../naming/resolver_component_tests_runner.py | 2 +- ...resolver_component_tests_runner_invoker.cc | 82 ++++++++++--------- test/cpp/util/windows/BUILD | 39 +++++++++ test/cpp/util/windows/manifest_file.cc | 71 ++++++++++++++++ test/cpp/util/windows/manifest_file.h | 57 +++++++++++++ 8 files changed, 221 insertions(+), 41 deletions(-) create mode 100644 test/cpp/util/windows/BUILD create mode 100644 test/cpp/util/windows/manifest_file.cc create mode 100644 test/cpp/util/windows/manifest_file.h diff --git a/templates/test/cpp/naming/resolver_component_tests_defs.include b/templates/test/cpp/naming/resolver_component_tests_defs.include index 6877f6a5b09..019686b2b64 100644 --- a/templates/test/cpp/naming/resolver_component_tests_defs.include +++ b/templates/test/cpp/naming/resolver_component_tests_defs.include @@ -47,7 +47,7 @@ def test_runner_log(msg): sys.stderr.write('\n%s: %s\n' % (__file__, msg)) def python_args(arg_list): - if platform.system() == 'Windows': + if platform.system() == 'Windows' and arg_list[0].endswith('.py'): return [sys.executable] + arg_list return arg_list diff --git a/test/cpp/naming/BUILD b/test/cpp/naming/BUILD index 13e08d98fba..24d193c7edd 100644 --- a/test/cpp/naming/BUILD +++ b/test/cpp/naming/BUILD @@ -54,4 +54,9 @@ grpc_cc_test( ], ) +filegroup( + name = "resolver_test_record_groups", + srcs = ["resolver_test_record_groups.yaml"], +) + generate_resolver_component_tests() diff --git a/test/cpp/naming/generate_resolver_component_tests.bzl b/test/cpp/naming/generate_resolver_component_tests.bzl index a91bc32649b..67c9067690b 100755 --- a/test/cpp/naming/generate_resolver_component_tests.bzl +++ b/test/cpp/naming/generate_resolver_component_tests.bzl @@ -75,6 +75,7 @@ def generate_resolver_component_tests(): ], external_deps = [ "absl/flags:flag", + "absl/strings", ], deps = [ "//test/cpp/util:test_util%s" % unsecure_build_config_suffix, @@ -83,6 +84,7 @@ def generate_resolver_component_tests(): "//:grpc%s" % unsecure_build_config_suffix, "//:gpr", "//test/cpp/util:test_config", + "//test/cpp/util/windows:manifest_file", ], data = [ ":resolver_component_tests_runner", @@ -90,7 +92,7 @@ def generate_resolver_component_tests(): "//test/cpp/naming/utils:dns_server", "//test/cpp/naming/utils:dns_resolver", "//test/cpp/naming/utils:tcp_connect", - "resolver_test_record_groups.yaml", # include the transitive dependency so that the dns server py binary can locate this + "//test/cpp/naming:resolver_test_record_groups", # include the transitive dependency so that the dns server py binary can locate this ], args = [ "--test_bin_name=resolver_component_test%s" % unsecure_build_config_suffix, diff --git a/test/cpp/naming/resolver_component_tests_runner.py b/test/cpp/naming/resolver_component_tests_runner.py index cf648ac1614..f6a32a646c4 100755 --- a/test/cpp/naming/resolver_component_tests_runner.py +++ b/test/cpp/naming/resolver_component_tests_runner.py @@ -47,7 +47,7 @@ def test_runner_log(msg): sys.stderr.write('\n%s: %s\n' % (__file__, msg)) def python_args(arg_list): - if platform.system() == 'Windows': + if platform.system() == 'Windows' and arg_list[0].endswith('.py'): return [sys.executable] + arg_list return arg_list diff --git a/test/cpp/naming/resolver_component_tests_runner_invoker.cc b/test/cpp/naming/resolver_component_tests_runner_invoker.cc index 1415e3ea0bf..549c7cc5961 100644 --- a/test/cpp/naming/resolver_component_tests_runner_invoker.cc +++ b/test/cpp/naming/resolver_component_tests_runner_invoker.cc @@ -1,6 +1,4 @@ -// -// -// Copyright 2017 gRPC authors. +// Copyright 2017 The gRPC Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -13,13 +11,17 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -// -// + +#include #include #include + +#ifndef GPR_WINDOWS #include +#endif // GPR_WINDOWS +#include #include #include #include @@ -43,6 +45,9 @@ #include "test/core/util/test_config.h" #include "test/cpp/util/subprocess.h" #include "test/cpp/util/test_config.h" +#ifdef GPR_WINDOWS +#include "test/cpp/util/windows/manifest_file.h" +#endif // GPR_WINDOWS ABSL_FLAG( bool, running_under_bazel, false, @@ -68,48 +73,22 @@ namespace grpc { namespace testing { -void InvokeResolverComponentTestsRunner( +int InvokeResolverComponentTestsRunner( std::string test_runner_bin_path, const std::string& test_bin_path, const std::string& dns_server_bin_path, const std::string& records_config_path, const std::string& dns_resolver_bin_path, const std::string& tcp_connect_bin_path) { int dns_server_port = grpc_pick_unused_port_or_die(); - - SubProcess* test_driver = new SubProcess( + auto test_driver = std::make_unique(std::vector( {std::move(test_runner_bin_path), "--test_bin_path=" + test_bin_path, "--dns_server_bin_path=" + dns_server_bin_path, "--records_config_path=" + records_config_path, "--dns_server_port=" + std::to_string(dns_server_port), "--dns_resolver_bin_path=" + dns_resolver_bin_path, "--tcp_connect_bin_path=" + tcp_connect_bin_path, - "--extra_args=" + absl::GetFlag(FLAGS_extra_args)}); - gpr_mu test_driver_mu; - gpr_mu_init(&test_driver_mu); - gpr_cv test_driver_cv; - gpr_cv_init(&test_driver_cv); - int status = test_driver->Join(); - if (WIFEXITED(status)) { - if (WEXITSTATUS(status)) { - grpc_core::Crash(absl::StrFormat( - "Resolver component test test-runner exited with code %d", - WEXITSTATUS(status))); - } - } else if (WIFSIGNALED(status)) { - grpc_core::Crash(absl::StrFormat( - "Resolver component test test-runner ended from signal %d", - WTERMSIG(status))); - } else { - grpc_core::Crash(absl::StrFormat( - "Resolver component test test-runner ended with unknown status %d", - status)); - } - gpr_mu_lock(&test_driver_mu); - gpr_cv_signal(&test_driver_cv); - gpr_mu_unlock(&test_driver_mu); - delete test_driver; - gpr_mu_destroy(&test_driver_mu); - gpr_cv_destroy(&test_driver_cv); + "--extra_args=" + absl::GetFlag(FLAGS_extra_args)})); + return test_driver->Join(); } } // namespace testing @@ -122,12 +101,14 @@ int main(int argc, char** argv) { grpc_init(); GPR_ASSERT(!absl::GetFlag(FLAGS_test_bin_name).empty()); std::string my_bin = argv[0]; + int result = 0; if (absl::GetFlag(FLAGS_running_under_bazel)) { GPR_ASSERT(!absl::GetFlag(FLAGS_grpc_test_directory_relative_to_test_srcdir) .empty()); // Use bazel's TEST_SRCDIR environment variable to locate the "test data" // binaries. auto test_srcdir = grpc_core::GetEnv("TEST_SRCDIR"); +#ifndef GPR_WINDOWS std::string const bin_dir = test_srcdir.value() + absl::GetFlag(FLAGS_grpc_test_directory_relative_to_test_srcdir) + @@ -135,18 +116,43 @@ int main(int argc, char** argv) { // Invoke bazel's executeable links to the .sh and .py scripts (don't use // the .sh and .py suffixes) to make // sure that we're using bazel's test environment. - grpc::testing::InvokeResolverComponentTestsRunner( + result = grpc::testing::InvokeResolverComponentTestsRunner( bin_dir + "/resolver_component_tests_runner", bin_dir + "/" + absl::GetFlag(FLAGS_test_bin_name), bin_dir + "/utils/dns_server", bin_dir + "/resolver_test_record_groups.yaml", bin_dir + "/utils/dns_resolver", bin_dir + "/utils/tcp_connect"); +#else + grpc::testing::ManifestFile manifest_file( + grpc::testing::NormalizeFilePath(test_srcdir.value()) + "\\MANIFEST"); + grpc::testing::InvokeResolverComponentTestsRunner( + grpc::testing::NormalizeFilePath( + manifest_file.Get("com_github_grpc_grpc/test/cpp/naming/" + "resolver_component_tests_runner.exe")), + grpc::testing::NormalizeFilePath( + manifest_file.Get("com_github_grpc_grpc/test/cpp/naming/" + + absl::GetFlag(FLAGS_test_bin_name) + ".exe")), + grpc::testing::NormalizeFilePath(manifest_file.Get( + "com_github_grpc_grpc/test/cpp/naming/utils/dns_server.exe")), + grpc::testing::NormalizeFilePath( + manifest_file.Get("com_github_grpc_grpc/test/cpp/naming/" + "resolver_test_record_groups.yaml")), + grpc::testing::NormalizeFilePath(manifest_file.Get( + "com_github_grpc_grpc/test/cpp/naming/utils/dns_resolver.exe")), + grpc::testing::NormalizeFilePath(manifest_file.Get( + "com_github_grpc_grpc/test/cpp/naming/utils/tcp_connect.exe"))); +#endif // GPR_WINDOWS } else { +#ifdef GPR_WINDOWS + grpc_core::Crash( + "Resolver component tests runner invoker does not support running " + "without Bazel on Windows for now."); +#endif // GPR_WINDOWS // Get the current binary's directory relative to repo root to invoke the // correct build config (asan/tsan/dbg, etc.). std::string const bin_dir = my_bin.substr(0, my_bin.rfind('/')); // Invoke the .sh and .py scripts directly where they are in source code. - grpc::testing::InvokeResolverComponentTestsRunner( + result = grpc::testing::InvokeResolverComponentTestsRunner( "test/cpp/naming/resolver_component_tests_runner.py", bin_dir + "/" + absl::GetFlag(FLAGS_test_bin_name), "test/cpp/naming/utils/dns_server.py", @@ -155,5 +161,5 @@ int main(int argc, char** argv) { "test/cpp/naming/utils/tcp_connect.py"); } grpc_shutdown(); - return 0; + return result; } diff --git a/test/cpp/util/windows/BUILD b/test/cpp/util/windows/BUILD new file mode 100644 index 00000000000..ffd2352efd4 --- /dev/null +++ b/test/cpp/util/windows/BUILD @@ -0,0 +1,39 @@ +# Copyright 2023 The gRPC Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +load("//bazel:grpc_build_system.bzl", "grpc_cc_library", "grpc_package") + +licenses(["notice"]) + +grpc_package( + name = "test/cpp/util/windows", + visibility = "tests", +) + +grpc_cc_library( + name = "manifest_file", + testonly = True, + srcs = [ + "manifest_file.cc", + ], + hdrs = [ + "manifest_file.h", + ], + external_deps = [ + "absl/strings", + ], + deps = [ + "//:gpr", + ], +) diff --git a/test/cpp/util/windows/manifest_file.cc b/test/cpp/util/windows/manifest_file.cc new file mode 100644 index 00000000000..a1d4e77553a --- /dev/null +++ b/test/cpp/util/windows/manifest_file.cc @@ -0,0 +1,71 @@ +// Copyright 2023 The gRPC Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "test/cpp/util/windows/manifest_file.h" + +#ifdef GPR_WINDOWS + +#include +#include +#include + +#include "absl/strings/str_format.h" +#include "absl/strings/str_replace.h" +#include "absl/strings/str_split.h" + +#include + +#include "src/core/lib/gprpp/crash.h" + +namespace grpc { +namespace testing { + +std::string NormalizeFilePath(const std::string& filepath) { + return absl::StrReplaceAll(filepath, {{"/", "\\"}}); +} + +ManifestFile::ManifestFile(const std::string& filepath) + : filestream_(filepath, std::ios::in | std::ios::binary) { + if (!filestream_.is_open()) { + grpc_core::Crash(absl::StrFormat("Failed to open %s", filepath)); + } +} + +std::string ManifestFile::Get(const std::string& key) { + auto iter = cache_.find(key); + if (iter != cache_.end()) { + return iter->second; + } + do { + std::string line; + std::getline(filestream_, line); + if (!line.empty()) { + std::vector kv = absl::StrSplit(line, " "); + GPR_ASSERT(kv.size() == 2); + cache_.emplace(kv[0], kv[1]); + if (kv[0] == key) { + return kv[1]; + } + } + } while (!filestream_.eof() && !filestream_.fail()); + grpc_core::Crash( + absl::StrFormat("Failed to find key: %s in MANIFEST file", key)); +} + +} // namespace testing +} // namespace grpc + +#endif // GPR_WINDOWS diff --git a/test/cpp/util/windows/manifest_file.h b/test/cpp/util/windows/manifest_file.h new file mode 100644 index 00000000000..cca75e2734b --- /dev/null +++ b/test/cpp/util/windows/manifest_file.h @@ -0,0 +1,57 @@ +// Copyright 2023 The gRPC Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef GRPC_TEST_CPP_UTIL_WINDOWS_MANIFEST_FILE_H +#define GRPC_TEST_CPP_UTIL_WINDOWS_MANIFEST_FILE_H + +#include + +#ifdef GPR_WINDOWS + +#include +#include +#include + +namespace grpc { +namespace testing { + +std::string NormalizeFilePath(const std::string& filepath); + +// This class is used for handling Runfiles for a Bazel target on Windows (e.g. +// the output of targets specified in the data attribute of the target). On +// Linux/macOS, Bazel creates a runfiles tree which contains symlinks to the +// actual runfiles. But on Windows, it only creates a MANIFEST file which +// contains a list of . +// Thus one initializes a ManifestFile object with the filepath to a MANIFEST +// file and uses it as a key-value datastore by querying the absolute symlink +// target path with the imaginative symlink relative path. See +// https://github.com/bazelbuild/bazel/issues/4261#issuecomment-350723457 for +// more details. +class ManifestFile { + public: + explicit ManifestFile(const std::string& filepath); + + std::string Get(const std::string& key); + + private: + std::fstream filestream_; + std::unordered_map cache_; +}; + +} // namespace testing +} // namespace grpc + +#endif // GPR_WINDOWS + +#endif // GRPC_TEST_CPP_UTIL_WINDOWS_MANIFEST_FILE_H