Copilot commented on code in PR #12610:
URL: https://github.com/apache/gluten/pull/12610#discussion_r3639591324
##########
.github/workflows/velox_backend_arm.yml:
##########
@@ -129,13 +138,22 @@ jobs:
container: apache/gluten:centos-9-jdk8
steps:
- uses: actions/checkout@v4
- - name: Get Ccache
- uses: actions/cache/restore@v4
+ - name: Install Stash restore dependencies # the stash action needs gh
and jq, absent from this container
+ run: |
+ case "$(uname -m)" in
+ x86_64) ARCH=amd64 ;;
+ aarch64) ARCH=arm64 ;;
+ esac
+ curl -fsSL
"https://github.com/cli/cli/releases/download/v2.63.2/gh_2.63.2_linux_${ARCH}.tar.gz";
\
+ | tar -xz -C /usr/local --strip-components=1
"gh_2.63.2_linux_${ARCH}/bin/gh"
+ curl -fsSL -o /usr/local/bin/jq
"https://github.com/jqlang/jq/releases/download/jq-1.7.1/jq-linux-${ARCH}";
+ chmod +x /usr/local/bin/jq
Review Comment:
These steps download and execute binaries without integrity verification. To
reduce supply-chain risk, fetch and validate checksums (e.g., SHA256) from the
projects’ published checksum artifacts (or pin expected hashes in the workflow)
before extracting/executing the binaries.
##########
.github/workflows/velox_backend_x86.yml:
##########
@@ -982,13 +980,22 @@ jobs:
container: apache/gluten:centos-9-jdk8
steps:
- uses: actions/checkout@v4
- - name: Get Ccache
- uses: actions/cache/restore@v4
+ - name: Install Stash restore dependencies # the stash action needs gh
and jq, absent from this container
+ run: |
+ case "$(uname -m)" in
+ x86_64) ARCH=amd64 ;;
+ aarch64) ARCH=arm64 ;;
+ esac
+ curl -fsSL
"https://github.com/cli/cli/releases/download/v2.63.2/gh_2.63.2_linux_${ARCH}.tar.gz";
\
Review Comment:
The `case` statement has no default branch, so if `uname -m` returns
something unexpected (e.g., `arm64` in some environments), `ARCH` will be empty
and the subsequent download URLs will be invalid. Add a fallback branch that
either maps additional known values (like `arm64`) or fails fast with a clear
error and non-zero exit.
##########
.github/workflows/velox_backend_arm.yml:
##########
@@ -56,13 +56,22 @@ jobs:
container: apache/gluten:vcpkg-centos-9
steps:
- uses: actions/checkout@v4
- - name: Get Ccache
- uses: actions/cache/restore@v4
+ - name: Install Stash restore dependencies # the stash action needs gh
and jq, absent from this container
+ run: |
+ case "$(uname -m)" in
+ x86_64) ARCH=amd64 ;;
+ aarch64) ARCH=arm64 ;;
+ esac
+ curl -fsSL
"https://github.com/cli/cli/releases/download/v2.63.2/gh_2.63.2_linux_${ARCH}.tar.gz";
\
+ | tar -xz -C /usr/local --strip-components=1
"gh_2.63.2_linux_${ARCH}/bin/gh"
+ curl -fsSL -o /usr/local/bin/jq
"https://github.com/jqlang/jq/releases/download/jq-1.7.1/jq-linux-${ARCH}";
+ chmod +x /usr/local/bin/jq
+ gh --version && jq --version
Review Comment:
The “Install Stash restore dependencies” script is duplicated across
multiple workflows/jobs, which increases the chance of divergence when versions
or fixes change (e.g., arch mappings, checksum verification). Consider
factoring this into a reusable workflow (`workflow_call`) or a local composite
action (e.g., `.github/actions/install-stash-deps`) so updates are made once
and applied consistently.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
