Skip to content

[23.x] Backport some changes necessary for google-cloud-cpp resp. conda-forge#12987

Merged
fowles merged 3 commits intoprotocolbuffers:23.xfrom
h-vetinari:backports_23.x
Jun 6, 2023
Merged

[23.x] Backport some changes necessary for google-cloud-cpp resp. conda-forge#12987
fowles merged 3 commits intoprotocolbuffers:23.xfrom
h-vetinari:backports_23.x

Conversation

@h-vetinari
Copy link
Copy Markdown
Contributor

@h-vetinari h-vetinari commented Jun 5, 2023

I don't know what the procedure is on backporting to maintenance branches, but I thought I'd try by opening a PR.

In conda-forge, we'd definitely need:

CC @fowles @mkruskal-google

coryan and others added 2 commits June 6, 2023 10:19
This is a macro on some (older) versions of GCC, and macOS, and Windows. Sigh. I moved the `#undef` block to a common section. I also took the opportunity to add a regression test for all these macros that need to be `#undef`'d.

Part of the work for googleapis/google-cloud-cpp#8125

Closes protocolbuffers#12903

PiperOrigin-RevId: 535649278
…ocolbuffers#12978)

This additional if  is necessary as of abseil 20230125.3 when abseil is consumed via add_subdirectory,
the abseil_dll target  is named abseil_dll, while if abseil is consumed via find_package, the target is called `absl::abseil_dll` .

Once abseil/abseil-cpp#1466 is merged and released in the minimum version of  abseil required by protobuf, it is possible to always link `absl::abseil_dll` and `absl::abseil_test_dll` and remove the if.

You may wonder how linking worked at all before when `protobuf_ABSL_PROVIDER STREQUAL "package"`, as `abseil_dll` was not an imported target defined by `find_package(absl)`. The reason behind this is that if a name that is not an imported target is passed to `target_link_libraries`, it is just regarded as a C++ library name. So, in the end the `abseil_dll` library was correctly linked, simply all the transitive usage requirements defined by the `absl::abseil_dll` target were not propagated, that could lead to compilation errors if abseil was compiled with the `ABSL_PROPAGATE_CXX_STD` CMake option enabled.

Closes protocolbuffers#12978

COPYBARA_INTEGRATE_REVIEW=protocolbuffers#12978 from traversaro:patch-1 39dd074
PiperOrigin-RevId: 537990391
@h-vetinari h-vetinari requested review from a team as code owners June 5, 2023 23:23
@h-vetinari h-vetinari requested review from deannagarcia and removed request for a team June 5, 2023 23:23
Copy link
Copy Markdown
Contributor Author

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

There were some trivial conflicts here that I resolved while backporting #12903. Here's some short comments about where they appeared.

Comment thread src/google/protobuf/port_def.inc
Comment thread src/google/protobuf/port_undef.inc
Comment thread src/google/protobuf/port_def.inc
@fowles fowles added 🅰️ safe for tests Mark a commit as safe to run presubmits over 23.x labels Jun 6, 2023
@github-actions github-actions Bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jun 6, 2023
@fowles fowles added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jun 6, 2023
@github-actions github-actions Bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jun 6, 2023
@h-vetinari
Copy link
Copy Markdown
Contributor Author

Forgot one: we also need fc1c551 for #12932

@fowles fowles added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jun 6, 2023
@github-actions github-actions Bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jun 6, 2023
@fowles fowles merged commit 727ece5 into protocolbuffers:23.x Jun 6, 2023
@h-vetinari h-vetinari deleted the backports_23.x branch June 6, 2023 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants