Skip to content
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.

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

Make rprotobuf compatible with protobuf >= 22.x #93

Merged
merged 5 commits into from
Dec 9, 2023

Conversation

TeoGiane
Copy link
Contributor

This should close #92

Still a WIP.

I tried to manage both cases introducing a preprocessor directive checking the version of protobuf. I tried to preserve as much as possible the existing code. Maybe we can improve src/RSourceTree.h/src/RSourceTree.cpp, since a lot of code is re-used.

On my machine (Arch Linux, gcc 13.1.1, protobuf 23.4-1) now the compilation is successfull. I have also tested compilation on Ubuntu 20.04 with protobuf 3.14.0, which relies on the old API.

@eddelbuettel
Copy link
Owner

Thanks for this! It looks pretty promising and we would get by without even a configure check.

I presume you have all the relevant piece (incl Abseil) on Arch?

@TeoGiane
Copy link
Contributor Author

Thanks for this! It looks pretty promising and we would get by without even a configure check.

I think so. Looking at the Protocol Buffers C++ installation instruction, abseil is a requirement for installation. Hence, in case a user would prefer to build protobuf and protoc from source needs to install abseil as well, starting from version 22.0. In case user would prefer to rely on a binary version in its own package repository, abseil would be installed as well as dependency (that is what has happened to me, too).

I presume you have all the relevant piece (incl Abseil) on Arch?

On Arch, abseil is installed as a dependency of the protobuf package, with the name of abseil-cpp (see here). Having a quick look in case of Ubuntu/Debian users, I guess the corresponding name is libabsl-dev (see here).

Question: What about Windows users? Do we need some extra effort for them?

I'll soon revise my changes in the code to reduce/avoid redundancy. All suggestions are appreciated!
Thanks a lot.

@eddelbuettel
Copy link
Owner

What about Windows users?

They usually get the package as a binary from CRAN, and CRAN relies on the ProtoBuf library it has prepared. Those tend to 'stick' for a while. With your #ifdef it still build, they will simply have an older ProtoBuf library powering it (not unlike Linux using a distro-supplied older one).

@bastistician
Copy link

Just ran into the same issue (#95) on Alpine 3.19 that ships protobuf 24.4. I can confirm that this PR also fixes #95.

@eddelbuettel
Copy link
Owner

Ok, ball in my court, with my head hanging slightly in shame for having sat on this for so long. I will add a yaml file to run an action with this utilising Alpine 3.19 while Debian / Ubuntu take their time to update to newer Protocol Buffer library versions.

@eddelbuettel
Copy link
Owner

As for the older questions "but what about Windows users": someone would need to assist @kalibera to get to a newer / current Protocol Buffers library for the setup at CRAN. Ditto for macOS and @s-u.

@eddelbuettel eddelbuettel merged commit db3c796 into eddelbuettel:master Dec 9, 2023
4 checks passed
@eddelbuettel
Copy link
Owner

Thank you both -- the @TeoGiane for the rather nice and clean PR, and to @bastistician for the wake-up call and demo of how to actually test this. CI has been added so I merged this now.

@kalibera
Copy link

As for the older questions "but what about Windows users": someone would need to assist @kalibera to get to a newer / current Protocol Buffers library for the setup at CRAN. Ditto for macOS and @s-u.

I've updated protobuf in development version of Rtools and rprotobuf passes its checks. Pending further testing, it should be available in the next release of Rtools. Ideally, anyone wanting a major update (like in this case) or addition of a library would first contribute such change upstream to MXE and then ping the Rtools maintainer via R bugzilla to include it in Rtools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rprotobuf incompatible with protobuf >= 22.0
4 participants