Update/move to windows 2025-2026 runners - #1844
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details🧰 Additional context used🪛 zizmor (1.25.2).github/workflows/main.workflow.yml[warning] 270-270: code injection via template expansion (template-injection): may expand into attacker-controllable code (template-injection) .github/workflows/python-main.yml[warning] 260-260: code injection via template expansion (template-injection): may expand into attacker-controllable code (template-injection) 🛑 Comments failed to post (1)
🔇 Additional comments (2)
📝 WalkthroughWalkthroughThis PR modernizes CI infrastructure by upgrading Windows runners from ChangesCI Environment Modernization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
MaticTonin
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/python-main.yml (1)
142-152: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider upgrading to
actions/cache@v4for consistency.The workflow uses
actions/cache@v3here butactions/cache@v4elsewhere (lines 238, 300, etc.). For consistency and to use the latest version, consider upgrading this step.♻️ Optional refactor to use v4
- name: Cache vcpkg folder if: runner.os != 'Windows' - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: /home/runner/.vcpkg/ key: vcpkg-${{ matrix.os }} - name: Cache vcpkg folder if: runner.os == 'Windows' - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: C:/.vcpkg/ key: vcpkg-${{ matrix.os }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/python-main.yml around lines 142 - 152, Update the workflow cache step that currently references actions/cache@v3 to actions/cache@v4 so it matches the other cache usages (the steps using "uses: actions/cache@v3" in the vcpkg cache blocks) — edit the two vcpkg cache steps (the non-Windows and Windows branches) to use "actions/cache@v4" instead of v3 to ensure consistency across the workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/main.workflow.yml:
- Around line 172-176: The workflow currently uses the unpinned GitHub Action
identifier ilammy/msvc-dev-cmd@v1 in three jobs (the main build job, the
integration job, and the Python workflow build job); replace each occurrence of
uses: ilammy/msvc-dev-cmd@v1 with a pinned commit SHA (uses:
ilammy/msvc-dev-cmd@<commit-sha>) so the action is fixed to a specific commit,
update all three locations consistently, and ensure the chosen commit SHA is the
same across the three occurrences.
---
Outside diff comments:
In @.github/workflows/python-main.yml:
- Around line 142-152: Update the workflow cache step that currently references
actions/cache@v3 to actions/cache@v4 so it matches the other cache usages (the
steps using "uses: actions/cache@v3" in the vcpkg cache blocks) — edit the two
vcpkg cache steps (the non-Windows and Windows branches) to use
"actions/cache@v4" instead of v3 to ensure consistency across the workflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 12569ae4-895a-47e2-848b-4f5645914c9c
📒 Files selected for processing (2)
.github/workflows/main.workflow.yml.github/workflows/python-main.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: integration (windows-2025-vs2026, Release, true, x64)
- GitHub Check: integration (windows-2025-vs2026, Debug, true, x64)
- GitHub Check: build (ubuntu-latest, 4.0.x)
- GitHub Check: build (windows-2025-vs2026, 4.0.x)
- GitHub Check: build (windows-2025-vs2026, 3.22.x)
- GitHub Check: build (ubuntu-latest, 3.22.x)
- GitHub Check: build (macos-latest, 4.0.x)
- GitHub Check: tidy
🧰 Additional context used
🪛 actionlint (1.7.12)
.github/workflows/python-main.yml
[error] 143-143: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 zizmor (1.25.2)
.github/workflows/main.workflow.yml
[warning] 180-180: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 180-180: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 174-174: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[warning] 238-238: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 238-238: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 238-238: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 238-238: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 245-245: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 228-228: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 233-233: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[warning] 270-270: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 270-270: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
.github/workflows/python-main.yml
[error] 143-143: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 143-143: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
[error] 243-243: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[warning] 264-264: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🔇 Additional comments (6)
.github/workflows/main.workflow.yml (4)
238-238: Also applies to: 270-270
241-242: LGTM!Also applies to: 248-252
180-180: Ninja doesn’t need an extra install onwindows-2025-vs2026.The
win25-vs2026runner image includes Ninja (e.g., Ninja 1.13.x), so the Windows-only-G NinjaCMake generator selection is appropriate.
142-142: Confirmwindows-2025-vs2026runner availability (GA)
windows-2025-vs2026is generally available (GA) as of May/June 2026 and is supported in workflows viaruns-on: windows-2025-vs2026, so the.github/workflows/main.workflow.ymlmatrix entry at line 142 is appropriate. (See runner image docs: https://github.com/actions/runner-images/blob/main/images/windows/Windows2025-VS2026-Readme.md).github/workflows/python-main.yml (2)
241-241: LGTM!
27-27: Update:10.0.26100.0is included onwindows-2025-vs2026
Thewindows-2025-vs2026runner image documentation lists Windows SDK 10.0.26100.0 (Windows 11 SDK 26100) as included, so the runner availability concern is covered. [1][2]
| - name: Configure MSVC environment | ||
| if: runner.os == 'Windows' | ||
| uses: ilammy/msvc-dev-cmd@v1 | ||
| with: | ||
| arch: x64 |
There was a problem hiding this comment.
Pin ilammy/msvc-dev-cmd action across all jobs.
The ilammy/msvc-dev-cmd@v1 action is unpinned in three locations (main workflow build job, integration job, and Python workflow build job). All three should be pinned to a specific commit SHA to prevent supply chain attacks and ensure reproducible builds.
🔒 Recommended fix for all occurrences
Check the latest release and apply consistently across all three locations:
- uses: ilammy/msvc-dev-cmd@v1
+ uses: ilammy/msvc-dev-cmd@v1.13.0 # or pin to commit SHA: `@0b201ec93dde9f1f0e49b7b84a51e4dbb7e43c5f`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Configure MSVC environment | |
| if: runner.os == 'Windows' | |
| uses: ilammy/msvc-dev-cmd@v1 | |
| with: | |
| arch: x64 | |
| - name: Configure MSVC environment | |
| if: runner.os == 'Windows' | |
| uses: ilammy/msvc-dev-cmd@v1.13.0 | |
| with: | |
| arch: x64 |
🧰 Tools
🪛 zizmor (1.25.2)
[error] 174-174: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/main.workflow.yml around lines 172 - 176, The workflow
currently uses the unpinned GitHub Action identifier ilammy/msvc-dev-cmd@v1 in
three jobs (the main build job, the integration job, and the Python workflow
build job); replace each occurrence of uses: ilammy/msvc-dev-cmd@v1 with a
pinned commit SHA (uses: ilammy/msvc-dev-cmd@<commit-sha>) so the action is
fixed to a specific commit, update all three locations consistently, and ensure
the chosen commit SHA is the same across the three occurrences.
moratom
left a comment
moratom
left a comment
There was a problem hiding this comment.
Thanks, left a few comments!
| @@ -207,7 +208,7 @@ def build_extension(self, ext): | |||
|
|
|||
| # Add additional cmake build args from environment | |||
| if 'CMAKE_BUILD_ARGS' in os.environ: | |||
| build_args += [os.environ['CMAKE_BUILD_ARGS']] | |||
| build_args += shlex.split(os.environ['CMAKE_BUILD_ARGS']) | |||
|
|
|||
| # Windows | |||
| if platform.system() == "Windows": | |||
| @@ -255,7 +256,7 @@ def build_extension(self, ext): | |||
|
|
|||
| # Add additional cmake args from environment | |||
| if 'CMAKE_ARGS' in os.environ: | |||
| cmake_args += [os.environ['CMAKE_ARGS']] | |||
| cmake_args += shlex.split(os.environ['CMAKE_ARGS']) | |||
|
|
|||
There was a problem hiding this comment.
Any idea why this is needed?
There was a problem hiding this comment.
This was due to issue here:
https://github.com/luxonis/depthai-core/actions/runs/27309139166/job/80674798382
| @@ -257,7 +267,7 @@ | |||
|
|
|||
| - name: Build and test find_package (installed) | |||
| run: | | |||
| cmake -S tests/integration/ -B tests/integration/build_find_package -D TEST_FIND_PACKAGE=ON -D CMAKE_PREFIX_PATH=$GITHUB_WORKSPACE/depthai_install/ ${{ env.CMAKE_ARGS }} | |||
| cmake -S tests/integration/ -B tests/integration/build_find_package ${{ runner.os == 'Windows' && '-G Ninja' || '' }} -D TEST_FIND_PACKAGE=ON -D CMAKE_PREFIX_PATH=$GITHUB_WORKSPACE/depthai_install/ ${{ env.CMAKE_ARGS }} | |||
| cmake --build tests/integration/build_find_package --config ${{ matrix.build-type }} --parallel 4 | |||
| cd tests/integration/build_find_package | |||
| ctest -C ${{ matrix.build-type }} --output-on-failure --no-tests=error | |||
There was a problem hiding this comment.
I think we used Visual Studio before, any reason we went with Ninja here?
There was a problem hiding this comment.
As far as I have tested, there was one failture if this was not Ninja, can find the GH action run.
|
Currently branch cannot be merged due to: |
|
@moratom can we assign someone who previously resolved too many symbols issue to take a look at what could be done? |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/main.workflow.yml:
- Line 270: The CMake commands at lines 238, 242, and 270 in
.github/workflows/main.workflow.yml use $GITHUB_WORKSPACE which will not expand
in PowerShell on Windows runners. Replace all three instances of
$GITHUB_WORKSPACE with ${{ github.workspace }}, which is the shell-agnostic
GitHub Actions context syntax that will work correctly across all runners
regardless of the default shell.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d56f1e99-7071-4dcc-8948-f05f93332479
📒 Files selected for processing (2)
.github/workflows/main.workflow.yml.github/workflows/python-main.yml
📜 Review details
🧰 Additional context used
🪛 zizmor (1.25.2)
.github/workflows/main.workflow.yml
[warning] 270-270: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
.github/workflows/python-main.yml
[warning] 260-260: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🔇 Additional comments (2)
.github/workflows/main.workflow.yml (1)
142-143: LGTM!Also applies to: 189-190
.github/workflows/python-main.yml (1)
130-131: LGTM!Also applies to: 220-223, 241-242, 260-260, 668-668
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/main.workflow.yml:
- Line 270: The CMake commands at lines 238, 242, and 270 in
.github/workflows/main.workflow.yml use $GITHUB_WORKSPACE which will not expand
in PowerShell on Windows runners. Replace all three instances of
$GITHUB_WORKSPACE with ${{ github.workspace }}, which is the shell-agnostic
GitHub Actions context syntax that will work correctly across all runners
regardless of the default shell.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d56f1e99-7071-4dcc-8948-f05f93332479
📒 Files selected for processing (2)
.github/workflows/main.workflow.yml.github/workflows/python-main.yml
📜 Review details
🔇 Additional comments (2)
.github/workflows/main.workflow.yml (1)
142-143: LGTM!Also applies to: 189-190
.github/workflows/python-main.yml (1)
130-131: LGTM!Also applies to: 220-223, 241-242, 260-260, 668-668
🛑 Comments failed to post (1)
.github/workflows/main.workflow.yml (1)
270-270:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify Windows run steps that use $GITHUB_WORKSPACE without explicit bash shell. rg -n -C2 '\$GITHUB_WORKSPACE' .github/workflows/main.workflow.yml rg -n -C2 '^\s*shell:\s*bash' .github/workflows/main.workflow.ymlRepository: luxonis/depthai-core
Length of output: 1529
🏁 Script executed:
# Read the workflow file to understand shell specifications and context head -n 280 .github/workflows/main.workflow.yml | tail -n 100Repository: luxonis/depthai-core
Length of output: 4391
🏁 Script executed:
# Check for any shell specifications in the workflow rg -n 'shell:' .github/workflows/main.workflow.yml | head -20Repository: luxonis/depthai-core
Length of output: 46
🏁 Script executed:
# Check the job context around line 238 and 270 sed -n '220,250p' .github/workflows/main.workflow.yml sed -n '250,280p' .github/workflows/main.workflow.ymlRepository: luxonis/depthai-core
Length of output: 3296
Use shell-agnostic workspace expansion in CMake arguments running on Windows.
Lines 238, 242, and 270 use
$GITHUB_WORKSPACEwithout accounting for the default PowerShell shell on Windows runners. In PowerShell, environment variables require$env:prefix—bare$VARIABLE_NAMEwon't expand. This will cause CMake and path configuration to fail onwindows-2025runner. Use${{ github.workspace }}(GitHub Actions context syntax, shell-agnostic) or$env:GITHUB_WORKSPACE(PowerShell-specific) consistently across all three locations.Affected lines and fix
- - name: Configure ${{ matrix.build-type }}, shared ${{ matrix.shared }}, ${{ matrix.platform }} - run: cmake -S . -B build ${{ runner.os == 'Windows' && '-G Ninja' || '' }} -D BUILD_SHARED_LIBS=${{ matrix.shared}} -D CMAKE_BUILD_TYPE=${{ matrix.build-type }} -D CMAKE_INSTALL_PREFIX=$GITHUB_WORKSPACE/depthai_install ${{ env.CMAKE_ARGS }} + - name: Configure ${{ matrix.build-type }}, shared ${{ matrix.shared }}, ${{ matrix.platform }} + run: cmake -S . -B build ${{ runner.os == 'Windows' && '-G Ninja' || '' }} -D BUILD_SHARED_LIBS=${{ matrix.shared}} -D CMAKE_BUILD_TYPE=${{ matrix.build-type }} -D CMAKE_INSTALL_PREFIX=${{ github.workspace }}/depthai_install ${{ env.CMAKE_ARGS }} - - name: Set path to shared library dll (Windows) - if: runner.os == 'Windows' - run: echo "$GITHUB_WORKSPACE/depthai_install/bin/" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append + - name: Set path to shared library dll (Windows) + if: runner.os == 'Windows' + run: echo "${{ github.workspace }}/depthai_install/bin/" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append - - name: Build and test find_package (installed) - run: | - cmake -S tests/integration/ -B tests/integration/build_find_package -D TEST_FIND_PACKAGE=ON -D CMAKE_PREFIX_PATH=$GITHUB_WORKSPACE/depthai_install/ ${{ env.CMAKE_ARGS }} + - name: Build and test find_package (installed) + run: | + cmake -S tests/integration/ -B tests/integration/build_find_package -D TEST_FIND_PACKAGE=ON -D CMAKE_PREFIX_PATH=${{ github.workspace }}/depthai_install/ ${{ env.CMAKE_ARGS }}🧰 Tools
🪛 zizmor (1.25.2)
[warning] 270-270: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/main.workflow.yml at line 270, The CMake commands at lines 238, 242, and 270 in .github/workflows/main.workflow.yml use $GITHUB_WORKSPACE which will not expand in PowerShell on Windows runners. Replace all three instances of $GITHUB_WORKSPACE with ${{ github.workspace }}, which is the shell-agnostic GitHub Actions context syntax that will work correctly across all runners regardless of the default shell.
2 participants
Purpose
GH are updating to latest runners Windows 2025-2026 and so we must update our CI / CD as well.
THere were some required changes to make this happen.
Passing DepthAI CI / CD (pyton one not tested yet).
https://github.com/luxonis/depthai-core/actions/runs/27312021250/job/80684235156
Summary by CodeRabbit