Figure out upb-situation vs. grpc; do not remove deprecated C++-backend until then

[h-vetinari]

Hi all

I help package various things for conda-forge, including protobuf, grpc, abseil, etc. With the deprecation of the C++ backend for the protobuf python bindings, we're in a bit of a problem, because grpc will already install its vendored upb into $PREFIX/lib.

As far as I understand from discussions with @coryan:

upb makes no guarantees about using the generated code with any version of upb other than the version used to generate it.

This is problematic because we now cannot change to the upb-backend for protobuf without putting upb on the path twice (even if one of them is in site-packages/lib), especially not if those two versions can be incompatible with each other (symbol clashes, segfaults, and other fun would ensue).

I don't have the full picture here, but from what I can tell, the options are:

• agree on a common upb version between protobuf & grpc (unlikely?)
• "mangle" the respective upb-installations so they use a private name that will not lead to symbol clashes (quite a bit of complexity)
• force one or both projects to always build upb statically, and absorb all relevant symbols into the respective builds (not a good idea from our POV)
• aggressive SO-versioning of upb (no symlinks to bare libupb.so), so that different versions can coexist (which libprotobuf started doing to some degree for 4.x already)
• something else?

Before that situation is not solved, I implore you not to actually execute the deprecation of the C++ backend, because without that we'd be up a certain creek without a paddle.

[fowles]

We do not have a short term plan to remove the C++-backend. Likely that will be on the radar in a year or so, but we can probably figure out a better answer here first.

[coryan]

@veblush FYI

[haberman]

force one or both projects to always build upb statically, and absorb all relevant symbols them into the respective builds (not a good idea from our POV)

FWIW this is what we do with the Python wheel currently. The native extension links upb statically but uses a version script to hide all symbols except the entry point PyInit__message.

$ wget https://files.pythonhosted.org/packages/14/23/f3b852093f3a7031049313913e04431e7a32bd9752adfaf79d1193cf05b1/protobuf-4.23.2-cp37-abi3-manylinux2014_x86_64.whl
$ unzip protobuf-4.23.2-cp37-abi3-manylinux2014_x86_64.whl
$ ldd google/_upb/_message.abi3.so
	linux-vdso.so.1 (0x00007ffeb7777000)
	libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f36d0800000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f36d0aac000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f36d061f000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f36d0bec000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f36d0a8c000)
$ readelf --dyn-syms google/_upb/_message.abi3.so | grep -v " UND "

Symbol table '.dynsym' contains 151 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
   148: 0000000000014420   428 FUNC    GLOBAL DEFAULT   12 PyInit__message
   149: 0000000000009000     0 FUNC    GLOBAL DEFAULT    9 _init
   150: 000000000002e428     0 FUNC    GLOBAL DEFAULT   13 _fini

For the moment I think this is an elegant solution to the problem, as it makes the extension self-contained and allows maximum linker stripping. Python Protobuf and gRPC do not actually share any data at a C level, so there is no compatibility reason to have them share a upb shared library. Size-wise, upb is very small, so the duplication will have little effect on binary size.

At some point in the future we may want to allow Python <-> C(++) sharing of upb messages. At that point I think it would make sense to have a .so that they both use. But at the moment that seems premature, because neither Python nor gRPC uses upb in their public interface. The fact that they use upb is an internal implementation detail.

There are also some tricky questions around how sharing would work, particularly when it comes to schemas and .upb.c files. If possible, it would help if we could defer those tricky questions to a later date when we can face them head-on.

That said, I would also be open to using SO versioning to give every release of upb its own major version number.

[h-vetinari]

Just saw that grpc switched to a submodule for upb instead of outright vendoring, which is definitely a step in the right direction! Not sure what other plans people have come up with in the meantime (i.e. if there will be a shared upb commit that gprc & protobuf point to), would be happy to have an update if there's anything to share already.

[github-actions]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

[h-vetinari]

Not stale.

[veblush]

Just saw that grpc switched to a submodule for upb instead of outright vendoring, which is definitely a step in the right direction!

gRPC and upb are going forward in this direction but it's not happened yet as upb is working to add cmake support. Once upb has it, gRPC will completely treat it as a regular dependency like protobuf.

[github-actions]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

[h-vetinari]

I just noticed #15671 through the announcement about removing setup.py support. What's the situation with upb between protobuf and grpc now? Or any news about the following?

@haberman: That said, I would also be open to using SO versioning to give every release of upb its own major version number.

@veblush: gRPC and upb are going forward in this direction but it's not happened yet as upb is working to add cmake support. Once upb has it, gRPC will completely treat it as a regular dependency like protobuf.

The most recent grpc 1.62 still vendors upb and ships a bunch of libupb*.so artefacts (in shared builds), so we cannot simply switch the python bindings to use upb (more details in OP).

