protobuf_25.tests.pythonProtobuf: disable faulty test #312957
Conversation
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
ofborg
bot
added
10.rebuild-darwin: 0
There was a problem hiding this comment.
Thanks @knedlsepp for this temporary fix.
I had this on my todo list for a while but got discouraged by the new (from 26) situation.
I agree that dealing with bazel doesn't look appealing at all.
Let's have this merged and then work on v26.
wegank
added
the
12.approvals: 1
I think what we rather want is to reproduce the python source in a simple derivation without Bazel. I think @GaetanLepage and I had looked into that at some point, only we went with trying to decipher the bazel scripts. These proved less than comprehensible. The PR you referenced includes a table (rendered), maybe we can use that as a starting point. I believe we should aim for a better transparency standard than pypi tarballs or bazel... |
| @@ -104,6 +104,8 @@ buildPythonPackage { | |||
| # https://github.com/protocolbuffers/protobuf/commit/5abab0f47e81ac085f0b2d17ec3b3a3b252a11f1 | |||
| # | |||
| "google/protobuf/internal/generator_test.py" | |||
| ] ++ lib.optionals (lib.versionAtLeast protobuf.version "25") [ | |||
| "minimal_test.py" # ModuleNotFoundError: No module named 'google3' | |||
There was a problem hiding this comment.
Is it the interpretation that google3 is a package that comes with upb?
There was a problem hiding this comment.
I don't think so. Grepping through the repo just makes it look like an oversight during moving code around.
There was a problem hiding this comment.
Let's update the comment to reflect that, "Refers to a non-existent module 'google3'. Presumably an oversight". An issue upstream?
There was a problem hiding this comment.
I mentioned it in the commit message. My rationale was that I wanted to keep the comment short as people can always check if they get the same error message.
SomeoneSerge
changed the title
Description of changes
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: #264902
cc @GaetanLepage, we may be able to rebase #264902 on this.
Next steps
I also looked into how to get the same working for the newer protobuf 26 and the upcoming 27 releases. I gave up on the option to generate the source package via
bazel. Instead we will probably want to use the PyPI tarballs generated from the GitHub repo via bazel. One downside is however that these tarballs don't ship any tests, which makes verifying the correctness of the build harder.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.