Conversation
|
@luhenry since the only merge option is "squash and merge" I would prefer you to open a new PR with all the claude related changes, otherwise it will be squashed into the |
thank you! I'll do a proper review later today for the protobuf pipeline |
threexc
left a comment
There was a problem hiding this comment.
LGTM. Glad to see opportunities to offload some parts of the build to the default runners.
As an aside, Mark originally said this on our protobuf issue in wheel_builder:
We may not want to build this straight away. There is a pure Python binary available, and building the wheels requires bazel. It's not clear at the time of writing whether bazel supports riscv64.
|
Bazel is only used to build the sdist on x86, then the bdist is built from the sdist (without bazel) on riscv64. It's documented upstream. The pure python version doesn't use the faster native library; see https://github.com/protocolbuffers/protobuf/tree/main/python#implementation-backends. |
| name: Build protobuf wheels (riscv64) | ||
|
|
There was a problem hiding this comment.
can you add a link to the original file on protobuf repo? like you did for numpy? it's easier for review
There was a problem hiding this comment.
https://github.com/protocolbuffers/protobuf doesn't make releases on GitHub, or not that I could find.
protobuf's setup.py cannot build from the git checkout (upstream's python README is explicit about this); the self-contained sdist that bundles the upb/utf8_range C sources and generated *_pb2.py files is assembled by Bazel via //python/dist:source_wheel. Mirror build-cffi.yml's sdist -> bdist split: - python_sdist: Bazel-build the architecture-independent sdist once on x86. - linux: build the riscv64 bdist from that sdist with cibuildwheel. The bundled C sources compile into google._upb._message with no protoc, Bazel, or external native dependency, so no CIBW_BEFORE_BUILD is needed. The test asserts the upb backend is active and round-trips a well-known type. - publish: hand off to the shared publish-wheels action. The workflow input is the git tag (e.g. 35.1); the Python package version (7.35.1) is derived from the built sdist filename. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
What
Adds
.github/workflows/build-protobuf.ymlto build and publishprotobufriscv64 wheels to pypi.riseproject.dev, following the sdist → bdist structure ofbuild-cffi.yml.Relates to #83
Why protobuf differs from cffi
protobuf's
setup.pycannot build from the git checkout — upstream's python README is explicit: "You cannot build from setup.py using the GitHub repo or the GitHub source tarball." The self-contained sdist (bundling theupb/utf8_rangeC sources and generated*_pb2.pyfiles) is assembled by Bazel via//python/dist:source_wheel.Jobs
python_sdist— Bazel-builds the sdist once on x86. The sdist is architecture-independent ("always the same and does not depend on a local version of Python"), so there's no reason to run Bazel/protoc on riscv64. bazelisk is installed and pinned to Bazel 8.5.2.linux— builds the riscv64 bdist from that sdist with cibuildwheel (matrix: cp312/cp313/cp314/cp314t). The bundled C sources compile intogoogle._upb._messagewith no protoc, Bazel, or external native dependency — so noCIBW_BEFORE_BUILD. The test asserts the upb backend is active (not a silent fall-back to pure-Python) and round-trips a well-known type.publish— hands off to the sharedpublish-wheelsaction, which also auto-createsdocs/packages/protobuf.yaml.Versioning
The workflow input is the git tag (e.g.
35.1); the Python package version (7.35.1) is derived from the built sdist filename, since protobuf's tag scheme (vNN.M) differs from its Python version (7.NN.M).Notes
7.35.1sdist viapip wheelproduces a working upb extension.🤖 Generated with Claude Code