s390x binaries included in librdkafka.redist NuGet package and static builds package#5365
s390x binaries included in librdkafka.redist NuGet package and static builds package#5365pranav shah (prashah-confluent) wants to merge 15 commits intomasterfrom
Conversation
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
afe3d09 to
095bcb8
Compare
9b1844d to
c066649
Compare
8270eb8 to
4edace1
Compare
d736613 to
15165b8
Compare
There was a problem hiding this comment.
Pull request overview
This PR bumps librdkafka to v2.14.1 and extends the release/packaging pipeline to produce and bundle prebuilt Linux s390x (glibc/centos8 manylinux) artifacts.
Changes:
- Bump project/version constants to 2.14.1 (vcpkg + public headers) and add a v2.14.1 changelog entry.
- Add
--platformsupport to the release-artifact docker build script and install an extra CentOS Perl dependency. - Add Semaphore CI jobs and NuGet/static package mappings for linux-s390x (glibc/centos8) artifacts.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
vcpkg.json |
Bumps vcpkg manifest version to 2.14.1. |
src/rdkafka.h |
Updates RD_KAFKA_VERSION macro for 2.14.1. |
src-cpp/rdkafkacpp.h |
Updates C++ header RD_KAFKA_VERSION macro for 2.14.1. |
packaging/tools/build-release-artifacts.sh |
Adds docker --platform option support and tweaks CentOS deps. |
packaging/nuget/staticpackage.py |
Adds static bundle mappings for glibc linux s390x. |
packaging/nuget/nugetpackage.py |
Adds NuGet runtime mapping for glibc linux s390x. |
CHANGELOG.md |
Adds v2.14.1 entry noting linux-s390x binaries. |
.semaphore/semaphore.yml |
Adds linux-s390x docker build block and wires it into packaging dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fi | ||
|
|
||
| docker_platform="" | ||
| if [ "$1" = "--platform" ]; then |
There was a problem hiding this comment.
--platform is parsed as --platform=$2 and then shift 2 without validating that a value was actually provided. If someone calls the script with --platform but no argument, this will fail with a generic shift error rather than a clear usage message. Add an explicit check (e.g., ensure $# -ge 2 and $2 is non-empty) and print usage/exit when the argument is missing.
| if [ "$1" = "--platform" ]; then | |
| if [ "$1" = "--platform" ]; then | |
| if [ $# -lt 2 ] || [ -z "$2" ]; then | |
| echo "Usage: $0 [--disable-gssapi] [--platform <docker-platform>] <manylinux-docker-image> <output-path.tgz>" | |
| exit 1 | |
| fi |
| # Linux glibc centos8 s390x without GSSAPI (no external deps) | ||
| Mapping({'arch': 's390x', | ||
| 'plat': 'linux', | ||
| 'dist': 'centos8', | ||
| 'lnk': 'all'}, | ||
| 'librdkafka.tgz', | ||
| './usr/local/lib/librdkafka.so.1', | ||
| 'runtimes/linux-s390x/native/librdkafka.so'), |
There was a problem hiding this comment.
This adds a linux/s390x runtime mapping, but packaging/nuget/packaging.py's magic_patterns currently has ELF checks for linux x64 and arm64 only. That means the s390x .so won't be validated for architecture during packaging, reducing defense-in-depth against accidentally bundling the wrong binary. Consider adding a corresponding magic_patterns entry for (linux, s390x, .so).
| prologue: | ||
| commands: | ||
| - '[[ -z $DOCKERHUB_APIKEY ]] || docker login --username $DOCKERHUB_USER --password $DOCKERHUB_APIKEY' | ||
| - docker run --privileged --rm tonistiigi/binfmt --install s390x |
There was a problem hiding this comment.
The workflow installs QEMU binfmt using tonistiigi/binfmt without a tag or digest, which makes the build non-reproducible and risks sudden CI breakage if the upstream image changes. Pin this image to a specific version tag or digest (similar to how the manylinux images are pinned).
| - docker run --privileged --rm tonistiigi/binfmt --install s390x | |
| - docker run --privileged --rm tonistiigi/binfmt:qemu-v8.1.5 --install s390x |
f5c7769 to
66085fb
Compare
Emanuele Sabellico (emasab)
left a comment
There was a problem hiding this comment.
LGTM, Thanks pranav shah (@prashah-confluent) Devarsh Patel (@Devarsh010) !
No description provided.