FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

CI Remove docker setup on windows in wheel workflow by virchan · Pull Request #34777 · scikit-learn/scikit-learn · GitHub

/ scikit-learn Public

CI Remove docker setup on windows in wheel workflow - #34777

Merged
lesteve merged 2 commits into
scikit-learn:mainfrom
virchan:issues/34723-remove-docker-set-up
Aug 20, 2026
ConversationCommits (2)ChecksFiles changed
Merged

CI Remove docker setup on windows in wheel workflow#34777
lesteve merged 2 commits into
scikit-learn:mainfrom
virchan:issues/34723-remove-docker-set-up

Conversation

virchan commented Aug 19, 2026 •
edited by lesteve
Loading

Copy link
Copy Markdown
Member

Reference Issues/PRs

Towards #34723.

Close #34569 (Windows Docker GHA intermittent error).

What does this implement/fix? Explain your changes.

This PR removes a Docker setup, duplicated in scikit-learn-release, where the wheel builder used to spin up a minimal Windows Docker image (build_minimal_windows_image.sh + test_windows_wheels.sh) to check that the built wheel doesn't implicitly depend on DLLs that happen to be present on the GHA runner.

Specifically, it drops the following Windows-specific overrides in wheels.yml:

  • CIBW_BEFORE_TEST_WINDOWS
  • CIBW_TEST_REQUIRES_WINDOWS
  • CIBW_TEST_COMMAND_WINDOWS

and removes the now-unused scripts:

  • build_minimal_windows_image.sh
  • test_windows_wheels.sh

AI usage disclosure

I used AI assistance for:

  • Research and understanding

Any other comments?

Should resolve the intermittent Docker failures reported in #34569.

virchan added the OS:Windows Problem specific to Windows label Aug 19, 2026

betatim commented Aug 19, 2026

Copy link
Copy Markdown
Member

How do we check what the wheel build was checking (no dependency on DLLs that just happened to exist) if we don't use docker? Do we still need this check?

lesteve commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

How do we check what the wheel build was checking (no dependency on DLLs that just happened to exist) if we don't use docker? Do we still need this check?

We still have the check in scikit-learn-release (which is done is a less hacky way as well if you ask me but I am biased) see https://github.com/scikit-learn/scikit-learn-release/blob/7a27d9e4d6b2a83735333679b6e8f165464e4395/.github/workflows/wheels.yml#L185-L191

IIRC, we are one of the only project to do this Docker minimal image testing though. I am guessing other projects rely on standard tooling like delvewheel on Windows (the equivalent of auditwheel for Linux), instead of having some custom DLL vendoring code like we do. Anyway this kind of things is longer-term 😉.

lesteve commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

BTW I have seen a few hanging CI logs like this one https://github.com/scikit-learn/scikit-learn/actions/runs/32233324547/job/96008091616?pr=34777.

We should limit the build to something reasonable like 30 minutes or maybe one hour to have some margin. PR more than welcome 😉. It seems like the default is 6 hours ...

Not sure about the reason this one hangs during the playwright install for some reason:

+ python -m playwright install --with-deps
Installing dependencies...
Switching to root user to install dependencies...
Get:1 file:/etc/apt/apt-mirrors.txt Mirrorlist [144 B]
Get:6 https://packages.microsoft.com/repos/azure-cli jammy InRelease [3596 B]
Get:7 https://packages.microsoft.com/ubuntu/22.04/prod jammy InRelease [3632 B]
Get:8 https://dl.google.com/linux/chrome-stable/deb stable InRelease [2548 B]
Get:9 https://packages.microsoft.com/repos/azure-cli jammy/main amd64 Packages [3562 B]
Get:10 https://packages.microsoft.com/repos/azure-cli jammy/main all Packages [1099 B]
Get:11 https://packages.microsoft.com/ubuntu/22.04/prod jammy/main armhf Packages [23.0 kB]
Get:12 https://packages.microsoft.com/ubuntu/22.04/prod jammy/main amd64 Packages [630 kB]
Get:13 https://packages.microsoft.com/ubuntu/22.04/prod jammy/main arm64 Packages [404 kB]
Get:14 https://dl.google.com/linux/chrome-stable/deb stable/main amd64 Packages [1417 B]
Ign:2 http://azure.archive.ubuntu.com/ubuntu jammy InRelease
Ign:3 http://azure.archive.ubuntu.com/ubuntu jammy-updates InRelease
Ign:4 http://azure.archive.ubuntu.com/ubuntu jammy-backports InRelease
Ign:5 http://azure.archive.ubuntu.com/ubuntu jammy-security InRelease
Ign:2 http://azure.archive.ubuntu.com/ubuntu jammy InRelease
Ign:3 http://azure.archive.ubuntu.com/ubuntu jammy-updates InRelease
Ign:4 http://azure.archive.ubuntu.com/ubuntu jammy-backports InRelease
Ign:5 http://azure.archive.ubuntu.com/ubuntu jammy-security InRelease
Ign:2 http://azure.archive.ubuntu.com/ubuntu jammy InRelease
Ign:3 http://azure.archive.ubuntu.com/ubuntu jammy-updates InRelease
Ign:4 http://azure.archive.ubuntu.com/ubuntu jammy-backports InRelease
Ign:5 http://azure.archive.ubuntu.com/ubuntu jammy-security InRelease
Ign:2 http://azure.archive.ubuntu.com/ubuntu jammy InRelease
Ign:3 http://azure.archive.ubuntu.com/ubuntu jammy-updates InRelease
Ign:4 http://azure.archive.ubuntu.com/ubuntu jammy-backports InRelease
Ign:5 http://azure.archive.ubuntu.com/ubuntu jammy-security InRelease
Hit:2 https://archive.ubuntu.com/ubuntu jammy InRelease
Get:3 https://archive.ubuntu.com/ubuntu jammy-updates InRelease [128 kB]
Ign:15 http://azure.archive.ubuntu.com/ubuntu jammy-updates/main amd64 Packages
Ign:16 http://azure.archive.ubuntu.com/ubuntu jammy-updates/main Translation-en
Ign:17 http://azure.archive.ubuntu.com/ubuntu jammy-updates/restricted amd64 Packages
Ign:18 http://azure.archive.ubuntu.com/ubuntu jammy-updates/restricted Translation-en
Ign:19 http://azure.archive.ubuntu.com/ubuntu jammy-updates/universe amd64 Packages
Ign:20 http://azure.archive.ubuntu.com/ubuntu jammy-updates/universe Translation-en
Get:4 https://archive.ubuntu.com/ubuntu jammy-backports InRelease [127 kB]
Get:5 https://archive.ubuntu.com/ubuntu jammy-security InRelease [129 kB]

virchan commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

Thanks, @lesteve, for re-running the CI!

How do we check what the wheel build was checking (no dependency on DLLs that just happened to exist) if we don't use docker? Do we still need this check?

My understanding is: yes, we still need this check, and it already exists in scikit-learn-release's wheels.yml.

Removing it here means we stop duplicating that same check on every scikit-learn PR, rather than dropping the check itself.

virchan marked this pull request as ready for review August 19, 2026 21:51

betatim commented Aug 20, 2026

Copy link
Copy Markdown
Member

Thanks for explaining. If there is a more standard way of doing this that seems like a good thing to switch to.

lesteve commented Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

LGTM, thanks!

As a potential follow-up, here is a few differences I noticed between scikit-learn and scikit-learn-release wheels.yml quickly looking at the diff:

  • scikit-learn sets manylinux image, not sure whether that was needed at one point and what the reason is ... maybe this can just be removed?
  • using cibuildwheel.toml in scikit-learn rather than env variables in wheels.yml would be nice.

lesteve changed the title CI Remove docker setup on windows CI Remove docker setup on windows in wheel workflow Aug 20, 2026
lesteve merged commit 3341d32 into scikit-learn:main Aug 20, 2026
87 of 89 checks passed
virchan deleted the issues/34723-remove-docker-set-up branch August 22, 2026 20:14
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lesteve lesteve approved these changes

Assignees

No one assigned

Labels

Build / CI OS:Windows Problem specific to Windows

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

⚠️ CI failed on Wheel builder (last failure: Aug 14, 2026) ⚠️

3 participants

Back | FazBrowse Home | New Git URL