@h-vetinari: force one or both projects to always build upb statically, and absorb all relevant symbols them into the respective builds (not a good idea from our POV)

@haberman: FWIW this is what we do with the Python wheel currently.

To understand correctly, you'd recommend building a static upb as part of the building the python bindings (but not shipping it)?

@haberman @fowles @mkruskal-google @veblush @yashykt

[h-vetinari]

grpc 1.63 still vendors upb and the C++ backend was removed in protobuf. What's the situation here now?

[haberman]

To understand correctly, you'd recommend building a static upb as part of the building the python bindings (but not shipping it)?

My goal is that our source package, as published on PyPI, would do this automatically without any special intervention on your part.

It looks like that is not currently the case. I just tried this and got:

$ wget https://files.pythonhosted.org/packages/d2/e5/7e22ca7201a6b1040aae7787d0fe6cd970311da376a86fdafa5182be1d1b/protobuf-5.26.1.tar.gz
$ tar zxf protobuf-5.26.1.tar.gz
$ cd protobuf-5.26.1
$ python setup.py build
$ readelf -W --dyn-syms build/lib.linux-x86_64-cpython-311/google/_upb/_message.cpython-311-x86_64-linux-gnu.so | grep -v " UND "

Symbol table '.dynsym' contains 773 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
   148: 0000000000023d20    28 FUNC    GLOBAL DEFAULT   12 _upb_Arena_SwapIn_dont_copy_me__upb_internal_use_only
   149: 000000000002cf00     8 FUNC    GLOBAL DEFAULT   12 upb_ExtensionRange_Options
   150: 0000000000031670    16 FUNC    GLOBAL DEFAULT   12 upb_MessageDef_ExtensionRange
   151: 0000000000022a00   115 FUNC    GLOBAL DEFAULT   12 upb_inttable_lookup
   152: 0000000000023ff0   129 FUNC    GLOBAL DEFAULT   12 upb_Array_New
[...]

We are currently exporting more than 600 symbols in .dynsym, including large swathes of the upb API. This could cause problems if gRPC is also shipping a upb.so. We should only be exporting a single symbol PyInit__message.

We should fix this by shipping our linker script version_script.lds in the source package and adding a command-line flag to our setup.py to reference the linker script. If we do that, then a normal build of our source package should result in a module that does not expose any of the upb API, which should avoid conflicting with anything else.

Would that be a satisfactory resolution for this issue?

[h-vetinari]

Thanks for the quick response!

My goal is that our source package, as published on PyPI, would do this automatically without any special intervention on your part.

