ajantha-bhat commented on code in PR #16945:
URL: https://github.com/apache/iceberg/pull/16945#discussion_r3468247316


##########
.github/workflows/spark-ci.yml:
##########
@@ -110,14 +110,21 @@ jobs:
           cache-read-only: true
       - run: echo -e "$(ip addr show eth0 | grep "inet\b" | awk '{print $2}' | 
cut -d/ -f1)\t$(hostname -f) $(hostname -s)" | sudo tee -a /etc/hosts
       - name: Run tests
+        env:
+          SPARK_VERSION: ${{ matrix.spark }}
+          SCALA_VERSION: ${{ matrix.scala }}
+          TEST_GROUP: ${{ matrix.tests }}
         run: |
-          if [[ "${{ matrix.tests }}" == "core" ]]; then
-            projects=":iceberg-spark:iceberg-spark-${{ matrix.spark }}_${{ 
matrix.scala }}:check"
+          if [[ "${TEST_GROUP}" == "core" ]]; then
+            
projects=(":iceberg-spark:iceberg-spark-${SPARK_VERSION}_${SCALA_VERSION}:check")
           else
-            projects=":iceberg-spark:iceberg-spark-extensions-${{ matrix.spark 
}}_${{ matrix.scala }}:check :iceberg-spark:iceberg-spark-runtime-${{ 
matrix.spark }}_${{ matrix.scala }}:check"
+            projects=(
+              
":iceberg-spark:iceberg-spark-extensions-${SPARK_VERSION}_${SCALA_VERSION}:check"
+              
":iceberg-spark:iceberg-spark-runtime-${SPARK_VERSION}_${SCALA_VERSION}:check"
+            )
           fi
-          ./gradlew -DsparkVersions=${{ matrix.spark }} -DscalaVersion=${{ 
matrix.scala }} -DflinkVersions= -DkafkaVersions= \
-            $projects -Pquick=true -x javadoc -DtestParallelism=auto
+          ./gradlew -DsparkVersions="${SPARK_VERSION}" 
-DscalaVersion="${SCALA_VERSION}" -DflinkVersions= -DkafkaVersions= \
+            "${projects[@]}" -Pquick=true -x javadoc -DtestParallelism=auto

Review Comment:
   found this using `actionlint`
   
   ```
   "${projects[@]}" is Bash array expansion.
   
   Before:
   
   projects=":spark-extension:check :spark-runtime:check"
   ./gradlew ... $projects ...
   That relies on unquoted word splitting so the string becomes two Gradle 
arguments. actionlint runs ShellCheck, and ShellCheck flags this as unsafe: 
unquoted variables can accidentally split/glob.
   
   After:
   
   projects=(
     ":spark-extension:check"
     ":spark-runtime:check"
   )
   
   ./gradlew ... "${projects[@]}" ...
   "${projects[@]}" expands each array item as its own safely quoted argument. 
So Gradle still receives:
   
   :spark-extension:check
   :spark-runtime:check
   but without unsafe word splitting.
   
   So behavior is the same, just cleaner and actionlint-safe.
   ```



-- 
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]

Reply via email to