From 88773c663c5405948520df7a1136a02d625ddb96 Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Thu, 4 Feb 2021 22:12:51 +0000 Subject: [PATCH 1/3] feat: fix compilation problems on MSVC Most of the problems were around `setenv()` and `getenv()`, which I solved by writing a small wrappers. Kept `std::getenv()` in the examples, because that is most familiar to C++ developers, had to turn off warnings from MSVC about it though. Avoid googletest regex because they are unsupported with MSVC Fix executable paths in integration tests --- .gitignore | 1 + examples/CMakeLists.txt | 6 ++ examples/site/testing_http/CMakeLists.txt | 4 ++ .../testing_http/http_integration_test.cc | 9 ++- .../testing_pubsub/pubsub_integration_test.cc | 5 +- .../storage_integration_test.cc | 5 +- examples/site_test.cc | 13 +++-- google/cloud/functions/CMakeLists.txt | 8 +++ .../basic_integration_test.cc | 22 +++++--- .../cloud_event_integration_test.cc | 28 +++++----- .../functions/internal/compiler_info_test.cc | 8 ++- .../functions/internal/framework_impl_test.cc | 9 ++- .../functions/internal/parse_options_test.cc | 30 +++++----- google/cloud/functions/internal/setenv.cc | 56 +++++++++++++++++++ google/cloud/functions/internal/setenv.h | 32 +++++++++++ 15 files changed, 183 insertions(+), 53 deletions(-) create mode 100644 google/cloud/functions/internal/setenv.cc create mode 100644 google/cloud/functions/internal/setenv.h diff --git a/.gitignore b/.gitignore index 97c95cf4..a72c91fb 100644 --- a/.gitignore +++ b/.gitignore @@ -15,4 +15,5 @@ cmake-build-*/ # Ignore Visual Studio Code files .vsbuild/ .vscode/ +build/ diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index 4ba87922..0e581241 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -51,6 +51,12 @@ add_library( site/tips_retry/tips_retry.cc site/tips_scopes/tips_scopes.cc) functions_framework_cpp_add_common_options(functions_framework_examples) +if (MSVC) + set_property( + SOURCE site/tips_gcp_apis/tips_gcp_apis.cc site/env_vars/env_vars.cc + APPEND + PROPERTY COMPILE_DEFINITIONS _CRT_SECURE_NO_WARNINGS) +endif () target_link_libraries( functions_framework_examples fmt::fmt functions-framework-cpp::framework google-cloud-cpp::pubsub google-cloud-cpp::storage) diff --git a/examples/site/testing_http/CMakeLists.txt b/examples/site/testing_http/CMakeLists.txt index e0ac043b..355e5f34 100644 --- a/examples/site/testing_http/CMakeLists.txt +++ b/examples/site/testing_http/CMakeLists.txt @@ -32,6 +32,10 @@ if (BUILD_TESTING) string(REPLACE ".cc" "" target "${target}") add_executable("${target}" ${fname}) functions_framework_cpp_add_common_options(${target}) + if (MSVC) + target_compile_definitions(${target} + PRIVATE "_CRT_SECURE_NO_WARNINGS") + endif () target_link_libraries( ${target} PRIVATE functions_framework_examples diff --git a/examples/site/testing_http/http_integration_test.cc b/examples/site/testing_http/http_integration_test.cc index 23c6d606..cb2b8884 100644 --- a/examples/site/testing_http/http_integration_test.cc +++ b/examples/site/testing_http/http_integration_test.cc @@ -38,6 +38,12 @@ HttpResponse HttpGet(std::string const& url, std::string const& payload); char const* argv0 = nullptr; +auto ExePath(bfs::path const& filename) { + static auto const kPath = std::vector{ + bfs::canonical(argv0).make_preferred().parent_path()}; + return bp::search_path(filename, kPath); +} + class HttpIntegrationTest : public ::testing::Test { protected: void SetUp() override { @@ -47,8 +53,7 @@ class HttpIntegrationTest : public ::testing::Test { return; } curl_global_init(CURL_GLOBAL_ALL); - auto const exe = bfs::path(argv0).parent_path() / "http_integration_server"; - auto server = bp::child(exe, "--port=8030"); + auto server = bp::child(ExePath("http_integration_server"), "--port=8030"); url_ = "http://localhost:8030"; ASSERT_TRUE(WaitForServerReady(url_)); process_ = std::move(server); diff --git a/examples/site/testing_pubsub/pubsub_integration_test.cc b/examples/site/testing_pubsub/pubsub_integration_test.cc index 9908626c..ffae889a 100644 --- a/examples/site/testing_pubsub/pubsub_integration_test.cc +++ b/examples/site/testing_pubsub/pubsub_integration_test.cc @@ -42,8 +42,9 @@ class PubsubIntegrationTest : public ::testing::Test { void SetUp() override { curl_global_init(CURL_GLOBAL_ALL); - auto const exe = - bfs::path(argv0).parent_path() / "pubsub_integration_server"; + static auto const kPath = std::vector{ + bfs::canonical(argv0).make_preferred().parent_path()}; + auto const exe = bp::search_path("pubsub_integration_server", kPath); auto server = bp::child(exe, "--port=8040", (bp::std_out & bp::std_err) > child_log_); ASSERT_TRUE(WaitForServerReady("http://localhost:8040/")); diff --git a/examples/site/testing_storage/storage_integration_test.cc b/examples/site/testing_storage/storage_integration_test.cc index 2fe79244..d615ad66 100644 --- a/examples/site/testing_storage/storage_integration_test.cc +++ b/examples/site/testing_storage/storage_integration_test.cc @@ -43,8 +43,9 @@ class StorageIntegrationTest : public ::testing::Test { void SetUp() override { curl_global_init(CURL_GLOBAL_ALL); - auto const exe = - bfs::path(argv0).parent_path() / "storage_integration_server"; + static auto const kPath = std::vector{ + bfs::canonical(argv0).make_preferred().parent_path()}; + auto const exe = bp::search_path("storage_integration_server", kPath); auto server = bp::child(exe, "--port=8050", (bp::std_out & bp::std_err) > child_log_); ASSERT_TRUE(WaitForServerReady("http://localhost:8050/")); diff --git a/examples/site_test.cc b/examples/site_test.cc index eb6359ad..36338467 100644 --- a/examples/site_test.cc +++ b/examples/site_test.cc @@ -13,12 +13,12 @@ // limitations under the License. #include "google/cloud/functions/internal/parse_cloud_event_json.h" +#include "google/cloud/functions/internal/setenv.h" #include "google/cloud/functions/cloud_event.h" #include "google/cloud/functions/http_request.h" #include "google/cloud/functions/http_response.h" #include #include -#include // NOLINT - we need the POSIX header, for setenv. namespace gcf = ::google::cloud::functions; extern gcf::HttpResponse hello_world_content(gcf::HttpRequest request); @@ -286,11 +286,11 @@ TEST(ExamplesSiteTest, ConceptsStateless) { } TEST(ExamplesSiteTest, EnvVars) { - unsetenv("FOO"); + google::cloud::functions_internal::SetEnv("FOO", std::nullopt); auto actual = env_vars(gcf::HttpRequest{}); EXPECT_THAT(actual.payload(), AllOf(HasSubstr("FOO"), HasSubstr("not set"))); - setenv("FOO", "test-value", 1); + google::cloud::functions_internal::SetEnv("FOO", "test-value"); actual = env_vars(gcf::HttpRequest{}); EXPECT_THAT(actual.payload(), HasSubstr("test-value")); } @@ -309,16 +309,17 @@ TEST(ExamplesSiteTest, TipsGcpApis) { GTEST_SKIP(); #endif // __has_feature(thread_sanitizer) #endif // defined(__has_feature) - unsetenv("GCP_PROJECT"); + google::cloud::functions_internal::SetEnv("GCP_PROJECT", std::nullopt); EXPECT_THROW(tips_gcp_apis(gcf::HttpRequest{}), std::runtime_error); - setenv("GCP_PROJECT", "test-unused", 1); + google::cloud::functions_internal::SetEnv("GCP_PROJECT", "test-unused"); EXPECT_THROW(tips_gcp_apis(gcf::HttpRequest{}), std::exception); EXPECT_THROW( tips_gcp_apis(gcf::HttpRequest{}.set_payload(nlohmann::json({}).dump())), std::runtime_error); - setenv("PUBSUB_EMULATOR_HOST", "localhost:1", 1); + google::cloud::functions_internal::SetEnv("PUBSUB_EMULATOR_HOST", + "localhost:1"); auto const actual = tips_gcp_apis(gcf::HttpRequest{}.set_payload( nlohmann::json({{"topic", "test-unused"}}).dump())); EXPECT_EQ(actual.result(), gcf::HttpResponse::kInternalServerError); diff --git a/google/cloud/functions/CMakeLists.txt b/google/cloud/functions/CMakeLists.txt index 1e906b30..94566e62 100644 --- a/google/cloud/functions/CMakeLists.txt +++ b/google/cloud/functions/CMakeLists.txt @@ -47,6 +47,8 @@ add_library( internal/parse_cloud_event_json.h internal/parse_options.cc internal/parse_options.h + internal/setenv.cc + internal/setenv.h internal/version_info.h internal/wrap_request.cc internal/wrap_request.h @@ -56,6 +58,12 @@ add_library( version.cc version.h) functions_framework_cpp_add_common_options(functions_framework_cpp) +if (MSVC) + set_property( + SOURCE internal/parse_options.cc + APPEND + PROPERTY COMPILE_DEFINITIONS "_CRT_SECURE_NO_WARNINGS") +endif () target_include_directories(functions_framework_cpp PUBLIC $) target_include_directories(functions_framework_cpp SYSTEM diff --git a/google/cloud/functions/integration_tests/basic_integration_test.cc b/google/cloud/functions/integration_tests/basic_integration_test.cc index 905a4d65..db402f50 100644 --- a/google/cloud/functions/integration_tests/basic_integration_test.cc +++ b/google/cloud/functions/integration_tests/basic_integration_test.cc @@ -83,9 +83,14 @@ char const* argv0 = nullptr; auto constexpr kServer = "echo_server"; auto constexpr kConformanceServer = "http_conformance"; +auto ExePath(bfs::path const& filename) { + static auto const kPath = std::vector{ + bfs::canonical(argv0).make_preferred().parent_path()}; + return bp::search_path(filename, kPath); +} + TEST(RunIntegrationTest, Basic) { - auto const exe = bfs::path(argv0).parent_path() / kServer; - auto server = bp::child(exe, "--port=8010"); + auto server = bp::child(ExePath(kServer), "--port=8010"); auto result = WaitForServerReady("localhost", "8010"); ASSERT_EQ(result, 0); @@ -121,9 +126,9 @@ TEST(RunIntegrationTest, Basic) { } TEST(RunIntegrationTest, ExceptionLogsToStderr) { - auto const exe = bfs::path(argv0).parent_path() / kServer; bp::ipstream child_stderr; - auto server = bp::child(exe, "--port=8010", bp::std_err > child_stderr); + auto server = + bp::child(ExePath(kServer), "--port=8010", bp::std_err > child_stderr); auto result = WaitForServerReady("localhost", "8010"); ASSERT_EQ(result, 0); @@ -145,11 +150,11 @@ TEST(RunIntegrationTest, ExceptionLogsToStderr) { } TEST(RunIntegrationTest, OutputIsFlushed) { - auto const exe = bfs::path(argv0).parent_path() / kServer; bp::ipstream child_stderr; bp::ipstream child_stdout; - auto server = bp::child(exe, "--port=8010", bp::std_err > child_stderr, - bp::std_out > child_stdout); + auto server = + bp::child(ExePath(kServer), "--port=8010", bp::std_err > child_stderr, + bp::std_out > child_stdout); auto result = WaitForServerReady("localhost", "8010"); ASSERT_EQ(result, 0); @@ -176,8 +181,7 @@ TEST(RunIntegrationTest, OutputIsFlushed) { } TEST(RunIntegrationTest, ConformanceSmokeTest) { - auto const exe = bfs::path(argv0).parent_path() / kConformanceServer; - auto server = bp::child(exe, "--port=8010"); + auto server = bp::child(ExePath(kConformanceServer), "--port=8010"); auto result = WaitForServerReady("localhost", "8010"); ASSERT_EQ(result, 0); diff --git a/google/cloud/functions/integration_tests/cloud_event_integration_test.cc b/google/cloud/functions/integration_tests/cloud_event_integration_test.cc index 13ff772d..a2d703f3 100644 --- a/google/cloud/functions/integration_tests/cloud_event_integration_test.cc +++ b/google/cloud/functions/integration_tests/cloud_event_integration_test.cc @@ -170,9 +170,14 @@ char const* argv0 = nullptr; auto constexpr kServer = "cloud_event_handler"; auto constexpr kConformanceServer = "cloud_event_conformance"; +auto ExePath(bfs::path const& filename) { + static auto const kPath = std::vector{ + bfs::canonical(argv0).make_preferred().parent_path()}; + return bp::search_path(filename, kPath); +} + TEST(RunIntegrationTest, Basic) { - auto const exe = bfs::path(argv0).parent_path() / kServer; - auto server = bp::child(exe, "--port=8020"); + auto server = bp::child(ExePath(kServer), "--port=8020"); auto result = WaitForServerReady("localhost", "8020"); ASSERT_EQ(result, 0); @@ -202,8 +207,7 @@ TEST(RunIntegrationTest, Basic) { } TEST(RunIntegrationTest, Batch) { - auto const exe = bfs::path(argv0).parent_path() / kServer; - auto server = bp::child(exe, "--port=8020"); + auto server = bp::child(ExePath(kServer), "--port=8020"); auto result = WaitForServerReady("localhost", "8020"); ASSERT_EQ(result, 0); @@ -218,8 +222,7 @@ TEST(RunIntegrationTest, Batch) { } TEST(RunIntegrationTest, Binary) { - auto const exe = bfs::path(argv0).parent_path() / kServer; - auto server = bp::child(exe, "--port=8020"); + auto server = bp::child(ExePath(kServer), "--port=8020"); auto result = WaitForServerReady("localhost", "8020"); ASSERT_EQ(result, 0); @@ -234,9 +237,9 @@ TEST(RunIntegrationTest, Binary) { } TEST(RunIntegrationTest, ExceptionLogsToStderr) { - auto const exe = bfs::path(argv0).parent_path() / kServer; bp::ipstream child_stderr; - auto server = bp::child(exe, "--port=8020", bp::std_err > child_stderr); + auto server = + bp::child(ExePath(kServer), "--port=8020", bp::std_err > child_stderr); auto result = WaitForServerReady("localhost", "8020"); ASSERT_EQ(result, 0); @@ -258,11 +261,11 @@ TEST(RunIntegrationTest, ExceptionLogsToStderr) { } TEST(RunIntegrationTest, OutputIsFlushed) { - auto const exe = bfs::path(argv0).parent_path() / kServer; bp::ipstream child_stderr; bp::ipstream child_stdout; - auto server = bp::child(exe, "--port=8020", bp::std_err > child_stderr, - bp::std_out > child_stdout); + auto server = + bp::child(ExePath(kServer), "--port=8020", bp::std_err > child_stderr, + bp::std_out > child_stdout); auto result = WaitForServerReady("localhost", "8020"); ASSERT_EQ(result, 0); @@ -289,8 +292,7 @@ TEST(RunIntegrationTest, OutputIsFlushed) { } TEST(RunIntegrationTest, ConformanceSmokeTest) { - auto const exe = bfs::path(argv0).parent_path() / kConformanceServer; - auto server = bp::child(exe, "--port=8020"); + auto server = bp::child(ExePath(kConformanceServer), "--port=8020"); auto result = WaitForServerReady("localhost", "8020"); ASSERT_EQ(result, 0); diff --git a/google/cloud/functions/internal/compiler_info_test.cc b/google/cloud/functions/internal/compiler_info_test.cc index 70908c6d..a186b34b 100644 --- a/google/cloud/functions/internal/compiler_info_test.cc +++ b/google/cloud/functions/internal/compiler_info_test.cc @@ -14,11 +14,14 @@ #include "google/cloud/functions/internal/compiler_info.h" #include +#include namespace google::cloud::functions_internal { inline namespace FUNCTIONS_FRAMEWORK_CPP_NS { namespace { +using ::testing::Contains; + TEST(CompilerInfo, CompilerId) { auto const cn = CompilerId(); EXPECT_NE(cn, ""); @@ -31,7 +34,10 @@ TEST(CompilerInfo, CompilerVersion) { auto const cv = CompilerVersion(); EXPECT_NE(cv, ""); // Look for something that looks vaguely like an X.Y version number. - EXPECT_THAT(cv, ::testing::ContainsRegex(R"([0-9]+.[0-9]+)")); + // Cannot use a regex because GTest does not have them with MSVC. + EXPECT_THAT(cv, Contains('.')); + EXPECT_TRUE(std::regex_match(cv, std::regex(R"(^[0-9]+\.[0-9]+.*)"))) + << "version=" << cv; } TEST(CompilerInfo, LanguageVersion) { diff --git a/google/cloud/functions/internal/framework_impl_test.cc b/google/cloud/functions/internal/framework_impl_test.cc index ff66e029..c2c53baf 100644 --- a/google/cloud/functions/internal/framework_impl_test.cc +++ b/google/cloud/functions/internal/framework_impl_test.cc @@ -114,7 +114,8 @@ TEST(FrameworkTest, Http) { argc, argv, std::move(f), [&shutdown]() { return shutdown.load(); }, [&port_p](int port) mutable { port_p.set_value(port); }); }; - auto done = std::async(std::launch::async, run, kTestArgc, kTestArgv, hello); + auto done = std::async(std::launch::async, run, static_cast(kTestArgc), + kTestArgv, hello); auto port = port_f.get(); auto actual = HttpGet("localhost", std::to_string(port), "/say/hello"); @@ -146,7 +147,8 @@ TEST(FrameworkTest, CloudEvent) { argc, argv, std::move(f), [&shutdown]() { return shutdown.load(); }, [&port_p](int port) mutable { port_p.set_value(port); }); }; - auto done = std::async(std::launch::async, run, kTestArgc, kTestArgv, hello); + auto done = std::async(std::launch::async, run, static_cast(kTestArgc), + kTestArgv, hello); auto port = port_f.get(); auto actual = CloudEventGet("localhost", std::to_string(port), "/"); @@ -163,7 +165,8 @@ TEST(FrameworkTest, CloudEvent) { TEST(FrameworkTest, CloudEventInvalidPort) { auto const exit_code = ::google::cloud::functions::Run( - kTestInvalidArgc, kTestInvalidArgv, functions::UserCloudEventFunction{}); + static_cast(kTestInvalidArgc), kTestInvalidArgv, + functions::UserCloudEventFunction{}); EXPECT_NE(exit_code, 0); } diff --git a/google/cloud/functions/internal/parse_options_test.cc b/google/cloud/functions/internal/parse_options_test.cc index 545fd358..a90d9a73 100644 --- a/google/cloud/functions/internal/parse_options_test.cc +++ b/google/cloud/functions/internal/parse_options_test.cc @@ -13,16 +13,16 @@ // limitations under the License. #include "google/cloud/functions/internal/parse_options.h" +#include "google/cloud/functions/internal/setenv.h" #include -#include namespace google::cloud::functions_internal { inline namespace FUNCTIONS_FRAMEWORK_CPP_NS { namespace { TEST(WrapRequestTest, NoCmd) { - char const* argv[] = {}; - auto const vm = ParseOptions(sizeof(argv) / sizeof(argv[0]), argv); + char const* argv[] = {"unused"}; + auto const vm = ParseOptions(0, argv); EXPECT_EQ(vm.count("help"), 0); } @@ -48,56 +48,56 @@ TEST(WrapRequestTest, ExceptionOnUnknown) { } TEST(WrapRequestTest, AcceptsRequiredOptions) { - ::unsetenv("PORT"); + SetEnv("PORT", std::nullopt); char const* argv[] = { - "unused", "--port=8085", "--address=localhost", + "unused", "--port=7060", "--address=localhost", "--target=foo", "--signature-type=bar", }; auto const vm = ParseOptions(sizeof(argv) / sizeof(argv[0]), argv); EXPECT_EQ(vm.count("help"), 0); - EXPECT_EQ(vm["port"].as(), 8085); + EXPECT_EQ(vm["port"].as(), 7060); EXPECT_EQ(vm["address"].as(), "localhost"); EXPECT_EQ(vm["target"].as(), "foo"); EXPECT_EQ(vm["signature-type"].as(), "bar"); } TEST(WrapRequestTest, UseEnvForPort) { - ::setenv("PORT", "8081", 1); + SetEnv("PORT", "7070"); char const* argv[] = {"unused"}; auto const vm = ParseOptions(sizeof(argv) / sizeof(argv[0]), argv); - EXPECT_EQ(vm["port"].as(), 8081); + EXPECT_EQ(vm["port"].as(), 7070); } TEST(WrapRequestTest, CommandLineOverridesEnv) { - ::setenv("PORT", "8081", 1); - char const* argv[] = {"unused", "--port=8082"}; + SetEnv("PORT", "7070"); + char const* argv[] = {"unused", "--port=7080"}; auto const vm = ParseOptions(sizeof(argv) / sizeof(argv[0]), argv); - EXPECT_EQ(vm["port"].as(), 8082); + EXPECT_EQ(vm["port"].as(), 7080); } TEST(WrapRequestTest, PortEnvInvalid) { - ::setenv("PORT", "not-a-number", 1); + SetEnv("PORT", "not-a-number"); char const* argv[] = {"unused"}; int argc = sizeof(argv) / sizeof(argv[0]); EXPECT_THROW(ParseOptions(argc, argv), std::exception); } TEST(WrapRequestTest, PortEnvTooLow) { - ::setenv("PORT", "-1", 1); + SetEnv("PORT", "-1"); char const* argv[] = {"unused"}; int argc = sizeof(argv) / sizeof(argv[0]); EXPECT_THROW(ParseOptions(argc, argv), std::exception); } TEST(WrapRequestTest, PortEnvTooHigh) { - ::setenv("PORT", "65536", 1); + SetEnv("PORT", "65536"); char const* argv[] = {"unused"}; int argc = sizeof(argv) / sizeof(argv[0]); EXPECT_THROW(ParseOptions(argc, argv), std::exception); } TEST(WrapRequestTest, PortCommandLineInvalid) { - ::unsetenv("PORT"); + SetEnv("PORT", std::nullopt); char const* argv_1[] = {"unused", "--port=invalid-not-a-number"}; char const* argv_2[] = {"unused", "--port=-1"}; char const* argv_3[] = {"unused", "--port=65536"}; diff --git a/google/cloud/functions/internal/setenv.cc b/google/cloud/functions/internal/setenv.cc new file mode 100644 index 00000000..a8cf08fe --- /dev/null +++ b/google/cloud/functions/internal/setenv.cc @@ -0,0 +1,56 @@ +// Copyright 2021 Google LLC +// +// 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 "google/cloud/functions/internal/setenv.h" +#ifdef _WIN32 +// We need _putenv_s() +#include +#else +// On Unix-like systems we need setenv()/unsetenv(), which are defined here: +#include // NOLINT +#endif // _WIN32 + +namespace google::cloud::functions_internal { +inline namespace FUNCTIONS_FRAMEWORK_CPP_NS { + +namespace { +void UnsetEnv(char const* variable) { +#ifdef _WIN32 + (void)_putenv_s(variable, ""); +#else + unsetenv(variable); +#endif // _WIN32 +} + +void SetEnv(char const* variable, char const* value) { +#ifdef _WIN32 + (void)_putenv_s(variable, value); +#else + (void)setenv(variable, value, 1); +#endif // _WIN32 +} + +} // namespace + +void SetEnv(std::string const& variable, + std::optional const& value) { + if (!value.has_value()) { + UnsetEnv(variable.c_str()); + return; + } + SetEnv(variable.c_str(), value->c_str()); +} + +} // namespace FUNCTIONS_FRAMEWORK_CPP_NS +} // namespace google::cloud::functions_internal diff --git a/google/cloud/functions/internal/setenv.h b/google/cloud/functions/internal/setenv.h new file mode 100644 index 00000000..c271f0d2 --- /dev/null +++ b/google/cloud/functions/internal/setenv.h @@ -0,0 +1,32 @@ +// Copyright 2021 Google LLC +// +// 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 FUNCTIONS_FRAMEWORK_CPP_GOOGLE_CLOUD_FUNCTIONS_SETENV_H +#define FUNCTIONS_FRAMEWORK_CPP_GOOGLE_CLOUD_FUNCTIONS_SETENV_H + +#include "google/cloud/functions/version.h" +#include +#include + +namespace google::cloud::functions_internal { +inline namespace FUNCTIONS_FRAMEWORK_CPP_NS { + +/// Changes an environment variable in the current process. +void SetEnv(std::string const& variable, + std::optional const& value); + +} // namespace FUNCTIONS_FRAMEWORK_CPP_NS +} // namespace google::cloud::functions_internal + +#endif // FUNCTIONS_FRAMEWORK_CPP_GOOGLE_CLOUD_FUNCTIONS_SETENV_H From 9ca2a1caa4afc50f57155f4028e3878d3223455f Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Sun, 7 Feb 2021 23:12:04 +0000 Subject: [PATCH 2/3] Address review comments --- .../site/testing_pubsub/pubsub_integration_test.cc | 13 ++++++++----- .../testing_storage/storage_integration_test.cc | 12 ++++++++---- .../cloud/functions/internal/compiler_info_test.cc | 7 ++++--- google/cloud/functions/internal/setenv.cc | 11 ++++------- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/examples/site/testing_pubsub/pubsub_integration_test.cc b/examples/site/testing_pubsub/pubsub_integration_test.cc index ffae889a..d9364bb0 100644 --- a/examples/site/testing_pubsub/pubsub_integration_test.cc +++ b/examples/site/testing_pubsub/pubsub_integration_test.cc @@ -37,16 +37,19 @@ HttpResponse HttpEvent(std::string const& url, std::string const& payload); char const* argv0 = nullptr; +auto ExePath(bfs::path const& filename) { + static auto const kPath = std::vector{ + bfs::canonical(argv0).make_preferred().parent_path()}; + return bp::search_path(filename, kPath); +} + class PubsubIntegrationTest : public ::testing::Test { protected: void SetUp() override { curl_global_init(CURL_GLOBAL_ALL); - static auto const kPath = std::vector{ - bfs::canonical(argv0).make_preferred().parent_path()}; - auto const exe = bp::search_path("pubsub_integration_server", kPath); - auto server = - bp::child(exe, "--port=8040", (bp::std_out & bp::std_err) > child_log_); + auto server = bp::child(ExePath("pubsub_integration_server"), "--port=8040", + (bp::std_out & bp::std_err) > child_log_); ASSERT_TRUE(WaitForServerReady("http://localhost:8040/")); process_ = std::move(server); } diff --git a/examples/site/testing_storage/storage_integration_test.cc b/examples/site/testing_storage/storage_integration_test.cc index d615ad66..707a3be7 100644 --- a/examples/site/testing_storage/storage_integration_test.cc +++ b/examples/site/testing_storage/storage_integration_test.cc @@ -38,16 +38,20 @@ HttpResponse HttpEvent(std::string const& url, std::string const& payload); char const* argv0 = nullptr; +auto ExePath(bfs::path const& filename) { + static auto const kPath = std::vector{ + bfs::canonical(argv0).make_preferred().parent_path()}; + return bp::search_path(filename, kPath); +} + class StorageIntegrationTest : public ::testing::Test { protected: void SetUp() override { curl_global_init(CURL_GLOBAL_ALL); - static auto const kPath = std::vector{ - bfs::canonical(argv0).make_preferred().parent_path()}; - auto const exe = bp::search_path("storage_integration_server", kPath); auto server = - bp::child(exe, "--port=8050", (bp::std_out & bp::std_err) > child_log_); + bp::child(ExePath("storage_integration_server"), "--port=8050", + (bp::std_out & bp::std_err) > child_log_); ASSERT_TRUE(WaitForServerReady("http://localhost:8050/")); process_ = std::move(server); } diff --git a/google/cloud/functions/internal/compiler_info_test.cc b/google/cloud/functions/internal/compiler_info_test.cc index a186b34b..0e597f69 100644 --- a/google/cloud/functions/internal/compiler_info_test.cc +++ b/google/cloud/functions/internal/compiler_info_test.cc @@ -33,10 +33,11 @@ TEST(CompilerInfo, CompilerId) { TEST(CompilerInfo, CompilerVersion) { auto const cv = CompilerVersion(); EXPECT_NE(cv, ""); - // Look for something that looks vaguely like an X.Y version number. - // Cannot use a regex because GTest does not have them with MSVC. + // Look for something that looks vaguely like an X.Y version number. Cannot + // use ::testing::ContainsRegex() because these do not work when GTest is + // compiled under MSVC. EXPECT_THAT(cv, Contains('.')); - EXPECT_TRUE(std::regex_match(cv, std::regex(R"(^[0-9]+\.[0-9]+.*)"))) + EXPECT_TRUE(std::regex_search(cv, std::regex(R"([0-9]+\.[0-9]+)"))) << "version=" << cv; } diff --git a/google/cloud/functions/internal/setenv.cc b/google/cloud/functions/internal/setenv.cc index a8cf08fe..f7dbc087 100644 --- a/google/cloud/functions/internal/setenv.cc +++ b/google/cloud/functions/internal/setenv.cc @@ -13,13 +13,10 @@ // limitations under the License. #include "google/cloud/functions/internal/setenv.h" -#ifdef _WIN32 -// We need _putenv_s() -#include -#else -// On Unix-like systems we need setenv()/unsetenv(), which are defined here: -#include // NOLINT -#endif // _WIN32 +// We need _putenv_s() on WIN32 and setenv()/unsetenv() on Posix. clang-tidy +// recommends including . That seems wrong, is not guaranteed +// to define the Posix/WIN32 functions. +#include // NOLINT(modernize-deprecated-headers) namespace google::cloud::functions_internal { inline namespace FUNCTIONS_FRAMEWORK_CPP_NS { From acc7af1ced3537b7813dc0d919ba32810595ef0c Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Mon, 8 Feb 2021 03:36:34 +0000 Subject: [PATCH 3/3] Address review comments --- google/cloud/functions/internal/compiler_info_test.cc | 2 +- google/cloud/functions/internal/setenv.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/google/cloud/functions/internal/compiler_info_test.cc b/google/cloud/functions/internal/compiler_info_test.cc index 0e597f69..698c54b8 100644 --- a/google/cloud/functions/internal/compiler_info_test.cc +++ b/google/cloud/functions/internal/compiler_info_test.cc @@ -34,7 +34,7 @@ TEST(CompilerInfo, CompilerVersion) { auto const cv = CompilerVersion(); EXPECT_NE(cv, ""); // Look for something that looks vaguely like an X.Y version number. Cannot - // use ::testing::ContainsRegex() because these do not work when GTest is + // use ::testing::ContainsRegex() because that does not work when GTest is // compiled under MSVC. EXPECT_THAT(cv, Contains('.')); EXPECT_TRUE(std::regex_search(cv, std::regex(R"([0-9]+\.[0-9]+)"))) diff --git a/google/cloud/functions/internal/setenv.cc b/google/cloud/functions/internal/setenv.cc index f7dbc087..f342e06a 100644 --- a/google/cloud/functions/internal/setenv.cc +++ b/google/cloud/functions/internal/setenv.cc @@ -14,8 +14,8 @@ #include "google/cloud/functions/internal/setenv.h" // We need _putenv_s() on WIN32 and setenv()/unsetenv() on Posix. clang-tidy -// recommends including . That seems wrong, is not guaranteed -// to define the Posix/WIN32 functions. +// recommends including . That seems wrong, as is not +// guaranteed to define the Posix/WIN32 functions. #include // NOLINT(modernize-deprecated-headers) namespace google::cloud::functions_internal {