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

[protobuf] Update protobuf to v25.x #3

Merged
merged 1 commit into from Dec 20, 2023
Merged

Conversation

nephros
Copy link

@nephros nephros commented Dec 9, 2023

This updates protobuf to the latest LTS branch, v25.

Reason is that several packages (e.g. on OBS/Chum) are starting to require newer
protobuf versions.

There are some major changes in the packaging:

  • upstream versioning scheme changed, see https://protobuf.dev/support/version-support
  • use cmake instead of autotools
  • introduce a minimum version for the abseil dependency, use pkgconfig for parts of that
  • because of the switch to cmake, drop support for building static libraries
  • Package the new utf8_range library

See also:

Depends-On: sailfishos/abseil-cpp#2

@nephros
Copy link
Author

nephros commented Dec 9, 2023

because of the switch to cmake, drop support for building static libraries

The current cmake setup does not support building both static and shared libraries at the same time, at least not without some hackish usage of cmake (build twice in two separate builddirs, install twice).

Questions:

  1. Are the -static packages used/needed by anything or is dropping OK?
  2. If no, what is the preferred way of supporting the static build?
  • %bcond/macros in the spec?
  • double-cmake-build-and-install?
  • separate .spec file?
  • other

Some experiments with %bcond and friends are at https://github.com/nephros/protobuf/tree/lts-25-static

@nephros
Copy link
Author

nephros commented Dec 9, 2023

  • Package the new utf8_range library

These are always built statically, see also protocolbuffers/protobuf#14958

I have for now lumped them into the -devel package, should those be split out from that, similar to the -lite packages?

@nephros
Copy link
Author

nephros commented Dec 9, 2023

  • Package the new utf8_range library

This also installs a %{_includedir}/java/core/src/main/java/com/google/protobuf/java_features.proto file, which I %exclude for now.
Is that ok, or shall it be added?

@mlehtima
Copy link

mlehtima commented Dec 9, 2023

You can see that Fedora packaging also dropped static package some time ago https://src.fedoraproject.org/rpms/protobuf/blob/rawhide/f/protobuf.spec. Need to be tested if there are any issues after that.

@nephros
Copy link
Author

nephros commented Dec 9, 2023

Fallout expected from this change:

There seem to be a lot of issues reported around the various bug trackers, mainly they are about issues with:

  • protobuf .pc files not including required dependencies
  • linking errors against abseil
  • headers created with protobuf-compiler requiring protobuf includes which were not required before. I.e. google/protobuf/port_def.inc

As noted in the abseil PR, these packages are dependants of protobuf:

  • buteo-sync-plugin-carddav: compiles without changes
  • buteo-sync-plugins-qt5: compiles without changes
  • commhistory-daemon: compiles without changes
  • contactsd: compiles without changes
  • libcommhistory-qt5: some changes may be required
  • libphonenumber: changes/patching required
  • nemo-qml-plugin-contacts-qt5: compiles without changes
  • nemo-qml-plugin-filemanager: compiles without changes
  • nemo-qml-plugin-messages-internal-qt5: not tested

In my tests, this results in the following dependent packages requiring changes:

  • libphonenumber: needs patching because of pkgconfig/cmake detection issues, leading to a linking error. My variant for fixing this is at libphonenumber:bump
  • libcommhistory-qt5: no changes required, but will fail to compile due to missing google/protobuf/port_def.inc header IF libphonenumber-devel does not pull in protobuf-devel (or libcommhistory-qt5 adds that dep). EDIT: wrong, created that problem myself.

@mlehtima
Copy link

mlehtima commented Dec 9, 2023

With updated libphonenumber (https://github.com/sailfishos/libphonenumber/tree/update) I had no issues using this branch to build all the packages which need protobuf and the packages requiring libphonenumber.

@mlehtima
Copy link

mlehtima commented Dec 9, 2023

The java file you mentioned is useless for us and can be excluded or even removed in install step.

@nephros
Copy link
Author

nephros commented Dec 10, 2023

I guess this is as far as I can take it, marking ready.

@nephros nephros marked this pull request as ready for review December 10, 2023 09:39
Copy link
Contributor

@pvuorela pvuorela left a comment

Choose a reason for hiding this comment

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

Could squash the commits.

rpm/protobuf.spec Outdated Show resolved Hide resolved
rpm/protobuf.spec Outdated Show resolved Hide resolved
rpm/protobuf.spec Outdated Show resolved Hide resolved
rpm/protobuf.spec Outdated Show resolved Hide resolved
Copy link
Contributor

@pvuorela pvuorela left a comment

Choose a reason for hiding this comment

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

Thanks. The version handling part looks a lot cleaner now.

@pvuorela pvuorela merged commit 21d2e00 into sailfishos:master Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants