Skip to content

Update to windows-2025 runner on ci #5571

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
youknowone merged 3 commits into from
Jan 7, 2026
Merged

Update to windows-2025 runner on ci #5571

youknowone merged 3 commits into from
Jan 7, 2026
+11 −40

Conversation

@coolreader18
Copy link
Member

@coolreader18 coolreader18 commented Feb 28, 2025 •
edited by coderabbitai bot
Loading

This comes preinstalled with openssl 3, so we can cut out building openssl ourselves, which takes 5 minutes every CI run.

Summary by CodeRabbit

  • Chores
    • Updated CI/CD infrastructure to use Windows 2025 runners.
    • Updated Windows build configuration with new OpenSSL environment variables.
    • Simplified Windows-specific environment setup in CI/CD pipelines.
    • Updated Windows library loading mechanism.

✏️ Tip: You can customize this high-level summary in your review settings.

@coolreader18 coolreader18 force-pushed the ci-windows-2025 branch 2 times, most recently from f70cf66 to 1dc3df4 Compare February 28, 2025 01:48
@arihant2math
Copy link
Collaborator

arihant2math commented Feb 28, 2025

The issue here is env variables that point to the openssl installation are not set.

Setting all the variables specified in rust-openssl/rust-openssl#1542 (comment) should resolve this.

@coolreader18 coolreader18 force-pushed the ci-windows-2025 branch 8 times, most recently from 2e9aa8d to be3130c Compare March 4, 2025 23:41
@coolreader18
Copy link
Member Author

coolreader18 commented Mar 5, 2025

@youknowone you added the win32_xstat impl in #5247 - do you know why the tests fixed by that might be failing now with a windows update? It's not like they would've removed the api-ms-win-core-file-l2-1-4 API set.

@youknowone
Copy link
Member

youknowone commented Mar 6, 2025

I am sorry. I code like a monkey with wniapi. I really don't know well about their details.

@bjia56
Copy link

bjia56 commented Mar 7, 2025

I mentioned this link in #5583, but openssl prebuilts are also available from the cpython project here https://github.com/python/cpython-bin-deps/

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 4, 2026 •
edited
Loading

📝 Walkthrough

Walkthrough

These changes update Windows build and CI configuration by pinning the Windows runner to windows-2025, replacing the vcpkg-based OpenSSL setup with direct path environment variables, removing OpenSSL bootstrap steps from CI workflows, and updating a Windows API library import to include the .dll extension.

Changes

Cohort / File(s) Summary
CI/Release Workflow Configuration
.github/workflows/ci.yaml, .github/workflows/release.yml		 Updated Windows runner from windows-latest to windows-2025 in build matrices; added X86_64_PC_WINDOWS_MSVC_OPENSSL_LIB_DIR and X86_64_PC_WINDOWS_MSVC_OPENSSL_INCLUDE_DIR environment variables pointing to OpenSSL paths; removed Windows-specific OpenSSL bootstrap steps (Git longpaths config, cargo-vcpkg, vcpkg build).
Build Configuration
Cargo.toml		 Removed entire [package.metadata.vcpkg] block including Git reference (microsoft/vcpkg at rev 2025.09.17) and vcpkg target triplet configuration for x86_64-pc-windows-msvc.
Windows API Import
crates/common/src/fileutils.rs		 Updated Windows API library name from "api-ms-win-core-file-l2-1-4" to "api-ms-win-core-file-l2-1-4.dll" to explicitly include the DLL extension.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related PRs

  • RustPython/RustPython#6148: Updates Windows runner pinning from windows-latest to windows-2025 in the same CI workflow matrices.

Poem

🐰 Hoppy improvements, oh what a sight!
Windows runners pinned just right,
OpenSSL paths now set with care,
vcpkg gone—cleaner air!
DLL extensions make APIs aware,
Build configurations beyond compare! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: updating CI to use the windows-2025 runner instead of windows-latest, which is the primary focus of the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1b22f1 and 8afe918.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_shutil.py is excluded by !Lib/**
📒 Files selected for processing (4)
  • .github/workflows/ci.yaml
  • .github/workflows/release.yml
  • Cargo.toml
  • crates/common/src/fileutils.rs
💤 Files with no reviewable changes (1)
  • Cargo.toml
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

  • crates/common/src/fileutils.rs
🧠 Learnings (4)
📚 Learning: 2025-07-11T09:35:43.571Z 
Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5947
File: .github/workflows/ci.yaml:116-116
Timestamp: 2025-07-11T09:35:43.571Z
Learning: GitHub Actions timeout-minutes field supports expressions that evaluate to integers, such as `${{ contains(matrix.os, 'windows') && 40 || 30 }}`, which will conditionally set different timeout values based on the runner OS.

Applied to files:

  • .github/workflows/ci.yaml
📚 Learning: 2025-07-11T09:35:43.571Z 
Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5947
File: .github/workflows/ci.yaml:116-116
Timestamp: 2025-07-11T09:35:43.571Z
Learning: GitHub Actions supports ternary-like expressions using `&&` and `||` operators in timeout-minutes. The syntax `${{ condition && value_if_true || value_if_false }}` is valid and commonly used, such as `${{ contains(matrix.os, 'windows') && 40 || 30 }}` which evaluates to 40 for Windows runners and 30 for others.

Applied to files:

  • .github/workflows/ci.yaml
📚 Learning: 2025-12-27T14:03:49.034Z 
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to **/*.rs : Always run clippy to lint code (`cargo clippy`) before completing tasks and fix any warnings or lints introduced by changes

