Skip to content

feat: don't require crc32c with recent Abseil #15402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: don't require crc32c with recent Abseil #15402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion google/cloud/storage/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@ google_cloud_cpp_doxygen_targets("storage" DEPENDS cloud-docs
include(GoogleCloudCppCommon)

include(IncludeNlohmannJson)
find_package(Crc32c)
if (BUILD_TESTING OR absl_VERSION VERSION_LESS "20230125")
set(NEED_CRC32C TRUE)
else ()
set(NEED_CRC32C FALSE)
endif ()
if (NEED_CRC32C)
find_package(Crc32c REQUIRED)
endif()

# Export the version information for Bazel.
include(CreateBazelConfig)
Expand Down
11 changes: 9 additions & 2 deletions google/cloud/storage/google_cloud_cpp_storage.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,11 @@ target_link_libraries(
google-cloud-cpp::common
google-cloud-cpp::rest_internal
nlohmann_json::nlohmann_json
Crc32c::crc32c
CURL::libcurl
Threads::Threads)
if (NEED_CRC32C)
target_link_libraries(google_cloud_cpp_storage PUBLIC Crc32c::crc32c)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we can use PRIVATE here because crc32c/crc32c.h isn't used in public headers.
But I keep using PUBLIC in this PR.

endif()
if (WIN32)
target_compile_definitions(google_cloud_cpp_storage
PRIVATE WIN32_LEAN_AND_MEAN)
Expand Down Expand Up @@ -333,6 +335,11 @@ install(
google_cloud_cpp_install_headers(google_cloud_cpp_storage
include/google/cloud/storage)

set(PKGCONFIG_LIBS)
if (NEED_CRC32C)
list(APPEND PKGCONFIG_LIBS crc32c)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we don't need to add crc32c to .pc because it's not used in public headers.
But I keep adding crc32c to .pc in this PR.

endif()

google_cloud_cpp_add_pkgconfig(
"storage"
"The Google Cloud Storage C++ Client Library"
Expand All @@ -348,7 +355,7 @@ google_cloud_cpp_add_pkgconfig(
NON_WIN32_REQUIRES
openssl
LIBS
crc32c
${PKGCONFIG_LIBS}
WIN32_LIBS
ws2_32
bcrypt)
Expand Down
16 changes: 10 additions & 6 deletions google/cloud/storage/internal/crc32c.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,15 @@
#include "absl/crc/crc32c.h"
#define GOOGLE_CLOUD_CPP_USE_ABSL_CRC32C 1
#else
#include <crc32c/crc32c.h>
#define GOOGLE_CLOUD_CPP_USE_ABSL_CRC32C 0
#endif // ABSL_LTS_RELEASE_VERSION
#include <crc32c/crc32c.h>

namespace google {
namespace cloud {
namespace storage_internal {
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN

std::uint32_t ExtendCrc32c(std::uint32_t crc, absl::string_view data) {
return crc32c::Extend(crc, reinterpret_cast<uint8_t const*>(data.data()),
data.size());
}

std::uint32_t ExtendCrc32c(std::uint32_t crc,
storage::internal::ConstBufferSequence const& data) {
for (auto const& b : data) {
Expand All @@ -49,6 +44,10 @@ std::uint32_t ExtendCrc32c(std::uint32_t crc, absl::Cord const& data) {

#if GOOGLE_CLOUD_CPP_USE_ABSL_CRC32C

std::uint32_t ExtendCrc32c(std::uint32_t crc, absl::string_view data) {
return static_cast<std::uint32_t>(absl::ExtendCrc32c(absl::crc32c_t{crc}, data));
}

std::uint32_t ExtendCrc32c(std::uint32_t crc, absl::string_view data,
std::uint32_t data_crc) {
return static_cast<std::uint32_t>(absl::ConcatCrc32c(
Expand All @@ -71,6 +70,11 @@ std::uint32_t ExtendCrc32c(std::uint32_t crc, absl::Cord const& data,

#else

std::uint32_t ExtendCrc32c(std::uint32_t crc, absl::string_view data) {
return crc32c::Extend(crc, reinterpret_cast<uint8_t const*>(data.data()),
data.size());
}

std::uint32_t ExtendCrc32c(std::uint32_t crc, absl::string_view data,
std::uint32_t /*data_crc*/) {
return ExtendCrc32c(crc, data);
Expand Down
Loading