Skip to content

Commit

Permalink
fix(cmake)!: only compile protos if asked - storage (#12481)
Browse files Browse the repository at this point in the history
  • Loading branch information
dbolduc authored Aug 28, 2023
1 parent f8827ee commit cc50dd4
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 24 deletions.
11 changes: 1 addition & 10 deletions external/googleapis/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,7 @@ set(external_googleapis_installed_libraries_list
google_cloud_cpp_iam_v1_options_protos
google_cloud_cpp_iam_v1_policy_protos
google_cloud_cpp_logging_protos
google_cloud_cpp_longrunning_operations_protos
google_cloud_cpp_storage_protos)
google_cloud_cpp_longrunning_operations_protos)

# These proto files cannot be added in the foreach() loop because they have
# dependencies.
Expand Down Expand Up @@ -371,14 +370,6 @@ set_target_properties(
target_link_libraries(google_cloud_cpp_logging_type_protos
INTERFACE google-cloud-cpp::logging_type_type_protos)

google_cloud_cpp_load_protolist(storage_list "protolists/storage.list")
google_cloud_cpp_load_protodeps(storage_deps "protodeps/storage.deps")
google_cloud_cpp_grpcpp_library(
google_cloud_cpp_storage_protos ${storage_list} PROTO_PATH_DIRECTORIES
"${EXTERNAL_GOOGLEAPIS_SOURCE}" "${PROTO_INCLUDE_DIR}")
external_googleapis_set_version_and_alias(storage_protos)
target_link_libraries(google_cloud_cpp_storage_protos PUBLIC ${storage_deps})

# Install the libraries and headers in the locations determined by
# GNUInstallDirs
include(GNUInstallDirs)
Expand Down
14 changes: 11 additions & 3 deletions google/cloud/storage/benchmarks/throughput_experiment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
#include "google/cloud/storage/benchmarks/throughput_experiment.h"
#include "google/cloud/storage/benchmarks/benchmark_utils.h"
#include "google/cloud/storage/client.h"
#if GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
#include "google/cloud/storage/internal/grpc/ctype_cord_workaround.h"
#include "google/cloud/grpc_error_delegate.h"
#include <google/storage/v2/storage.grpc.pb.h>
#endif // GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
#include <curl/curl.h>
#include <iterator>
#include <vector>
Expand Down Expand Up @@ -335,6 +337,7 @@ class DownloadObjectLibcurl : public ThroughputExperiment {
std::shared_ptr<google::cloud::storage::oauth2::Credentials> creds_;
};

#if GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
std::shared_ptr<grpc::ChannelInterface> CreateGcsChannel(
ThroughputOptions const& options, int thread_id,
ExperimentTransport transport) {
Expand Down Expand Up @@ -409,6 +412,7 @@ class DownloadObjectRawGrpc : public ThroughputExperiment {
std::unique_ptr<google::storage::v2::Storage::StubInterface> stub_;
ExperimentTransport transport_;
};
#endif // GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC

} // namespace

Expand Down Expand Up @@ -451,10 +455,14 @@ std::vector<std::unique_ptr<ThroughputExperiment>> CreateDownloadExperiments(
if (t != ExperimentTransport::kGrpc &&
t != ExperimentTransport::kDirectPath) {
result.push_back(std::make_unique<DownloadObjectLibcurl>(options));
} else {
result.push_back(
std::make_unique<DownloadObjectRawGrpc>(options, thread_id, t));
continue;
}
#if GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
result.push_back(
std::make_unique<DownloadObjectRawGrpc>(options, thread_id, t));
#else
(void)thread_id;
#endif // GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
}
}
return result;
Expand Down
40 changes: 29 additions & 11 deletions google/cloud/storage/google_cloud_cpp_storage_grpc.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,28 @@ if (NOT GOOGLE_CLOUD_CPP_STORAGE_ENABLE_GRPC)
google_cloud_cpp_storage_grpc
INTERFACE GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND)
endif ()
add_library(google_cloud_cpp_storage_protos INTERFACE)
add_library(google-cloud-cpp::storage_protos ALIAS
google_cloud_cpp_storage_protos)
set_target_properties(
google_cloud_cpp_storage_protos
PROPERTIES EXPORT_NAME "google-cloud-cpp::storage_protos")
else ()
include(CompileProtos)
google_cloud_cpp_find_proto_include_dir(PROTO_INCLUDE_DIR)
google_cloud_cpp_load_protolist(
storage_list
"${PROJECT_SOURCE_DIR}/external/googleapis/protolists/storage.list")
google_cloud_cpp_load_protodeps(
storage_deps
"${PROJECT_SOURCE_DIR}/external/googleapis/protodeps/storage.deps")
google_cloud_cpp_grpcpp_library(
google_cloud_cpp_storage_protos ${storage_list} PROTO_PATH_DIRECTORIES
"${EXTERNAL_GOOGLEAPIS_SOURCE}" "${PROTO_INCLUDE_DIR}")
external_googleapis_set_version_and_alias(storage_protos)
target_link_libraries(google_cloud_cpp_storage_protos
PUBLIC ${storage_deps})

add_library(
google_cloud_cpp_storage_grpc # cmake-format: sort
async_client.cc
Expand Down Expand Up @@ -160,28 +181,25 @@ google_cloud_cpp_add_pkgconfig(
"openssl")

install(
TARGETS google_cloud_cpp_storage_grpc
TARGETS google_cloud_cpp_storage_grpc google_cloud_cpp_storage_protos
EXPORT storage-targets
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
COMPONENT google_cloud_cpp_runtime
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
COMPONENT google_cloud_cpp_runtime
NAMELINK_SKIP
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
COMPONENT google_cloud_cpp_development)
# With CMake-3.12 and higher we could avoid this separate command (and the
# duplication).
install(
TARGETS google_cloud_cpp_storage_grpc
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
COMPONENT google_cloud_cpp_development
NAMELINK_ONLY
NAMELINK_COMPONENT google_cloud_cpp_development
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
COMPONENT google_cloud_cpp_development)

include(CompileProtos)
google_cloud_cpp_install_proto_library_protos(google_cloud_cpp_storage_protos
"${EXTERNAL_GOOGLEAPIS_SOURCE}")
google_cloud_cpp_install_proto_library_headers(google_cloud_cpp_storage_protos)
google_cloud_cpp_install_headers(google_cloud_cpp_storage_grpc
include/google/cloud/storage)

external_googleapis_install_pc(google_cloud_cpp_storage_protos)

if (BUILD_TESTING AND GOOGLE_CLOUD_CPP_STORAGE_ENABLE_GRPC)
# This is a bit weird, we add an additional link library to
# `storage_client_testing`
Expand Down

0 comments on commit cc50dd4

Please sign in to comment.