Copilot commented on code in PR #4644:
URL: https://github.com/apache/arrow-adbc/pull/4644#discussion_r3717836417
##########
.github/workflows/packaging.yml:
##########
@@ -810,18 +810,10 @@ jobs:
- name: Install Homebrew dependencies
run: brew install autoconf bash pkg-config ninja
- - name: Retrieve VCPKG version from .env
- id: vcpkg_version
- run: |
- pushd adbc
- vcpkg_version=$(cat ".env" | grep "VCPKG" | cut -d "=" -f2 | tr -d
'"')
- echo "VCPKG_VERSION=$vcpkg_version" | tee -a "$GITHUB_ENV"
- popd
-
- name: Install vcpkg
run: |
pushd adbc
- ci/scripts/install_vcpkg.sh $VCPKG_ROOT $VCPKG_VERSION
+ ci/scripts/install_vcpkg.sh $VCPKG_ROOT
Review Comment:
`$VCPKG_ROOT` should be quoted when passed to the install script to avoid
path-splitting issues on runners where the workspace path contains spaces.
This issue also appears on line 988 of the same file.
##########
ci/scripts/install_vcpkg.sh:
##########
@@ -27,15 +27,15 @@ if [ "$#" -lt 1 ]; then
fi
arrow_dir=$(cd -- "$(dirname -- "$0")/../.." && pwd -P)
-default_vcpkg_version=$(cat "${arrow_dir}/.env" | grep "VCPKG" | cut -d "="
-f2 | tr -d '"')
+default_vcpkg_version=$(cat "${arrow_dir}/.env" | grep "VCPKG" | cut -d "="
-f2 | awk '{print $1}' | tr -d '"')
Review Comment:
`default_vcpkg_version` parsing is too broad: `grep "VCPKG"` also matches
the nearby `.env` comment lines mentioning "VCPKG", which will produce `#`
tokens and/or multiple fields and break `git checkout` when no explicit version
argument is passed (as the workflows now do). Anchor the match to the `VCPKG=`
assignment (and avoid the Useless Use of `cat`).
This issue also appears on line 38 of the same file.
##########
.github/workflows/java.yml:
##########
@@ -127,13 +127,12 @@ jobs:
cache: "maven"
distribution: "temurin"
java-version: 11
- - name: Retrieve Go, VCPKG version from .env
+ - name: Retrieve Go from .env
run: |
(. .env && echo "GO_VERSION=${GO}") >> $GITHUB_ENV
- (. .env && echo "VCPKG_VERSION=${VCPKG}") >> $GITHUB_ENV
- name: Install vcpkg
run: |
- ./ci/scripts/install_vcpkg.sh $VCPKG_ROOT $VCPKG_VERSION
+ ./ci/scripts/install_vcpkg.sh $VCPKG_ROOT
Review Comment:
`$VCPKG_ROOT` should be quoted when passed to the install script to avoid
path-splitting issues on runners where the workspace path contains spaces.
--
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]
