Skip to content

Windows wheels: init submodules in pre-build script with OpenSSL#18262

Open
manuelcandales wants to merge 2 commits intomainfrom
manuel/build-windows-wheels-fix-2
Open

Windows wheels: init submodules in pre-build script with OpenSSL#18262
manuelcandales wants to merge 2 commits intomainfrom
manuel/build-windows-wheels-fix-2

Conversation

@manuelcandales
Copy link
Contributor

The previous fix (setting sslBackend in pre_build_script.sh) only applied to nested tokenizer submodules. The top-level submodule checkout still used schannel via the reusable workflow's submodules: true, causing SEC_E_ILLEGAL_MESSAGE errors when cloning from git.gitlab.arm.com.

Move all submodule initialization into the pre-build script where we can control the SSL backend, and disable submodule checkout in the workflow.

The previous fix (setting sslBackend in pre_build_script.sh) only
applied to nested tokenizer submodules. The top-level submodule
checkout still used schannel via the reusable workflow's
`submodules: true`, causing SEC_E_ILLEGAL_MESSAGE errors when
cloning from git.gitlab.arm.com.

Move all submodule initialization into the pre-build script where
we can control the SSL backend, and disable submodule checkout in
the workflow.
Copilot AI review requested due to automatic review settings March 18, 2026 01:29
@pytorch-bot
Copy link

pytorch-bot bot commented Mar 18, 2026

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18262

Note: Links to docs will display an error until the docs builds have been completed.

❌ 6 New Failures, 4 Cancelled Jobs, 4 Unrelated Failures

As of commit 77989a2 with merge base 94e9ca6 (image):

NEW FAILURES - The following jobs have failed:

CANCELLED JOBS - The following jobs were cancelled. Please retry:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

BROKEN TRUNK - The following jobs failed but was present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 18, 2026
@manuelcandales manuelcandales added ciflow/binaries ciflow/binaries/all Release PRs with this label will build wheels for all python versions labels Mar 18, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Windows wheel build workflow to avoid Git TLS failures on Windows (schannel SEC_E_ILLEGAL_MESSAGE) when initializing submodules hosted outside GitHub, by moving submodule initialization into the pre-build script where the SSL backend can be controlled.

Changes:

  • Disable submodule checkout in the Windows wheels reusable workflow invocation.
  • Initialize top-level submodules in pre_build_script.sh, using http.sslBackend=openssl on Windows.
  • Keep the existing tokenizers nested-submodule initialization workaround (path-length related), now after top-level init.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/build-wheels-windows.yml Disables checkout-time submodules and documents that submodules are initialized in the pre-build script.
.ci/scripts/wheel/pre_build_script.sh Adds top-level submodule init with OpenSSL on Windows; retains nested tokenizers submodule init.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@manuelcandales manuelcandales added the release notes: none Do not include this in the release notes label Mar 18, 2026
Move submodule initialization above the aarch64 sed workaround so
the file it edits is guaranteed to exist even if the caller disables
submodule checkout. Also remove the redundant UNAME_S assignment
later in the script.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/binaries/all Release PRs with this label will build wheels for all python versions ciflow/binaries CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: none Do not include this in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants