ci: use common vcpkg setup script for Windows CI - #4644
Conversation
amoeba
left a comment
There was a problem hiding this comment.
Nice. Could we do a bit of cleanup to simplify areas in CI where we duplicate the logic of parsing the vcpkg version from .env? And also try this for the python-windows job in `packaging.yml'?
diff --git a/.github/workflows/java.yml b/.github/workflows/java.yml
index 1eea72513..2867e1a4d 100644
--- a/.github/workflows/java.yml
+++ b/.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 version 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
- uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
with:
go-version: "${{ env.GO_VERSION }}"
diff --git a/.github/workflows/packaging.yml b/.github/workflows/packaging.yml
index 77a876371..091d5532c 100644
--- a/.github/workflows/packaging.yml
+++ b/.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
popd
- name: Get required Go version
@@ -953,7 +945,7 @@ jobs:
env:
PYTHON_VERSION: "${{ matrix.python_version }}"
# Where to install vcpkg
- VCPKG_ROOT: "${{ github.workspace }}\\vcpkg"
+ VCPKG_ROOT: "${{ github.workspace }}/vcpkg"
steps:
- uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
with:
@@ -990,21 +982,11 @@ jobs:
choco install --no-progress -y cmake --installargs 'ADD_CMAKE_TO_PATH=System'
choco install --no-progress -y visualcpp-build-tools
- - name: Retrieve VCPKG version from .env
- shell: pwsh
- run: |
- pushd adbc
- Select-String -Path .env -Pattern 'VCPKG="(.+)"' | % {"VCPKG_VERSION=$($_.matches.groups[1])"} >> $env:GITHUB_ENV
- popd
-
- name: Install vcpkg
- shell: pwsh
+ shell: bash
run: |
- echo $env:VCPKG_VERSION
- git clone --shallow-since=2024-06-01 https://github.com/microsoft/vcpkg $env:VCPKG_ROOT
- pushd $env:VCPKG_ROOT
- .\bootstrap-vcpkg.bat -disableMetrics
- popd
+ cd adbc
+ ci/scripts/install_vcpkg.sh "$VCPKG_ROOT"
- uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
with:
|
Good catch, shoulda used my own eyes on this a bit more 😅 |
Assisted-by: GPT-5.6 Sol <codex@openai.com>
There was a problem hiding this comment.
Pull request overview
This PR updates Windows CI to use the same vcpkg setup approach/version as other CI by routing workflows through the shared ci/scripts/install_vcpkg.sh script (aligned with Issue #3825).
Changes:
- Switch Windows CI workflows from ad-hoc vcpkg cloning/bootstrapping to the common
install_vcpkg.shscript. - Simplify workflows by removing per-workflow “retrieve vcpkg version” steps and relying on
.envas the source of truth. - Update the vcpkg install script to better handle
.envlines that include trailing comments and to reduce clone size via--shallow-since.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
ci/scripts/install_vcpkg.sh |
Reads vcpkg version from .env and installs/bootstraps vcpkg (now used by Windows CI too). |
.github/workflows/packaging.yml |
Updates packaging jobs (incl. Windows wheels) to install vcpkg via the shared script. |
.github/workflows/native-windows.yml |
Replaces the pinned vcpkg clone step with the shared installer to unify versioning. |
.github/workflows/java.yml |
Stops exporting VCPKG_VERSION and relies on the shared installer to pick up the .env version. |
Suppressed comments (2)
ci/scripts/install_vcpkg.sh:40
vcpkg_destinationis used unquoted ingit clone/pushd. If the destination path contains spaces (e.g., on some self-hosted runners), the clone/pushd will fail. Quote the destination consistently.
git clone --shallow-since=2026-05-01 https://github.com/microsoft/vcpkg ${vcpkg_destination}
pushd ${vcpkg_destination}
.github/workflows/packaging.yml:988
$VCPKG_ROOTshould be quoted when passed to the install script to avoid path-splitting issues on runners where the workspace path contains spaces.
./ci/scripts/install_vcpkg.sh $VCPKG_ROOT
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Closes #3825.
Assisted-by: GPT-5.6 Sol codex@openai.com