Applied to files:

  • .github/workflows/ci.yaml
📚 Learning: 2025-06-28T16:31:03.991Z 
Learnt from: arihant2math
Repo: RustPython/RustPython PR: 5790
File: build.rs:2-2
Timestamp: 2025-06-28T16:31:03.991Z
Learning: In Cargo build scripts (build.rs), the environment variable CARGO_CFG_TARGET_OS is guaranteed to exist and is automatically set by Cargo during the build process, making unwrap() safe to use when accessing this variable.

Applied to files:

  • .github/workflows/release.yml
⏰ 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). (11)
  • GitHub Check: Run tests under miri
  • GitHub Check: Run snippets and cpython tests (ubuntu-latest)
  • GitHub Check: Run snippets and cpython tests (macos-latest)
  • GitHub Check: Run rust tests (windows-2025)
  • GitHub Check: Run snippets and cpython tests (windows-2025)
  • GitHub Check: Check Rust code with clippy
  • GitHub Check: Run rust tests (macos-latest)
  • GitHub Check: Run snippets and cpython tests on wasm-wasi
  • GitHub Check: Run rust tests (ubuntu-latest)
  • GitHub Check: Check the WASM package and demo
  • GitHub Check: Ensure compilation on various targets
🔇 Additional comments (6)
.github/workflows/ci.yaml (3)

123-123: Runner update to windows-2025 looks good, contingent on OpenSSL configuration.

The update to windows-2025 aligns with the PR objective to use the preinstalled OpenSSL 3. However, this change depends on the correct OpenSSL environment variable configuration (Lines 110-111).

254-254: Runner update to windows-2025 is consistent with rust_tests job.

This change mirrors the runner update in the rust_tests job and is appropriate for the PR objectives.

110-111: Verify the OpenSSL paths on the windows-2025 runner to confirm they exist and work with OpenSSL 3.

The target-specific environment variables X86_64_PC_WINDOWS_MSVC_OPENSSL_LIB_DIR and X86_64_PC_WINDOWS_MSVC_OPENSSL_INCLUDE_DIR are a valid approach according to rust-openssl documentation and are preferred over generic OPENSSL_* variables for MSVC targets. However, the specific paths (C:\Program Files\OpenSSL\lib\VC\x64\MD and C:\Program Files\OpenSSL\include) must be verified to exist on the windows-2025 runner with the preinstalled OpenSSL 3, as the directory structure may differ from previous Windows runners.

If these paths are incorrect or the build fails, consider:

  • Adding generic OPENSSL_DIR as a fallback, or
  • Checking if vcpkg is available as an alternative detection method on windows-2025.
.github/workflows/release.yml (2)

20-21: Verify OpenSSL environment variables match the windows-2025 runner configuration.

These environment variables mirror those added in ci.yaml. The same verification concerns apply here - please ensure the paths are correct for windows-2025 and check if additional environment variables are required per the rust-openssl documentation mentioned in the PR comments.

Refer to the verification steps suggested for ci.yaml (Lines 110-111) to validate these paths.

47-47: Runner update to windows-2025 is consistent with CI workflow changes.

This change aligns with the updates in ci.yaml and supports the PR objective to use the preinstalled OpenSSL 3.

crates/common/src/fileutils.rs (1)

306-307: The .dll extension is required for API set loading on Windows — the code is correct.

The Windows loader requires the ".dll" suffix for API set contract names like api-ms-win-core-file-l2-1-4. While API sets are virtual and don't reference actual files on disk, the ".dll" extension is a mandatory loader convention. LoadLibraryW will automatically append ".DLL" if no extension is provided, but explicitly including it (as the code does) is the correct and recommended approach.

Likely an incorrect or invalid review comment.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@youknowone youknowone merged commit bf80d27 into RustPython:main Jan 7, 2026
12 of 13 checks passed
terryluan12 pushed a commit to terryluan12/RustPython that referenced this pull request Jan 15, 2026
@coolreader18 @terryluan12
Update to windows-2025 runner on ci (RustPython#5571)
3f98b6c 
* Update to windows-2025 on ci

* Unmark unexpected successes

* Try adding .dll
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@coolreader18 @arihant2math @youknowone @bjia56