I understand that this is desirable for the general case on PyPI, but it is at odds with how we need to distribute things (we can control versions and ABI-compatibility to a high degree, so if there's a lib providing the required symbols, we want to use that instead of doing a duplicated rebuild of those symbols in the python bindings). In the past, we've patched the builds of the protobuf python bindings to link directly against libprotobuf (the C++ bits), which worked well for us.

The fact that grpc ships libupb.so and exposes all those symbols is obviously also a big headache there.

Would that be a satisfactory resolution for this issue?

What I'd love is to have a semi-supported way to build the python bindings against existing libraries, i.e. don't rebuild libprotobuf, upb, utf8_range, etc. as necessary, as well as a upb build that can be shared between grpc and the protobuf python bindings. If unicorns are in short supply, I'd settle for ensuring that no symbols leak out of the protobuf python bindings which may collide with libprotobuf, upb, utf8_range, present in the same environment.

[haberman]

What I'd love is to have a semi-supported way to build the python bindings against existing libraries

There are two main obstacles to this:

The first is that the upb API is unstable. Unlike our user-level APIs in Python, Ruby, PHP, etc. the upb API is only meant for our internal use, and it breaks semi-frequently. Because it is not a user-level API, we don't track these breaking changes with SemVer, so there is no version number to help you determine how to keep Python and upb compatible.

The Python source packages published by us will take both Python/upb and upb from a single revision in our repo, and thus will always perfectly match. If you are trying to match Python/upb and upb that were distributed separately, there is no straightforward way of doing this.

The second is that the upb API has many optional parts, so libupb.so is not a stable concept. I just did a dynamic build of gRPC from v1.62.0 and it installed these shared libraries:

$ find . | grep upb.*so$
./lib/libupb_json_lib.so
./lib/libupb_mem_lib.so
./lib/libupb_message_lib.so
./lib/libupb_textformat_lib.so
./lib/libupb_base_lib.so

These file names were created by gRPC's build system -- upb does not standardize these artifact names. Also, it looks like gRPC is not including some parts of upb that are used by Python/upb, such as

protobuf/upb/util/def_to_proto.h

Lines 21 to 22 in f70d90b

	google_protobuf_DescriptorProto* upb_MessageDef_ToProto(const upb_MessageDef* m,
	upb_Arena* a);

If unicorns are in short supply, I'd settle for ensuring that no symbols leak out of the protobuf python bindings which may collide with libprotobuf, upb, utf8_range, present in the same environment.

I think this is what I proposed in my previous message?

[h-vetinari]

The first is that the upb API is unstable.

I get this. It would be much easier though if grpc used an unmodified submodule, rather than vendoring it + potentially layering changes on top.

If it was a submodule, we could could probably find an intersection of compatible upb versions commits that can be shared by some protobuf version and some grpc version. I know that this use case probably won't ever be explicitly supported, but if the test suite passes on both sides (grpc/grpcio resp. protobuf python bindings), I'd be happy to do that ourselves.

Even better would be if grpc/protobuf agreed on the upb that they vendor, or that upb gets a version (though I understand that this is not realistic for now).

I think this is what I proposed in my previous message?

Yeah, that would at least be workable. I'm not a huge fan though, not least because - as you showed - it can happen relatively easily that these symbols start leaking.

[h-vetinari]

@haberman: That said, I would also be open to using SO versioning to give every release of upb its own major version number.

Just scrolled upthread a bit, and this would be a pretty good solution IMO, all things considered.

[haberman]

Unfortunately I made that comment before considering that we don't have a clear set of APIs to put into libupb.so, in order to version it.

[h-vetinari]

@haberman, given that no other solution seems to be in play, could you perhaps open a PR that does what you suggested above:

We should fix this by shipping our linker script version_script.lds in the source package and adding a command-line flag to our setup.py to reference the linker script. If we do that, then a normal build of our source package should result in a module that does not expose any of the upb API, which should avoid conflicting with anything else.

I'm happy to test patches against our infrastructure.

[haberman]

Hi @h-vetinari, I created #17207. Give it a spin and see if it works for you.

To build the Python source package as we distribute it, you can run this command:

$ bazel build python/dist:source_wheel

[github-actions]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago. This issue will be closed and archived after 14 additional days without activity.

[h-vetinari]

Hi @h-vetinari, I created #17207. Give it a spin and see if it works for you.

Hey @haberman, sorry for the long delay, the forced switch to protobuf really did a number on us...

I've picked the change from #17207 into conda-forge/protobuf-feedstock#215, which is where we now finally unblocked the situation after much heartache. I haven't tested the visibility of the upb symbols yet, but I'm hopeful that this should work. 🤞

We'll find out soon once we button up the remaining issues and start migrating our builds over to the newly-built protobuf/grpc.

In any case, that PR looks like it didn't get merged, it should probably be revived?

[haberman]

In any case, that PR looks like it didn't get merged, it should probably be revived?

I've refreshed it in #18467. If it helps your use case, I can submit the change.

[haberman]

Hey @haberman, sorry for the long delay, the forced switch to protobuf really did a number on us...

Which switch do you mean? I don't think I quite understand your setup; I had thought you were building your distribution from our source package. Has our PyPI source package changed in any major way lately?

[h-vetinari]

Which switch do you mean? I don't think I quite understand your setup; I had thought you were building your distribution from our source package. Has our PyPI source package changed in any major way lately?

We're building from source, not the source package. That's partially because we support more architectures than you do (e.g. osx-64 resp. linux-ppc64le), but also because we want to keep building with use_fast_cpp_protos=true, while building against shared libraries (ideally; this point has currently been weakened), rather than vendoring stuff or using static libs (e.g. for abseil). While we'd like to avoid it, we've generally had to patch things on source-level, e.g. we definitely want to avoid using the static msvc runtime, because we can always ship an up-to-date dynamic one in the user environment.

Before, we were building protobuf (our name for the python bindings; based on libprotobuf - the C++ lib / protoc) with a simple python setup.py bdist_wheel --cpp_implementation; that became impossible with the removal of setup.py in favour of bazel.

Consequently we ran into a bunch of issues (e.g. #12947), not limited to the fact that building protobuf with bazel creates a bootstrapping issue for us, as (our) bazel itself depends on grpc/libprotobuf. You can click through conda-forge/protobuf-feedstock#215 to get a sense of it.

The fundamental issue (IMO, from previous such discussions) is that the (third-party) binary distribution case is diametrically at odds to what many projects (and especially projects built by bazel) consider the primary installation channel for their users. It's (often) considered too hard to get right, so it's the last thing on anyone's mind, but such distribution is exactly our use-case, so we keep hitting these things (c.f. bazelbuild/bazel#20947, grpc/grpc#33032, #13726).

We would love to build from an unpatched source if we can make it to a point where our key requirements can be met. I'm happy to contribute back bits and pieces as appropriate, but step 0 in this is to understand the constraints/requirements of cross-platform, rolling, binary distribution (ideally based on shared libs) that conda-forge represents. I'm happy to explain more about that, e.g. how we ensure that ABI isn't broken and users get a compatible set of packages at all times.