Skip to content

github/workflows: Stop using windows-2019 runner for Windows CI. #17468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jun 11, 2025
Merged

github/workflows: Stop using windows-2019 runner for Windows CI. #17468

merged 2 commits into from
Jun 11, 2025

Conversation

dpgeorge
Copy link
Member

Summary

This runner has been deprecated by GitHub.

Testing

CI should test this.

@dpgeorge
Copy link
Member Author

@stinos FYI. I'm not sure what the best thing to do is, but I simply replaced windows-2019 with windows-2025.

@github-actions GitHub Actions
Copy link

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

@codecov Codecov
Copy link

codecov bot commented Jun 10, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.54%. Comparing base (2ce63b1) to head (9bde125).
Report is 2 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #17468   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files         169      169           
  Lines       21941    21941           
=======================================
  Hits        21621    21621           
  Misses        320      320

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dlech
Copy link
Contributor

dlech commented Jun 10, 2025 •
edited
Loading

The windows-2025 builder only has VS 2022 installed. So if we drop windows-2019, we will need to install VS 2019 manually like we do for VS 2017.

I.e. duplicate this and adjust for Visual Studio 2019. (And we can just make it window-latest.)

micropython/.github/workflows/ports_windows.yml

Lines 49 to 54 in 5f4abeb

- name: Install Visual Studio 2017
if: matrix.visualstudio == '2017'
run: |
choco install visualstudio2017buildtools
choco install visualstudio2017-workload-vctools
choco install windows-sdk-8.1

dlech
dlech reviewed Jun 10, 2025
View reviewed changes
.github/workflows/ports_windows.yml Outdated
Comment on lines 58 to 61
- uses: actions/setup-python@v5
if: matrix.runner == 'windows-2019'
if: matrix.runner == 'windows-2025'
with:
python-version: '3.9'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last time I checked, windows-2025 still uses python 3.9 by default, so we can just drop this whole section if we are dropping use of the windows-2019 runner. This was only needed because windows-2019 used an older python version by default.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've now removed this setup-python action.

@dpgeorge dpgeorge force-pushed the github-workflows-ports-windows-stop-using-2019 branch from 53912b1 to 3007f0c Compare June 10, 2025 04:27
@dpgeorge
Copy link
Member Author

The windows-2025 builder only has VS 2022 installed. So if we drop windows-2019, we will need to install VS 2019 manually like we do for VS 2017.

Thanks for the hint, that's now done.

But now we get errors again about NAN not being a constant... any ideas how to extend the existing defined(_MSC_VER) && _MSC_VER >= 1942 check to make the CI work again?

stinos
stinos reviewed Jun 10, 2025
View reviewed changes
.github/workflows/ports_windows.yml Outdated
@@ -31,7 +31,7 @@ jobs:
runner: windows-latest
vs_version: '[15, 16)'
- visualstudio: '2019'
runner: windows-2019
runner: windows-latest
vs_version: '[16, 17)'
- visualstudio: '2022'
runner: windows-2022
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should also be windows-latest for consistency? In the end we don't really care on what windows (or whatever GH makes from it) version this runs. The build tools are the key difference.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the intention was to test on different Windows version? But I'm happy to change them all to windows-latest.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the intention was to test on different Windows version?

@dlech was it? Or rather just a byproduct of which image contained which VS version ?

Copy link
Contributor

@dlech dlech Jun 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was just a byproduct of VS version. So, yes, we could pull that out of the matrix and just set the runner to always be windows-latest.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've made everything use windows-latest and factored that out of the matrix.

@stinos
Copy link
Contributor

stinos commented Jun 10, 2025

But now we get errors again about NAN not being a constant... any ideas how to extend the existing defined(_MSC_VER) && _MSC_VER >= 1942 check to make the CI work again?

Based on https://stackoverflow.com/questions/79195142/recent-msvc-versions-dont-treat-nan-as-constant-workaround/79324199#79324199 the original commit only worked by accident at that time, because this is not actually related to compiler version (i.e. _MSC_VER) but to the window SDK version. I think

#if defined(_MSC_VER) && !defined(_UCRT_NOISY_NAN)
#define _UCRT_NOISY_NAN 
#endif

should be good.

@dpgeorge dpgeorge force-pushed the github-workflows-ports-windows-stop-using-2019 branch from 3007f0c to 1eba656 Compare June 10, 2025 12:16
@dpgeorge
Copy link
Member Author

I've updated it to use _UCRT_NOISY_NAN and indeed it looks like that's a good solution.

@dpgeorge dpgeorge force-pushed the github-workflows-ports-windows-stop-using-2019 branch from 1eba656 to 6d9b2e0 Compare June 10, 2025 14:33
dpgeorge added 2 commits June 11, 2025 00:43
@dpgeorge
py/objfloat: Change MSVC workaround for NAN being a constant.
ca80aab 
It's actually a bug in the Windows SDK, not MSVC, as per
https://stackoverflow.com/questions/79195142/recent-msvc-versions-dont-treat-nan-as-constant-workaround/79324199#79324199

Thanks to @stinos.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
github/workflows: Use windows-latest runner for all Windows CI jobs.
9bde125 
The windows-2019 runner has been deprecated by GitHub, so stop using that.
Also take the chance to stop using windows-2022 and just use windows-latest
everywhere.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge force-pushed the github-workflows-ports-windows-stop-using-2019 branch from 6d9b2e0 to 9bde125 Compare June 10, 2025 14:43
@dpgeorge dpgeorge merged commit 9bde125 into micropython:master Jun 11, 2025
69 checks passed
@dpgeorge dpgeorge deleted the github-workflows-ports-windows-stop-using-2019 branch June 11, 2025 00:58
@dpgeorge
Copy link
Member Author

Thanks for the quick review on this @stinos and @dlech .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stinos stinos stinos left review comments

@dlech dlech dlech approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dpgeorge @dlech @stinos