Skip to content

GH-45807: [C++] compatibility patches for protobuf 30#45805

Closed
christian-heusel wants to merge 1 commit intoapache:mainfrom
christian-heusel:protobuf-30
Closed

GH-45807: [C++] compatibility patches for protobuf 30#45805
christian-heusel wants to merge 1 commit intoapache:mainfrom
christian-heusel:protobuf-30

Conversation

@christian-heusel
Copy link

@christian-heusel christian-heusel commented Mar 15, 2025

Rationale for this change

The current version of arrow is not compatible with protobuf 30.0 and beyond, which we are currently rolling out in Arch Linux:
https://archlinux.org/todo/protobuf-300/

What changes are included in this PR?

In order to migrate to the new APIs based on std::string_view we just use a few casts to std::string.

Are these changes tested?

Compile tested only, but there are no problems expected.

@github-actions
Copy link

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@christian-heusel christian-heusel changed the title compatibility patches for protobuf 30 MINOR: [C++] compatibility patches for protobuf 30 Mar 15, 2025
@assignUser
Copy link
Member

Thanks for the contribution, this is not a minor PR though. I will create and link an issue for you!

@assignUser assignUser changed the title MINOR: [C++] compatibility patches for protobuf 30 GH-45807: [C++] compatibility patches for protobuf 30 Mar 15, 2025
@github-actions
Copy link

⚠️ GitHub issue #45807 has been automatically assigned in GitHub to PR creator.

@assignUser
Copy link
Member

Could you apply this diff to fix the format: https://github.com/apache/arrow/actions/runs/13872657236/job/38822487820?pr=45805#step:6:33

@christian-heusel christian-heusel force-pushed the protobuf-30 branch 2 times, most recently from 6a06ed1 to eff85c8 Compare March 15, 2025 13:34
@christian-heusel
Copy link
Author

christian-heusel commented Mar 15, 2025

Thanks for creating the issue, I have added the reference to it in the latest version of the commit an applied the formatting changes 🤗

Edit: Argh, created a whitespace issue with my editor, now applied it with clang-format manually 😅

This version of protobuf has switched a few interface types from
`std::string` to `std::string_view` and therefore introcuces some
breaking changes in the code. Fix these by applying appropriate casts.

Link: https://protobuf.dev/support/migration/#v30
Signed-off-by: Christian Heusel <christian@heusel.eu>
@kou
Copy link
Member

kou commented Mar 16, 2025

@github-actions crossbow submit -g cpp

@github-actions
Copy link

Revision: 6994dac

Submitted crossbow builds: ursacomputing/crossbow @ actions-4471dd3b4d

Task Status
example-cpp-minimal-build-static GitHub Actions
example-cpp-minimal-build-static-system-dependency GitHub Actions
example-cpp-tutorial GitHub Actions
test-alpine-linux-cpp GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-meson GitHub Actions
test-conda-cpp-valgrind GitHub Actions
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1 GitHub Actions
test-debian-12-cpp-amd64 GitHub Actions
test-debian-12-cpp-i386 GitHub Actions
test-fedora-39-cpp GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-20 GitHub Actions
test-ubuntu-22.04-cpp-bundled GitHub Actions
test-ubuntu-22.04-cpp-emscripten GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-24.04-cpp GitHub Actions
test-ubuntu-24.04-cpp-bundled-offline GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions
test-ubuntu-24.04-cpp-gcc-14 GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizer GitHub Actions

@trxcllnt trxcllnt removed their request for review March 16, 2025 15:30
ARROW_RETURN_NOT_OK(
ParseFromBufferImpl(buf, Message::descriptor()->full_name(), &message));
ARROW_RETURN_NOT_OK(ParseFromBufferImpl(
buf, std::string(Message::descriptor()->full_name()), &message));
Copy link
Member

Choose a reason for hiding this comment

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

Instead we can change the ParseFromBufferImpl signature to take a string_view

@tacaswell
Copy link
Contributor

e635e05 #46134 / #46136 is an identical patch (I tried to rebase this onto main and got):

✔ 09:13:03 [belanna] @ git rebase upstream/main
warning: skipped previously applied commit 6994dacad3
hint: use --reapply-cherry-picks to include skipped commits
hint: Disable this message with "git config set advice.skippedCherryPicks false"
Successfully rebased and updated refs/heads/protobuf-30.
~/s/p/a/arrow protobuf-30↑·179↓·1|+1
✔ 09:13:07 [belanna] @ git checkout main
M	cpp/submodules/parquet-testing
Switched to branch 'main'
Your branch is behind 'upstream/main' by 59 commits, and can be fast-forwarded.
  (use "git pull" to update your local branch)

@pitrou
Copy link
Member

pitrou commented Jun 12, 2025

Closing as superseded by #46136

@pitrou pitrou closed this Jun 12, 2025
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