Copilot commented on code in PR #12820:
URL: https://github.com/apache/gluten/pull/12820#discussion_r3811453820
##########
.github/workflows/delta_spark_ut.yml:
##########
@@ -313,27 +328,31 @@ jobs:
# tests may need. We deliberately do NOT install the full `curl`
# package -- it conflicts with the pre-installed curl-minimal.
yum install -y java-17-openjdk-devel which findutils gzip python3
+ case "$(uname -m)" in
+ x86_64) ARCH=amd64 ;;
+ aarch64) ARCH=arm64 ;;
+ *) echo "unsupported arch: $(uname -m)" >&2; exit 1 ;;
+ 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
export JAVA_HOME=/usr/lib/jvm/java-17-openjdk
export PATH=$JAVA_HOME/bin:$PATH
java -version
git --version
curl --version | head -n 1
+ python3 --version
+ gh --version
+ jq --version
- - name: Cache sbt / Ivy / Coursier
- uses: actions/cache@v4
+ - name: Get sbt / Ivy / Coursier from Apache Stash
+ uses:
apache/infrastructure-actions/stash/restore@0ba14156c9f4c3cfbe4b0c9f36339ab0f8d81e53
with:
- path: |
- /root/.sbt
- /root/.ivy2
- /root/.cache/coursier
- # Intentionally NOT keyed by ${{ matrix.shard }} -- all shards share
- # the same dependency tree, so a single shared cache (with parallel
- # save races resolved by GH on a first-write-wins basis) gives the
- # best storage / hit-rate tradeoff.
+ # Stash restore accepts one destination. The save step preserves the
+ # three selected cache paths relative to their common /root
directory.
Review Comment:
The comment states the save step preserves the three cache paths relative to
a common `/root` directory, but the save step later uploads three absolute
paths (not `/root`). Either update the comment to match the actual save
behavior, or align save/restore by saving `/root` (and controlling included
content) so the intent is correctly documented and less error-prone.
##########
.github/workflows/delta_spark_ut.yml:
##########
@@ -197,14 +195,25 @@ jobs:
with:
name: delta-spark-ut-native-lib-centos-7-${{ github.sha }}
path: ./cpp/build/
- - name: Cache Maven repository
- uses: actions/cache@v4
+ - name: Install Stash restore dependencies
+ run: |
+ set -euo pipefail
+ yum install -y python3
+ case "$(uname -m)" in
+ x86_64) ARCH=amd64 ;;
+ aarch64) ARCH=arm64 ;;
+ *) echo "unsupported arch: $(uname -m)" >&2; exit 1 ;;
+ 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:
This installs `gh` and `jq` by downloading executables over HTTPS without
integrity verification. To reduce supply-chain risk, verify SHA256
checksums/signatures from the release artifacts (or install via the distro
package manager where feasible) before placing binaries into `/usr/local/bin`.
##########
.github/workflows/delta_spark_ut.yml:
##########
@@ -152,14 +152,11 @@ jobs:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v4
- - name: Get Ccache
- uses: actions/cache/restore@v4
+ - name: Get Ccache from Apache Stash
+ uses:
apache/infrastructure-actions/stash/restore@0ba14156c9f4c3cfbe4b0c9f36339ab0f8d81e53
with:
path: '${{ env.CCACHE_DIR }}'
- key: ccache-delta-spark-ut-centos7-release-default-${{github.sha}}
- restore-keys: |
- ccache-delta-spark-ut-centos7-release-default
- ccache-centos7-release-default
+ key: ccache-centos7-release-default-${{
hashFiles('ep/build-velox/src/**') }}
Review Comment:
Using `hashFiles('ep/build-velox/src/**')` can be expensive if that
directory contains many files, since the workflow must compute a content hash
across the entire tree at runtime. Consider hashing a narrower set of inputs
that actually influence the ccache contents (e.g., specific build
scripts/manifests/toolchain configs), to reduce key computation time while
still keeping the cache safely invalidated.
##########
.github/workflows/delta_spark_ut.yml:
##########
@@ -313,27 +328,31 @@ jobs:
# tests may need. We deliberately do NOT install the full `curl`
# package -- it conflicts with the pre-installed curl-minimal.
yum install -y java-17-openjdk-devel which findutils gzip python3
+ case "$(uname -m)" in
+ x86_64) ARCH=amd64 ;;
+ aarch64) ARCH=arm64 ;;
+ *) echo "unsupported arch: $(uname -m)" >&2; exit 1 ;;
+ 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:
The Stash restore dependency installation logic (ARCH detection + `gh`/`jq`
installs) is duplicated across jobs. Consider extracting this into a shared
script in the repo (e.g., under `.github/workflows/util/`) or a reusable
composite action so version bumps and security hardening (checksums, retries,
etc.) only need to be done in one place.
--
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]
