Copilot commented on code in PR #419:
URL: https://github.com/apache/fluss-rust/pull/419#discussion_r2877883607
##########
.github/workflows/release_python.yml:
##########
@@ -94,37 +94,59 @@ jobs:
- name: Install protoc (Windows)
if: runner.os == 'Windows'
- run: choco install protobuf -y
+ run: choco install protoc -y
shell: pwsh
+ # Install protoc in manylinux container (x86_64/aarch64); script shared
via YAML anchor
- uses: PyO3/maturin-action@v1
with:
working-directory: bindings/python
target: ${{ matrix.target }}
command: build
args: --release -o dist -i python3.9
manylinux: ${{ matrix.manylinux || 'auto' }}
+ container: ${{ matrix.os == 'ubuntu-latest' && matrix.target ==
'x86_64' && 'off' || '' }}
+ before-script-linux: &protoc-install |
+ set -e
+ ARCH=$(uname -m)
+ case "$ARCH" in
+ x86_64) ZIP=protoc-27.1-linux-x86_64.zip ;;
+ aarch64) ZIP=protoc-27.1-linux-aarch_64.zip ;;
+ *) echo "Unsupported arch $ARCH"; exit 1 ;;
+ esac
+ curl -sLO
"https://github.com/protocolbuffers/protobuf/releases/download/v27.1/${ZIP}";
+ python3 -c "import zipfile;
zipfile.ZipFile('${ZIP}').extractall('/tmp/protoc_install')"
+ chmod +x /tmp/protoc_install/bin/protoc
+ rm -f "${ZIP}"
+ export PATH="/tmp/protoc_install/bin:$PATH"
+ export PROTOC=/tmp/protoc_install/bin/protoc
Review Comment:
The workflow downloads and executes a prebuilt `protoc` binary from GitHub
releases without verifying its integrity (e.g., SHA256). To reduce supply-chain
risk for release artifacts, add a checksum verification step (and fail closed)
or use a trusted package source available in the manylinux environment.
##########
.github/workflows/release_python.yml:
##########
@@ -94,37 +94,59 @@ jobs:
- name: Install protoc (Windows)
if: runner.os == 'Windows'
- run: choco install protobuf -y
+ run: choco install protoc -y
shell: pwsh
+ # Install protoc in manylinux container (x86_64/aarch64); script shared
via YAML anchor
- uses: PyO3/maturin-action@v1
with:
working-directory: bindings/python
target: ${{ matrix.target }}
command: build
args: --release -o dist -i python3.9
manylinux: ${{ matrix.manylinux || 'auto' }}
+ container: ${{ matrix.os == 'ubuntu-latest' && matrix.target ==
'x86_64' && 'off' || '' }}
Review Comment:
`container` is being set to `'off'` for the Ubuntu x86_64 build even though
`manylinux` is still `auto`. Disabling the container for the Linux x86_64 wheel
risks producing a non-manylinux wheel (or a wheel linked against a newer
glibc), which can reduce portability or cause PyPI upload/runtime issues.
Consider keeping the container enabled and relying on `before-script-linux` to
provide `protoc`, or otherwise ensure the produced wheel remains
manylinux-compliant.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
