-
-
Notifications
You must be signed in to change notification settings - Fork 35
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 5.26.1 #215
protobuf 5.26.1 #215
Conversation
…nda-forge-pinning 2024.05.06.07.27.40
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
In order to enable bumping the default protobuf version from protobuf_24 to protobuf_25, we address the build failure of pythonPackages.protobuf against that version. Protobuf's python package is moving away from cpp backend in favor of a µpb backend. (See: https://github.com/protocolbuffers/protobuf/tree/main/upb) The work on that seems to have lead to the introduction of a broken test in "minimal_test.py": protocolbuffers/protobuf@501ecec I suspect that this is not an issue on the nixpkgs packaging end but rather that this file is uncovered code upstream. I don't know enough about Bazel to be sure, but it looks like that file is not part of their protobuf/python/BUILD.bazel file. (I wanted to prove that in protocolbuffers/protobuf#16888 but couldn't trigger upstream's CI) So for now let's just skip that file. Note that protobuf_26.tests.pythonProtobuf is still broken. This is due to the fact that upstream removed support for building the library directly from the GitHub repo. (See: protocolbuffers/protobuf#15708) Conda packaging is also currently struggling with this: conda-forge/protobuf-feedstock#215 Related: NixOS#264902
I want to do 2-3 more cleanups of possibly unnecessary things in the Windows build, but this is already ready for review, and I would like to merge it on my own if I get a positive review. |
Spectacular work! ❤️🙏 I was wondering how we can check that the |
Yes, they are statically linked, and there is no system libs option. I would probably be able to build one, but I don't have the patience for this atm. We would have to copy what Tensorflow does over here. |
From my side, this is ready for a merge. @h-vetinari feel free to add what you want on top. There is at least one unit test that checks for |
OK, I'm fine to merge this for now. We'll have to keep iterating anyway going forward. I'd also tend towards not migrating 5.26, as that seems to have already been fully dropped upstream (i.e. 5.25, 5.27 & 5.28 receive patch releases, but not 5.26). My preference would be to build 5.27.5 against the new abseil, bootstrap a recent grpc on top (1.66?), and then try for a migration. We can also discuss this in conda-forge/conda-forge-pinning-feedstock#4075 |
Then, I'll merge this and spent some time on migrating the patches for each release until we hit the newest. If we at one point still need an older release, we then have them. |
I don't think we need the older patch releases for 5.27.x; I think we could go directly for 5.27.5 from here, but if you wanna build out the whole line, go for it. At least the libprotobuf bits should be there already.
We might want to remove the run-deps here then for now |
oh well. 🙃 |
We can remove them in a follow-up PR. Packages with smaller dependencies will be preferred. |
Thank you, @xhochy. 🙏 |
I know, it's no big deal. |
Though we might want to still have a run-constraint? I guess we'd open ourselves up to some possibly weird interactions if we let the versions of |
This is going to be a bigger operation I fear - just opening this PR as a status update for now. Protobuf removed the usual
setup.py
approach, and we now either need to deal with the PyPI sources (harder to patch) or build with bazel.Also, the new way to build python from source seems to just rebuild a lot of stuff we already have in libprotobuf (e.g.
utf8_range
), and more importantly, alsoupb
, which is also being vendored by grpc, causing potential clashes there. I had tried to avoid this in protocolbuffers/protobuf#12927 but to no avail (c.f. also #170).The C++ backend is also removed, so I guess it's not even possible anymore to build atop of
libprotobuf
.