diff options
18 files changed, 245 insertions, 209 deletions
diff --git a/src/buildtool/common/cli.hpp b/src/buildtool/common/cli.hpp index ab4a7ff1..adfa3445 100644 --- a/src/buildtool/common/cli.hpp +++ b/src/buildtool/common/cli.hpp @@ -62,7 +62,7 @@ struct EndpointArguments { /// \brief Arguments required for building. struct BuildArguments { std::optional<std::vector<std::string>> local_launcher{std::nullopt}; - std::map<std::string, std::string> platform_properties; + std::vector<std::string> platform_properties; std::chrono::milliseconds timeout{kDefaultTimeout}; std::size_t build_jobs{}; std::optional<std::string> dump_artifacts{std::nullopt}; @@ -262,25 +262,14 @@ static inline auto SetupBuildArguments( ->type_name("JSON") ->default_val(nlohmann::json{"env", "--"}.dump()); - app->add_option_function<std::string>( + app->add_option( "--remote-execution-property", - [clargs](auto const& property) { - std::istringstream pss(property); - std::string key; - std::string val; - if (not std::getline(pss, key, ':') or - not std::getline(pss, val, ':')) { - throw CLI::ConversionError{property, - "--remote-execution-property"}; - } - clargs->platform_properties[std::move(key)] = std::move(val); - }, + clargs->platform_properties, "Property for remote execution as key-value pair. Specifying this " "option multiple times will accumulate pairs (latest wins).") ->type_name("KEY:VAL") ->allow_extra_args(false) - ->expected(1, 1) - ->trigger_on_parse(true); // call this everytime the option is parsed + ->expected(1, 1); app->add_option_function<unsigned int>( "--action-timeout", diff --git a/src/buildtool/execution_api/remote/config.hpp b/src/buildtool/execution_api/remote/config.hpp index e29b8aa7..14995b89 100644 --- a/src/buildtool/execution_api/remote/config.hpp +++ b/src/buildtool/execution_api/remote/config.hpp @@ -15,8 +15,72 @@ class RemoteExecutionConfig { public: + struct ServerAddress { + std::string host{}; + int port{}; + }; + + // Obtain global instance + [[nodiscard]] static auto Instance() noexcept -> RemoteExecutionConfig& { + static RemoteExecutionConfig config; + return config; + } + + // Set remote execution and cache address, unsets if parsing `address` fails + [[nodiscard]] static auto SetRemoteAddress( + std::string const& address) noexcept -> bool { + auto& inst = Instance(); + return static_cast<bool>(inst.remote_address_ = inst.cache_address_ = + ParseAddress(address)); + } + + // Set specific cache address, unsets if parsing `address` fails + [[nodiscard]] static auto SetCacheAddress( + std::string const& address) noexcept -> bool { + return static_cast<bool>(Instance().cache_address_ = + ParseAddress(address)); + } + + // Add platform property from string of form "key:val" + [[nodiscard]] static auto AddPlatformProperty( + std::string const& property) noexcept -> bool { + if (auto pair = ParseProperty(property)) { + Instance().platform_properties_[std::move(pair->first)] = + std::move(pair->second); + return true; + } + return false; + } + + // Remote execution address, if set + [[nodiscard]] static auto RemoteAddress() noexcept + -> std::optional<ServerAddress> { + return Instance().remote_address_; + } + + // Cache address, if set + [[nodiscard]] static auto CacheAddress() noexcept + -> std::optional<ServerAddress> { + return Instance().cache_address_; + } + + [[nodiscard]] static auto PlatformProperties() noexcept + -> std::map<std::string, std::string> { + return Instance().platform_properties_; + } + + private: + // Server address of remote execution. + std::optional<ServerAddress> remote_address_{}; + + // Server address of cache endpoint for rebuild. + std::optional<ServerAddress> cache_address_{}; + + // Platform properies for execution. + std::map<std::string, std::string> platform_properties_{}; + [[nodiscard]] static auto ParseAddress(std::string const& address) noexcept - -> std::optional<std::pair<std::string, int>> { + -> std::optional<ServerAddress> { std::istringstream iss(address); std::string host; std::string port; @@ -25,48 +89,34 @@ class RemoteExecutionConfig { return std::nullopt; } try { - return std::make_pair(host, std::stoi(port)); + static constexpr int kMaxPortNumber{ + std::numeric_limits<uint16_t>::max()}; + auto port_num = std::stoi(port); + if (not host.empty() and port_num >= 0 and + port_num <= kMaxPortNumber) { + return ServerAddress{std::move(host), port_num}; + } } catch (std::out_of_range const& e) { Logger::Log(LogLevel::Error, "Port raised out_of_range exception."); - return std::nullopt; } catch (std::invalid_argument const& e) { Logger::Log(LogLevel::Error, "Port raised invalid_argument exception."); - return std::nullopt; } + return std::nullopt; } - // Obtain global instance - [[nodiscard]] static auto Instance() noexcept -> RemoteExecutionConfig& { - static RemoteExecutionConfig config; - return config; - } - - [[nodiscard]] auto IsValidAddress() const noexcept -> bool { - return valid_; - } - - [[nodiscard]] auto SetAddress(std::string const& address) noexcept -> bool { - auto pair = ParseAddress(address); - return pair and SetAddress(pair->first, pair->second); - } - - [[nodiscard]] auto SetAddress(std::string const& host, int port) noexcept - -> bool { - host_ = host; - port_ = port, - valid_ = (not host.empty() and port >= 0 and port <= kMaxPortNumber); - return valid_; + [[nodiscard]] static auto ParseProperty( + std::string const& property) noexcept + -> std::optional<std::pair<std::string, std::string>> { + std::istringstream pss(property); + std::string key; + std::string val; + if (not std::getline(pss, key, ':') or + not std::getline(pss, val, ':')) { + return std::nullopt; + } + return std::make_pair(key, val); } - - [[nodiscard]] auto Host() const noexcept -> std::string { return host_; } - [[nodiscard]] auto Port() const noexcept -> int { return port_; } - - private: - static constexpr int kMaxPortNumber{std::numeric_limits<uint16_t>::max()}; - std::string host_{}; - int port_{}; - bool valid_{false}; }; #endif // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_REMOTE_CONFIG_HPP diff --git a/src/buildtool/graph_traverser/graph_traverser.hpp b/src/buildtool/graph_traverser/graph_traverser.hpp index 5169e61e..53e5eb20 100644 --- a/src/buildtool/graph_traverser/graph_traverser.hpp +++ b/src/buildtool/graph_traverser/graph_traverser.hpp @@ -36,7 +36,6 @@ class GraphTraverser { public: struct CommandLineArguments { std::size_t jobs; - EndpointArguments endpoint; BuildArguments build; std::optional<StageArguments> stage; std::optional<RebuildArguments> rebuild; @@ -50,13 +49,13 @@ class GraphTraverser { explicit GraphTraverser(CommandLineArguments clargs) : clargs_{std::move(clargs)}, - api_{CreateExecutionApi(clargs_.endpoint)}, + api_{CreateExecutionApi(RemoteExecutionConfig::RemoteAddress())}, reporter_{[](auto done, auto cv) {}} {} explicit GraphTraverser(CommandLineArguments clargs, progress_reporter_t reporter) : clargs_{std::move(clargs)}, - api_{CreateExecutionApi(clargs_.endpoint)}, + api_{CreateExecutionApi(RemoteExecutionConfig::RemoteAddress())}, reporter_{std::move(reporter)} {} /// \brief Parses actions and blobs into graph, traverses it and retrieves @@ -214,21 +213,14 @@ class GraphTraverser { } [[nodiscard]] static auto CreateExecutionApi( - EndpointArguments const& clargs) -> gsl::not_null<IExecutionApi::Ptr> { - if (clargs.remote_execution_address) { - auto remote = RemoteExecutionConfig{}; - if (not remote.SetAddress(*clargs.remote_execution_address)) { - Logger::Log(LogLevel::Error, - "parsing remote execution address '{}' failed.", - *clargs.remote_execution_address); - std::exit(EXIT_FAILURE); - } - + std::optional<RemoteExecutionConfig::ServerAddress> const& address) + -> gsl::not_null<IExecutionApi::Ptr> { + if (address) { ExecutionConfiguration config; config.skip_cache_lookup = false; return std::make_unique<BazelApi>( - "remote-execution", remote.Host(), remote.Port(), config); + "remote-execution", address->host, address->port, config); } return std::make_unique<LocalApi>(); } @@ -299,8 +291,9 @@ class GraphTraverser { [[nodiscard]] auto Traverse( DependencyGraph const& g, std::vector<ArtifactIdentifier> const& artifact_ids) const -> bool { - Executor executor{ - &(*api_), clargs_.build.platform_properties, clargs_.build.timeout}; + Executor executor{&(*api_), + RemoteExecutionConfig::PlatformProperties(), + clargs_.build.timeout}; bool result{}; std::atomic<bool> done = false; std::condition_variable cv{}; @@ -320,20 +313,12 @@ class GraphTraverser { [[nodiscard]] auto TraverseRebuild( DependencyGraph const& g, std::vector<ArtifactIdentifier> const& artifact_ids) const -> bool { - // create second configuration for cache endpoint - auto cache_args = clargs_.endpoint; - if (not clargs_.rebuild->cache_endpoint.value_or("").empty()) { - cache_args.remote_execution_address = - *clargs_.rebuild->cache_endpoint == "local" - ? std::nullopt // disable - : clargs_.rebuild->cache_endpoint; // set endpoint - } - // setup rebuilder with api for cache endpoint - auto api_cached = CreateExecutionApi(cache_args); + auto api_cached = + CreateExecutionApi(RemoteExecutionConfig::CacheAddress()); Rebuilder executor{&(*api_), &(*api_cached), - clargs_.build.platform_properties, + RemoteExecutionConfig::PlatformProperties(), clargs_.build.timeout}; bool success{false}; { diff --git a/src/buildtool/main/main.cpp b/src/buildtool/main/main.cpp index b41d7a11..23ea7b8e 100644 --- a/src/buildtool/main/main.cpp +++ b/src/buildtool/main/main.cpp @@ -205,15 +205,44 @@ void SetupLogging(CommonArguments const& clargs) { } #ifndef BOOTSTRAP_BUILD_TOOL -void SetupLocalExecution(EndpointArguments const& eargs, - BuildArguments const& bargs) { +void SetupExecutionConfig(EndpointArguments const& eargs, + BuildArguments const& bargs, + RebuildArguments const& rargs) { using LocalConfig = LocalExecutionConfig; + using RemoteConfig = RemoteExecutionConfig; if (not LocalConfig::SetKeepBuildDir(bargs.persistent_build_dir) or not(not eargs.local_root or (LocalConfig::SetBuildRoot(*eargs.local_root))) or not(not bargs.local_launcher or LocalConfig::SetLauncher(*bargs.local_launcher))) { Logger::Log(LogLevel::Error, "failed to configure local execution."); + std::exit(kExitFailure); + } + for (auto const& property : bargs.platform_properties) { + if (not RemoteConfig::AddPlatformProperty(property)) { + Logger::Log(LogLevel::Error, + "addding platform property '{}' failed.", + property); + std::exit(kExitFailure); + } + } + if (eargs.remote_execution_address) { + if (not RemoteConfig::SetRemoteAddress( + *eargs.remote_execution_address)) { + Logger::Log(LogLevel::Error, + "setting remote execution address '{}' failed.", + *eargs.remote_execution_address); + std::exit(kExitFailure); + } + } + if (rargs.cache_endpoint) { + if (not(RemoteConfig::SetCacheAddress(*rargs.cache_endpoint) == + (*rargs.cache_endpoint != "local"))) { + Logger::Log(LogLevel::Error, + "setting cache endpoint address '{}' failed.", + *rargs.cache_endpoint); + std::exit(kExitFailure); + } } } @@ -1251,7 +1280,8 @@ auto main(int argc, char* argv[]) -> int { SetupLogging(arguments.common); #ifndef BOOTSTRAP_BUILD_TOOL SetupHashGenerator(); - SetupLocalExecution(arguments.endpoint, arguments.build); + SetupExecutionConfig( + arguments.endpoint, arguments.build, arguments.rebuild); #endif auto jobs = arguments.build.build_jobs > 0 ? arguments.build.build_jobs @@ -1270,7 +1300,6 @@ auto main(int argc, char* argv[]) -> int { #ifndef BOOTSTRAP_BUILD_TOOL GraphTraverser const traverser{{jobs, - std::move(arguments.endpoint), std::move(arguments.build), std::move(stage_args), std::move(rebuild_args)}, diff --git a/test/buildtool/execution_api/bazel/bazel_ac_client.test.cpp b/test/buildtool/execution_api/bazel/bazel_ac_client.test.cpp index 4a352a8e..ca97c7ce 100644 --- a/test/buildtool/execution_api/bazel/bazel_ac_client.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_ac_client.test.cpp @@ -23,9 +23,9 @@ auto CreateActionCacheEntry(BazelAcClient* ac_client, // run only the current test case. See issue#30 in // https://rnd-gitlab-eu-c.huawei.com/germany-research-center/intelligent-cloud-technologies-laboratory/9424510-devcloud-build-tool-technology-project-de/-/issues/30 TEST_CASE("Bazel internals: AC Client", "[!hide][execution_api]") { - auto const& info = RemoteExecutionConfig::Instance(); + auto const& info = RemoteExecutionConfig::RemoteAddress(); - BazelAcClient ac_client(info.Host(), info.Port()); + BazelAcClient ac_client(info->host, info->port); std::string instance_name{"remote-execution"}; std::string content("test"); @@ -34,7 +34,7 @@ TEST_CASE("Bazel internals: AC Client", "[!hide][execution_api]") { auto action_id = CreateAction(instance_name, {"echo", "-n", content}, {}, - ReadPlatformPropertiesFromEnv()); + RemoteExecutionConfig::PlatformProperties()); REQUIRE(action_id); // TODO(investigate): Upload fails due to permission issues. The BuildBarn diff --git a/test/buildtool/execution_api/bazel/bazel_api.test.cpp b/test/buildtool/execution_api/bazel/bazel_api.test.cpp index e6eb011e..580a7751 100644 --- a/test/buildtool/execution_api/bazel/bazel_api.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_api.test.cpp @@ -10,40 +10,45 @@ namespace { auto const kApiFactory = []() { - static auto const& server = RemoteExecutionConfig::Instance(); + static auto const& server = RemoteExecutionConfig::RemoteAddress(); return IExecutionApi::Ptr{ - new BazelApi{"remote-execution", server.Host(), server.Port(), {}}}; + new BazelApi{"remote-execution", server->host, server->port, {}}}; }; } // namespace TEST_CASE("BazelAPI: No input, no output", "[execution_api]") { - TestNoInputNoOutput(kApiFactory, ReadPlatformPropertiesFromEnv()); + TestNoInputNoOutput(kApiFactory, + RemoteExecutionConfig::PlatformProperties()); } TEST_CASE("BazelAPI: No input, create output", "[execution_api]") { - TestNoInputCreateOutput(kApiFactory, ReadPlatformPropertiesFromEnv()); + TestNoInputCreateOutput(kApiFactory, + RemoteExecutionConfig::PlatformProperties()); } TEST_CASE("BazelAPI: One input copied to output", "[execution_api]") { - TestOneInputCopiedToOutput(kApiFactory, ReadPlatformPropertiesFromEnv()); + TestOneInputCopiedToOutput(kApiFactory, + RemoteExecutionConfig::PlatformProperties()); } TEST_CASE("BazelAPI: Non-zero exit code, create output", "[execution_api]") { - TestNonZeroExitCodeCreateOutput(kApiFactory, - ReadPlatformPropertiesFromEnv()); + TestNonZeroExitCodeCreateOutput( + kApiFactory, RemoteExecutionConfig::PlatformProperties()); } TEST_CASE("BazelAPI: Retrieve two identical trees to path", "[execution_api]") { TestRetrieveTwoIdenticalTreesToPath( - kApiFactory, ReadPlatformPropertiesFromEnv(), "two_trees"); + kApiFactory, RemoteExecutionConfig::PlatformProperties(), "two_trees"); } TEST_CASE("BazelAPI: Retrieve mixed blobs and trees", "[execution_api]") { - TestRetrieveMixedBlobsAndTrees( - kApiFactory, ReadPlatformPropertiesFromEnv(), "blobs_and_trees"); + TestRetrieveMixedBlobsAndTrees(kApiFactory, + RemoteExecutionConfig::PlatformProperties(), + "blobs_and_trees"); } TEST_CASE("BazelAPI: Create directory prior to execution", "[execution_api]") { - TestCreateDirPriorToExecution(kApiFactory, ReadPlatformPropertiesFromEnv()); + TestCreateDirPriorToExecution(kApiFactory, + RemoteExecutionConfig::PlatformProperties()); } diff --git a/test/buildtool/execution_api/bazel/bazel_cas_client.test.cpp b/test/buildtool/execution_api/bazel/bazel_cas_client.test.cpp index 30866ffe..dc75da7e 100644 --- a/test/buildtool/execution_api/bazel/bazel_cas_client.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_cas_client.test.cpp @@ -6,13 +6,13 @@ #include "src/buildtool/execution_api/remote/config.hpp" TEST_CASE("Bazel internals: CAS Client", "[execution_api]") { - auto const& info = RemoteExecutionConfig::Instance(); + auto const& info = RemoteExecutionConfig::RemoteAddress(); std::string instance_name{"remote-execution"}; std::string content("test"); // Create CAS client - BazelCasClient cas_client(info.Host(), info.Port()); + BazelCasClient cas_client(info->host, info->port); SECTION("Valid digest and blob") { // digest of "test" diff --git a/test/buildtool/execution_api/bazel/bazel_execution_client.test.cpp b/test/buildtool/execution_api/bazel/bazel_execution_client.test.cpp index a80eb1af..f0246fd7 100755 --- a/test/buildtool/execution_api/bazel/bazel_execution_client.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_execution_client.test.cpp @@ -7,22 +7,23 @@ #include "test/utils/test_env.hpp" TEST_CASE("Bazel internals: Execution Client", "[execution_api]") { - auto const& info = RemoteExecutionConfig::Instance(); + auto const& info = RemoteExecutionConfig::RemoteAddress(); std::string instance_name{"remote-execution"}; std::string content("test"); auto test_digest = ArtifactDigest::Create(content); - BazelExecutionClient execution_client(info.Host(), info.Port()); + BazelExecutionClient execution_client(info->host, info->port); ExecutionConfiguration config; config.skip_cache_lookup = false; SECTION("Immediate execution and response") { - auto action_immediate = CreateAction(instance_name, - {"echo", "-n", content}, - {}, - ReadPlatformPropertiesFromEnv()); + auto action_immediate = + CreateAction(instance_name, + {"echo", "-n", content}, + {}, + RemoteExecutionConfig::PlatformProperties()); REQUIRE(action_immediate); auto response = execution_client.Execute( @@ -41,7 +42,7 @@ TEST_CASE("Bazel internals: Execution Client", "[execution_api]") { CreateAction(instance_name, {"sh", "-c", "sleep 1s; echo -n test"}, {}, - ReadPlatformPropertiesFromEnv()); + RemoteExecutionConfig::PlatformProperties()); SECTION("Blocking, immediately obtain result") { auto response = execution_client.Execute( @@ -73,13 +74,13 @@ TEST_CASE("Bazel internals: Execution Client", "[execution_api]") { TEST_CASE("Bazel internals: Execution Client using env variables", "[execution_api]") { - auto const& info = RemoteExecutionConfig::Instance(); + auto const& info = RemoteExecutionConfig::RemoteAddress(); std::string instance_name{"remote-execution"}; std::string content("contents of env variable"); auto test_digest = ArtifactDigest::Create(content); - BazelExecutionClient execution_client(info.Host(), info.Port()); + BazelExecutionClient execution_client(info->host, info->port); ExecutionConfiguration config; config.skip_cache_lookup = false; @@ -87,7 +88,7 @@ TEST_CASE("Bazel internals: Execution Client using env variables", CreateAction(instance_name, {"/bin/sh", "-c", "set -e\necho -n ${MYTESTVAR}"}, {{"MYTESTVAR", content}}, - ReadPlatformPropertiesFromEnv()); + RemoteExecutionConfig::PlatformProperties()); REQUIRE(action); auto response = diff --git a/test/buildtool/execution_api/bazel/bazel_network.test.cpp b/test/buildtool/execution_api/bazel/bazel_network.test.cpp index 6a668406..b67013a0 100644 --- a/test/buildtool/execution_api/bazel/bazel_network.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_network.test.cpp @@ -8,9 +8,9 @@ constexpr std::size_t kLargeSize = GRPC_DEFAULT_MAX_RECV_MESSAGE_LENGTH + 1; TEST_CASE("Bazel network: write/read blobs", "[execution_api]") { - auto const& info = RemoteExecutionConfig::Instance(); + auto const& info = RemoteExecutionConfig::RemoteAddress(); std::string instance_name{"remote-execution"}; - auto network = BazelNetwork{instance_name, info.Host(), info.Port(), {}}; + auto network = BazelNetwork{instance_name, info->host, info->port, {}}; std::string content_foo("foo"); std::string content_bar("bar"); diff --git a/test/buildtool/execution_api/bazel/bytestream_client.test.cpp b/test/buildtool/execution_api/bazel/bytestream_client.test.cpp index fa09862c..3187c99c 100644 --- a/test/buildtool/execution_api/bazel/bytestream_client.test.cpp +++ b/test/buildtool/execution_api/bazel/bytestream_client.test.cpp @@ -9,8 +9,8 @@ constexpr std::size_t kLargeSize = GRPC_DEFAULT_MAX_RECV_MESSAGE_LENGTH + 1; TEST_CASE("ByteStream Client: Transfer single blob", "[execution_api]") { - auto const& info = RemoteExecutionConfig::Instance(); - auto stream = ByteStreamClient{info.Host(), info.Port()}; + auto const& info = RemoteExecutionConfig::RemoteAddress(); + auto stream = ByteStreamClient{info->host, info->port}; auto uuid = CreateUUIDVersion4(*CreateProcessUniqueId()); SECTION("Upload small blob") { @@ -78,8 +78,8 @@ TEST_CASE("ByteStream Client: Transfer single blob", "[execution_api]") { } TEST_CASE("ByteStream Client: Transfer multiple blobs", "[execution_api]") { - auto const& info = RemoteExecutionConfig::Instance(); - auto stream = ByteStreamClient{info.Host(), info.Port()}; + auto const& info = RemoteExecutionConfig::RemoteAddress(); + auto stream = ByteStreamClient{info->host, info->port}; auto uuid = CreateUUIDVersion4(*CreateProcessUniqueId()); SECTION("Upload small blobs") { diff --git a/test/buildtool/execution_engine/executor/executor_api.test.hpp b/test/buildtool/execution_engine/executor/executor_api.test.hpp index 48d37d98..8459569b 100755 --- a/test/buildtool/execution_engine/executor/executor_api.test.hpp +++ b/test/buildtool/execution_engine/executor/executor_api.test.hpp @@ -8,6 +8,7 @@ #include "src/buildtool/common/artifact_description.hpp" #include "src/buildtool/common/artifact_factory.hpp" #include "src/buildtool/execution_api/common/execution_api.hpp" +#include "src/buildtool/execution_api/remote/config.hpp" #include "src/buildtool/execution_engine/dag/dag.hpp" #include "src/buildtool/execution_engine/executor/executor.hpp" #include "src/buildtool/file_system/file_system_manager.hpp" @@ -84,7 +85,7 @@ static inline void RunHelloWorldCompilation(ApiFactory const& factory, CHECK(g.ArtifactNodeWithId(exec_id)->HasBuilderAction()); auto api = factory(); - Executor runner{api.get(), ReadPlatformPropertiesFromEnv()}; + Executor runner{api.get(), RemoteExecutionConfig::PlatformProperties()}; // upload local artifacts auto const* main_cpp_node = g.ArtifactNodeWithId(main_cpp_id); @@ -185,7 +186,7 @@ static inline void RunGreeterCompilation(ApiFactory const& factory, CHECK(g.Add({compile_greet_desc, make_lib_desc, make_exe_desc})); auto api = factory(); - Executor runner{api.get(), ReadPlatformPropertiesFromEnv()}; + Executor runner{api.get(), RemoteExecutionConfig::PlatformProperties()}; // upload local artifacts for (auto const& id : {greet_hpp_id, greet_cpp_id, main_cpp_id}) { @@ -284,7 +285,7 @@ static inline void TestUploadAndDownloadTrees(ApiFactory const& factory, auto foo_id = g.AddArtifact(foo_desc); auto bar_id = g.AddArtifact(bar_desc); - Executor runner{api.get(), ReadPlatformPropertiesFromEnv()}; + Executor runner{api.get(), RemoteExecutionConfig::PlatformProperties()}; REQUIRE(runner.Process(g.ArtifactNodeWithId(foo_id))); REQUIRE(runner.Process(g.ArtifactNodeWithId(bar_id))); @@ -422,7 +423,7 @@ static inline void TestRetrieveOutputDirectories(ApiFactory const& factory, // run action auto api = factory(); - Executor runner{api.get(), ReadPlatformPropertiesFromEnv()}; + Executor runner{api.get(), RemoteExecutionConfig::PlatformProperties()}; REQUIRE(runner.Process(action)); // read output @@ -465,7 +466,7 @@ static inline void TestRetrieveOutputDirectories(ApiFactory const& factory, // run action auto api = factory(); - Executor runner{api.get(), ReadPlatformPropertiesFromEnv()}; + Executor runner{api.get(), RemoteExecutionConfig::PlatformProperties()}; REQUIRE(runner.Process(action)); // read output @@ -524,7 +525,7 @@ static inline void TestRetrieveOutputDirectories(ApiFactory const& factory, // run action auto api = factory(); - Executor runner{api.get(), ReadPlatformPropertiesFromEnv()}; + Executor runner{api.get(), RemoteExecutionConfig::PlatformProperties()}; REQUIRE(runner.Process(action)); // read output @@ -588,7 +589,8 @@ static inline void TestRetrieveOutputDirectories(ApiFactory const& factory, // run action auto api = factory(); - Executor runner{api.get(), ReadPlatformPropertiesFromEnv()}; + Executor runner{api.get(), + RemoteExecutionConfig::PlatformProperties()}; CHECK_FALSE(runner.Process(action)); } @@ -606,7 +608,8 @@ static inline void TestRetrieveOutputDirectories(ApiFactory const& factory, // run action auto api = factory(); - Executor runner{api.get(), ReadPlatformPropertiesFromEnv()}; + Executor runner{api.get(), + RemoteExecutionConfig::PlatformProperties()}; CHECK_FALSE(runner.Process(action)); } } diff --git a/test/buildtool/execution_engine/executor/executor_api_remote_bazel.test.cpp b/test/buildtool/execution_engine/executor/executor_api_remote_bazel.test.cpp index d6ad57b1..ea68298b 100755 --- a/test/buildtool/execution_engine/executor/executor_api_remote_bazel.test.cpp +++ b/test/buildtool/execution_engine/executor/executor_api_remote_bazel.test.cpp @@ -6,11 +6,11 @@ TEST_CASE("Executor<BazelApi>: Upload blob", "[executor]") { ExecutionConfiguration config; - auto const& info = RemoteExecutionConfig::Instance(); + auto const& info = RemoteExecutionConfig::RemoteAddress(); TestBlobUpload([&] { return BazelApi::Ptr{ - new BazelApi{"remote-execution", info.Host(), info.Port(), config}}; + new BazelApi{"remote-execution", info->host, info->port, config}}; }); } @@ -18,12 +18,12 @@ TEST_CASE("Executor<BazelApi>: Compile hello world", "[executor]") { ExecutionConfiguration config; config.skip_cache_lookup = false; - auto const& info = RemoteExecutionConfig::Instance(); + auto const& info = RemoteExecutionConfig::RemoteAddress(); TestHelloWorldCompilation( [&] { return BazelApi::Ptr{new BazelApi{ - "remote-execution", info.Host(), info.Port(), config}}; + "remote-execution", info->host, info->port, config}}; }, false /* not hermetic */); } @@ -32,12 +32,12 @@ TEST_CASE("Executor<BazelApi>: Compile greeter", "[executor]") { ExecutionConfiguration config; config.skip_cache_lookup = false; - auto const& info = RemoteExecutionConfig::Instance(); + auto const& info = RemoteExecutionConfig::RemoteAddress(); TestGreeterCompilation( [&] { return BazelApi::Ptr{new BazelApi{ - "remote-execution", info.Host(), info.Port(), config}}; + "remote-execution", info->host, info->port, config}}; }, false /* not hermetic */); } @@ -46,12 +46,12 @@ TEST_CASE("Executor<BazelApi>: Upload and download trees", "[executor]") { ExecutionConfiguration config; config.skip_cache_lookup = false; - auto const& info = RemoteExecutionConfig::Instance(); + auto const& info = RemoteExecutionConfig::RemoteAddress(); TestUploadAndDownloadTrees( [&] { return BazelApi::Ptr{new BazelApi{ - "remote-execution", info.Host(), info.Port(), config}}; + "remote-execution", info->host, info->port, config}}; }, false /* not hermetic */); } @@ -60,12 +60,12 @@ TEST_CASE("Executor<BazelApi>: Retrieve output directories", "[executor]") { ExecutionConfiguration config; config.skip_cache_lookup = false; - auto const& info = RemoteExecutionConfig::Instance(); + auto const& info = RemoteExecutionConfig::RemoteAddress(); TestRetrieveOutputDirectories( [&] { return BazelApi::Ptr{new BazelApi{ - "remote-execution", info.Host(), info.Port(), config}}; + "remote-execution", info->host, info->port, config}}; }, false /* not hermetic */); } diff --git a/test/buildtool/graph_traverser/graph_traverser.test.hpp b/test/buildtool/graph_traverser/graph_traverser.test.hpp index b1041d73..2b5e1d07 100644 --- a/test/buildtool/graph_traverser/graph_traverser.test.hpp +++ b/test/buildtool/graph_traverser/graph_traverser.test.hpp @@ -30,18 +30,15 @@ class TestProject { }; // NOLINTNEXTLINE(modernize-pass-by-value) - explicit TestProject(std::string const& example_name, - bool run_local = false) + explicit TestProject(std::string const& example_name) : example_name_{example_name}, - root_dir_{kWorkspacePrefix / example_name_}, - run_local_{run_local} { + root_dir_{kWorkspacePrefix / example_name_} { SetupConfig(); } - explicit TestProject(std::string&& example_name, bool run_local = false) + explicit TestProject(std::string&& example_name) : example_name_{std::move(example_name)}, - root_dir_{kWorkspacePrefix / example_name_}, - run_local_{run_local} { + root_dir_{kWorkspacePrefix / example_name_} { SetupConfig(); } @@ -79,7 +76,6 @@ class TestProject { "_entry_points"; std::string example_name_{}; std::filesystem::path root_dir_{}; - bool run_local_{}; void SetupConfig() { auto info = RepositoryConfig::RepositoryInfo{FileRoot{root_dir_}}; @@ -91,7 +87,7 @@ class TestProject { -> CommandLineArguments { static int id{}; - GraphTraverser::CommandLineArguments gtargs{0, {}, {}, {}, {}}; + GraphTraverser::CommandLineArguments gtargs{0, {}, {}, {}}; CommandLineArguments clargs{gtargs}; clargs.artifacts = entry_points; @@ -106,18 +102,6 @@ class TestProject { clargs.gtargs.jobs = std::max(1U, std::thread::hardware_concurrency()); clargs.gtargs.stage = StageArguments{ kOutputDirPrefix / (example_name_ + std::to_string(id++))}; - if (not run_local_) { - clargs.gtargs.endpoint.remote_execution_address = - ReadRemoteAddressFromEnv(); - clargs.gtargs.build.platform_properties = - ReadPlatformPropertiesFromEnv(); - if (not clargs.gtargs.endpoint.remote_execution_address) { - Logger::Log(LogLevel::Error, - "Missing env var 'REMOTE_EXECUTION_ADDRESS' for " - "non-local graph_traverser tests."); - std::exit(EXIT_FAILURE); - } - } return clargs; } }; @@ -125,9 +109,8 @@ class TestProject { } // namespace [[maybe_unused]] static void TestHelloWorldCopyMessage( - bool run_local, bool is_hermetic = true) { - TestProject p("hello_world_copy_message", run_local); + TestProject p("hello_world_copy_message"); auto const clargs = p.CmdLineArgs(); GraphTraverser const gt{clargs.gtargs}; @@ -169,9 +152,8 @@ class TestProject { } } -[[maybe_unused]] static void TestCopyLocalFile(bool run_local, - bool is_hermetic = true) { - TestProject p("copy_local_file", run_local); +[[maybe_unused]] static void TestCopyLocalFile(bool is_hermetic = true) { + TestProject p("copy_local_file"); auto const clargs = p.CmdLineArgs(); GraphTraverser const gt{clargs.gtargs}; @@ -189,9 +171,8 @@ class TestProject { } [[maybe_unused]] static void TestSequencePrinterBuildLibraryOnly( - bool run_local, bool is_hermetic = true) { - TestProject p("sequence_printer_build_library_only", run_local); + TestProject p("sequence_printer_build_library_only"); auto const clargs = p.CmdLineArgs(); GraphTraverser const gt{clargs.gtargs}; @@ -221,9 +202,8 @@ class TestProject { } [[maybe_unused]] static void TestHelloWorldWithKnownSource( - bool run_local, bool is_hermetic = true) { - TestProject full_hello_world("hello_world_copy_message", run_local); + TestProject full_hello_world("hello_world_copy_message"); auto const clargs_update_cpp = full_hello_world.CmdLineArgs("_entry_points_upload_source"); @@ -240,7 +220,7 @@ class TestProject { CHECK(Statistics::Instance().ActionsQueuedCounter() == 0); CHECK(Statistics::Instance().ActionsCachedCounter() == 0); } - TestProject hello_world_known_cpp("hello_world_known_source", run_local); + TestProject hello_world_known_cpp("hello_world_known_source"); auto const clargs = hello_world_known_cpp.CmdLineArgs(); GraphTraverser const gt{clargs.gtargs}; @@ -260,8 +240,8 @@ class TestProject { } } -static void TestBlobsUploadedAndUsed(bool run_local, bool is_hermetic = true) { - TestProject p("use_uploaded_blobs", run_local); +static void TestBlobsUploadedAndUsed(bool is_hermetic = true) { + TestProject p("use_uploaded_blobs"); auto const clargs = p.CmdLineArgs(); GraphTraverser gt{clargs.gtargs}; @@ -286,9 +266,8 @@ static void TestBlobsUploadedAndUsed(bool run_local, bool is_hermetic = true) { } } -static void TestEnvironmentVariablesSetAndUsed(bool run_local, - bool is_hermetic = true) { - TestProject p("use_env_variables", run_local); +static void TestEnvironmentVariablesSetAndUsed(bool is_hermetic = true) { + TestProject p("use_env_variables"); auto const clargs = p.CmdLineArgs(); GraphTraverser gt{clargs.gtargs}; @@ -313,8 +292,8 @@ static void TestEnvironmentVariablesSetAndUsed(bool run_local, } } -static void TestTreesUsed(bool run_local, bool is_hermetic = true) { - TestProject p("use_trees", run_local); +static void TestTreesUsed(bool is_hermetic = true) { + TestProject p("use_trees"); auto const clargs = p.CmdLineArgs(); GraphTraverser gt{clargs.gtargs}; @@ -339,8 +318,8 @@ static void TestTreesUsed(bool run_local, bool is_hermetic = true) { } } -static void TestNestedTreesUsed(bool run_local, bool is_hermetic = true) { - TestProject p("use_nested_trees", run_local); +static void TestNestedTreesUsed(bool is_hermetic = true) { + TestProject p("use_nested_trees"); auto const clargs = p.CmdLineArgs(); GraphTraverser gt{clargs.gtargs}; @@ -365,9 +344,8 @@ static void TestNestedTreesUsed(bool run_local, bool is_hermetic = true) { } } -static void TestFlakyHelloWorldDetected(bool run_local, - bool /*is_hermetic*/ = true) { - TestProject p("flaky_hello_world", run_local); +static void TestFlakyHelloWorldDetected(bool /*is_hermetic*/ = true) { + TestProject p("flaky_hello_world"); { auto clargs = p.CmdLineArgs("_entry_points_ctimes"); diff --git a/test/buildtool/graph_traverser/graph_traverser_local.test.cpp b/test/buildtool/graph_traverser/graph_traverser_local.test.cpp index 86317625..75fca394 100644 --- a/test/buildtool/graph_traverser/graph_traverser_local.test.cpp +++ b/test/buildtool/graph_traverser/graph_traverser_local.test.cpp @@ -5,53 +5,53 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Output created when entry point is local artifact", "[graph_traverser]") { - TestCopyLocalFile(true); + TestCopyLocalFile(); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Output created and contents are correct", "[graph_traverser]") { - TestHelloWorldCopyMessage(true); + TestHelloWorldCopyMessage(); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Actions are not re-run", "[graph_traverser]") { - TestSequencePrinterBuildLibraryOnly(true); + TestSequencePrinterBuildLibraryOnly(); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: KNOWN artifact", "[graph_traverser]") { - TestHelloWorldWithKnownSource(true); + TestHelloWorldWithKnownSource(); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Blobs uploaded and correctly used", "[graph_traverser]") { - TestBlobsUploadedAndUsed(true); + TestBlobsUploadedAndUsed(); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Environment variables are set and used", "[graph_traverser]") { - TestEnvironmentVariablesSetAndUsed(true); + TestEnvironmentVariablesSetAndUsed(); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Trees correctly used", "[graph_traverser]") { - TestTreesUsed(true); + TestTreesUsed(); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Nested trees correctly used", "[graph_traverser]") { - TestNestedTreesUsed(true); + TestNestedTreesUsed(); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Detect flaky actions", "[graph_traverser]") { - TestFlakyHelloWorldDetected(true); + TestFlakyHelloWorldDetected(); } diff --git a/test/buildtool/graph_traverser/graph_traverser_remote.test.cpp b/test/buildtool/graph_traverser/graph_traverser_remote.test.cpp index 8aad88ad..3460b175 100644 --- a/test/buildtool/graph_traverser/graph_traverser_remote.test.cpp +++ b/test/buildtool/graph_traverser/graph_traverser_remote.test.cpp @@ -3,39 +3,39 @@ TEST_CASE("Remote: Output created and contents are correct", "[graph_traverser]") { - TestHelloWorldCopyMessage(false, false /* not hermetic */); + TestHelloWorldCopyMessage(false /* not hermetic */); } TEST_CASE("Remote: Output created when entry point is local artifact", "[graph_traverser]") { - TestCopyLocalFile(false, false /* not hermetic */); + TestCopyLocalFile(false /* not hermetic */); } TEST_CASE("Remote: Actions are not re-run", "[graph_traverser]") { - TestSequencePrinterBuildLibraryOnly(false, false /* not hermetic */); + TestSequencePrinterBuildLibraryOnly(false /* not hermetic */); } TEST_CASE("Remote: KNOWN artifact", "[graph_traverser]") { - TestHelloWorldWithKnownSource(false, false /* not hermetic */); + TestHelloWorldWithKnownSource(false /* not hermetic */); } TEST_CASE("Remote: Blobs uploaded and correctly used", "[graph_traverser]") { - TestBlobsUploadedAndUsed(false, false /* not hermetic */); + TestBlobsUploadedAndUsed(false /* not hermetic */); } TEST_CASE("Remote: Environment variables are set and used", "[graph_traverser]") { - TestEnvironmentVariablesSetAndUsed(false, false /* not hermetic */); + TestEnvironmentVariablesSetAndUsed(false /* not hermetic */); } TEST_CASE("Remote: Trees correctly used", "[graph_traverser]") { - TestTreesUsed(false, false /* not hermetic */); + TestTreesUsed(false /* not hermetic */); } TEST_CASE("Remote: Nested trees correctly used", "[graph_traverser]") { - TestNestedTreesUsed(false, false /* not hermetic */); + TestNestedTreesUsed(false /* not hermetic */); } TEST_CASE("Remote: Detect flaky actions", "[graph_traverser]") { - TestFlakyHelloWorldDetected(false, false /* not hermetic */); + TestFlakyHelloWorldDetected(false /* not hermetic */); } diff --git a/test/utils/remote_execution/bazel_action_creator.hpp b/test/utils/remote_execution/bazel_action_creator.hpp index 27864408..5589a1e1 100644 --- a/test/utils/remote_execution/bazel_action_creator.hpp +++ b/test/utils/remote_execution/bazel_action_creator.hpp @@ -16,7 +16,7 @@ std::map<std::string, std::string> const& env_vars, std::map<std::string, std::string> const& properties) noexcept -> std::unique_ptr<bazel_re::Digest> { - auto const& info = RemoteExecutionConfig::Instance(); + auto const& info = RemoteExecutionConfig::RemoteAddress(); auto platform = std::make_unique<bazel_re::Platform>(); for (auto const& [name, value] : properties) { @@ -63,7 +63,7 @@ auto action_id = ArtifactDigest::Create(action_data); blobs.emplace_back(action_id, action_data); - BazelCasClient cas_client(info.Host(), info.Port()); + BazelCasClient cas_client(info->host, info->port); if (cas_client.BatchUpdateBlobs(instance_name, blobs.begin(), blobs.end()) .size() == blobs.size()) { diff --git a/test/utils/remote_execution/main-remote-execution.cpp b/test/utils/remote_execution/main-remote-execution.cpp index 4db1d690..e930d893 100644 --- a/test/utils/remote_execution/main-remote-execution.cpp +++ b/test/utils/remote_execution/main-remote-execution.cpp @@ -27,12 +27,18 @@ void wait_for_grpc_to_shutdown() { HashGenerator::SetHashGenerator(HashGenerator::HashType::SHA256); } auto address = ReadRemoteAddressFromEnv(); - auto& config = RemoteExecutionConfig::Instance(); - if (address and not config.SetAddress(*address)) { + if (address and not RemoteExecutionConfig::SetRemoteAddress(*address)) { Logger::Log(LogLevel::Error, "parsing address '{}' failed.", *address); std::exit(EXIT_FAILURE); } - return config.IsValidAddress(); + for (auto const& property : ReadPlatformPropertiesFromEnv()) { + if (not RemoteExecutionConfig::AddPlatformProperty(property)) { + Logger::Log( + LogLevel::Error, "parsing property '{}' failed.", property); + std::exit(EXIT_FAILURE); + } + } + return static_cast<bool>(RemoteExecutionConfig::RemoteAddress()); } } // namespace diff --git a/test/utils/test_env.hpp b/test/utils/test_env.hpp index d4f18994..a5444969 100644 --- a/test/utils/test_env.hpp +++ b/test/utils/test_env.hpp @@ -11,24 +11,14 @@ #include "test/utils/logging/log_config.hpp" [[nodiscard]] static inline auto ReadPlatformPropertiesFromEnv() - -> std::map<std::string, std::string> { - std::map<std::string, std::string> properties{}; + -> std::vector<std::string> { + std::vector<std::string> properties{}; auto* execution_props = std::getenv("REMOTE_EXECUTION_PROPERTIES"); if (execution_props not_eq nullptr) { std::istringstream pss(std::string{execution_props}); std::string keyval_pair; while (std::getline(pss, keyval_pair, ';')) { - std::istringstream kvss{keyval_pair}; - std::string key; - std::string val; - if (not std::getline(kvss, key, ':') or - not std::getline(kvss, val, ':')) { - Logger::Log(LogLevel::Error, - "parsing property '{}' failed.", - keyval_pair); - std::exit(EXIT_FAILURE); - } - properties.emplace(std::move(key), std::move(val)); + properties.emplace_back(keyval_pair); } } return properties; |