Skip to content

GH-44421: [Python] Add configuration for building & testing free-threaded wheels on Windows#44804

Merged
kou merged 3 commits intoapache:mainfrom
lysnikolaou:windows-free-threaded-wheels
Jan 8, 2025
Merged

GH-44421: [Python] Add configuration for building & testing free-threaded wheels on Windows#44804
kou merged 3 commits intoapache:mainfrom
lysnikolaou:windows-free-threaded-wheels

Conversation

@lysnikolaou
Copy link
Contributor

@lysnikolaou lysnikolaou commented Nov 21, 2024

Rationale for this change

There's no blockers anymore for building Windows wheels for the free-threaded build, so this PR adds the necessary configuration to do that.

What changes are included in this PR?

  • Add jobs to build free-threaded wheels on Windows
  • Move VCPKG-related stuff to a reusable base job

Are these changes tested?

No tests required.

Are there any user-facing changes?

No.

@github-actions
Copy link

⚠️ GitHub issue #44421 has been automatically assigned in GitHub to PR creator.

@github-actions github-actions bot added the awaiting review Awaiting review label Nov 21, 2024
@lysnikolaou
Copy link
Contributor Author

If someone could trigger the new jobs, that'd be really helpful!

@raulcd
Copy link
Member

raulcd commented Nov 27, 2024

@github-actions crossbow submit -g wheel-windows-*

@github-actions
Copy link

Invalid group(s) {'wheel-windows-*'}. Must be one of {'wheel', 'cuda', 'example-cpp', 'linux', 'homebrew', 'fuzz', 'integration', 'cpp', 'example-python', 'nightly-release', 'verify-rc-source-linux', 'c-glib', 'example', 'python', 'nightly', 'conan', 'verify-rc-wheels', 'packaging', 'r', 'verify-rc-source', 'verify-rc', 'nightly-packaging', 'nightly-tests', 'vcpkg', 'java', 'linux-arm64', 'verify-rc-binaries', 'conda', 'test', 'verify-rc-source-macos', 'ruby', 'verify-rc-jars', 'linux-amd64'}
The Archery job run can be found at: https://github.com/apache/arrow/actions/runs/12047528593

@raulcd
Copy link
Member

raulcd commented Nov 27, 2024

@github-actions crossbow submit wheel-windows-*

@github-actions
Copy link

Revision: 78d8f9e1f3649a9769046917c54f8bf6df379f6f

Submitted crossbow builds: ursacomputing/crossbow @ actions-86ba33e477

Task Status
wheel-windows-cp310-cp310-amd64 GitHub Actions
wheel-windows-cp311-cp311-amd64 GitHub Actions
wheel-windows-cp312-cp312-amd64 GitHub Actions
wheel-windows-cp313-cp313-amd64 GitHub Actions
wheel-windows-cp313-cp313t-amd64 GitHub Actions
wheel-windows-cp39-cp39-amd64 GitHub Actions

Copy link
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I triggered the CI jobs

@github-actions github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting review Awaiting review awaiting changes Awaiting changes labels Nov 27, 2024
@lysnikolaou
Copy link
Contributor Author

Unfortunately, while the free-threaded wheel is built correctly, py -3.13t -c "import pyarrow" fails. No error shown. Does anyone know how I could inspect stderr to see why this fails?

@lysnikolaou
Copy link
Contributor Author

Also, the failure on 3.9 is that py cannot be found when running py -%PYTHON% setup.py bdist_wheel || exit /B 1, which seems odd.

@raulcd
Copy link
Member

raulcd commented Dec 2, 2024

Unfortunately, while the free-threaded wheel is built correctly, py -3.13t -c "import pyarrow" fails. No error shown. Does anyone know how I could inspect stderr to see why this fails?

I haven't tested and I am unsure if this works but maybe we can try this (https://stackoverflow.com/a/21912169):
py -3.13t -c "import pyarrow" || echo ERROR && exit /B 1 instead of py -3.13t -c "import pyarrow" || exit /B 1

@lysnikolaou
Copy link
Contributor Author

The Windows failure is probably related to pypa/setuptools#4662 and should be fixed in the next setuptools release.

@pitrou
Copy link
Member

pitrou commented Dec 4, 2024

The Windows failure is probably related to pypa/setuptools#4662 and should be fixed in the next setuptools release.

Hmm, that one should be fixed in CPython AFAIU: python/cpython#111650

@pitrou
Copy link
Member

pitrou commented Dec 4, 2024

Ah, scratch that, the rationale is actually here:
https://github.com/pypa/distutils/pull/310/files

@pitrou
Copy link
Member

pitrou commented Dec 4, 2024

Unfortunately, while the free-threaded wheel is built correctly, py -3.13t -c "import pyarrow" fails. No error shown. Does anyone know how I could inspect stderr to see why this fails?

Well, stderr should already be shown by default, so it's weird that nothing gets displayed.

@pitrou pitrou force-pushed the windows-free-threaded-wheels branch from 95a0a53 to 83a76e3 Compare December 18, 2024 17:31
@pitrou
Copy link
Member

pitrou commented Dec 18, 2024

@github-actions crossbow submit wheel-windows-*

@pitrou
Copy link
Member

pitrou commented Dec 18, 2024

Given setuptools does not seem in a hurry to merge the distutils fix, I've tried to push a manual workaround in this PR.

@github-actions
Copy link

Revision: 83a76e31e1272824d84ca97e4e513f12d7c73f03

Submitted crossbow builds: ursacomputing/crossbow @ actions-5bfc22ac30

Task Status
wheel-windows-cp310-cp310-amd64 GitHub Actions
wheel-windows-cp311-cp311-amd64 GitHub Actions
wheel-windows-cp312-cp312-amd64 GitHub Actions
wheel-windows-cp313-cp313-amd64 GitHub Actions
wheel-windows-cp313-cp313t-amd64 GitHub Actions
wheel-windows-cp39-cp39-amd64 GitHub Actions

@pitrou
Copy link
Member

pitrou commented Dec 18, 2024

Well, building the free-threaded wheel failed with:

LINK : fatal error LNK1104: cannot open file 'python313t.lib' [C:\arrow\python\build\temp.win-amd64-cpython-313t\arrow_python.vcxproj]

Edit: I've checked: that file exists in a Python 3.13 Windows install.

@pitrou
Copy link
Member

pitrou commented Dec 18, 2024

@github-actions crossbow submit wheel-windows-cp313*

@pitrou
Copy link
Member

pitrou commented Jan 6, 2025

@github-actions crossbow submit wheel-windows*

@github-actions
Copy link

github-actions bot commented Jan 6, 2025

Revision: b3ce08c75c66571a7eb84f627b4e585a2d0ade6f

Submitted crossbow builds: ursacomputing/crossbow @ actions-ec43532048

Task Status
wheel-windows-cp310-cp310-amd64 GitHub Actions
wheel-windows-cp311-cp311-amd64 GitHub Actions
wheel-windows-cp312-cp312-amd64 GitHub Actions
wheel-windows-cp313-cp313-amd64 GitHub Actions
wheel-windows-cp313-cp313t-amd64 GitHub Actions
wheel-windows-cp39-cp39-amd64 GitHub Actions

Copy link
Member

Choose a reason for hiding this comment

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

Can we use python-wheel-windows-test-vs2019-base.dockerfile as the base image?

Copy link
Member

Choose a reason for hiding this comment

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

Why would we? That would make things more complicated.

Copy link
Member

Choose a reason for hiding this comment

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

Because I felt that python-wheel-windows-{,test-}vs2019-base.dockerfile have many duplication. But I re-read them now, there are not so much duplication. Sorry.

@pitrou pitrou force-pushed the windows-free-threaded-wheels branch from 4c9872d to 097c9c6 Compare January 7, 2025 09:29
@pitrou
Copy link
Member

pitrou commented Jan 7, 2025

@github-actions crossbow submit wheel-windows*

@github-actions
Copy link

github-actions bot commented Jan 7, 2025

Revision: 764437b2d222f6b7f769629b387cbbddb27673e3

Submitted crossbow builds: ursacomputing/crossbow @ actions-f31cf1d015

Task Status
wheel-windows-cp310-cp310-amd64 GitHub Actions
wheel-windows-cp311-cp311-amd64 GitHub Actions
wheel-windows-cp312-cp312-amd64 GitHub Actions
wheel-windows-cp313-cp313-amd64 GitHub Actions
wheel-windows-cp313-cp313t-amd64 GitHub Actions
wheel-windows-cp39-cp39-amd64 GitHub Actions

@pitrou
Copy link
Member

pitrou commented Jan 7, 2025

Updated CI still green, this is ready for review again @kou

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting merge Awaiting merge labels Jan 7, 2025
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

.env Outdated
Comment on lines 98 to 99
Copy link
Member

Choose a reason for hiding this comment

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

Can we avoid the a suffix?

Suggested change
PYTHON_WHEEL_WINDOWS_IMAGE_REVISION=2025-01-06
PYTHON_WHEEL_WINDOWS_TEST_IMAGE_REVISION=2025-01-06a
PYTHON_WHEEL_WINDOWS_IMAGE_REVISION=2025-01-08
PYTHON_WHEEL_WINDOWS_TEST_IMAGE_REVISION=2025-01-08

Copy link
Member

Choose a reason for hiding this comment

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

Does this match python-wheel-windows-vs2019-base.dockerfile?

Copy link
Member

@pitrou pitrou Jan 7, 2025

Choose a reason for hiding this comment

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

Probably not, but this is asking for more and more things that are outside of the scope of this PR...

Also, the diagnostics emitted by hadolint don't seem terribly useful, especially when we have to silence most of them.

Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment that explains what is DL3006?
A link to https://github.com/hadolint/hadolint/wiki/DL3006 and the summary in the page ("Always tag the version of an image explicitly.") will be enough.

Copy link
Member

Choose a reason for hiding this comment

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

Uh, sure. But is hadolint actually useful? It seems terribly limited.

Copy link
Member

Choose a reason for hiding this comment

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

Is there any useful hadolint alternative? I'm OK with migrating to it from hadolint.

Copy link
Member

Choose a reason for hiding this comment

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

I have no idea. I can add a comment in the meantime.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I've added comments. But this is polluting the source code with pointless comments and making it less readable.

Comment on lines 30 to 35
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I missed it.

@github-actions github-actions bot added awaiting merge Awaiting merge awaiting changes Awaiting changes and removed awaiting changes Awaiting changes awaiting merge Awaiting merge labels Jan 7, 2025
@pitrou pitrou force-pushed the windows-free-threaded-wheels branch from 764437b to f212df9 Compare January 8, 2025 07:42
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jan 8, 2025
@pitrou
Copy link
Member

pitrou commented Jan 8, 2025

@github-actions crossbow submit wheel-windows*

@github-actions
Copy link

github-actions bot commented Jan 8, 2025

Revision: f212df9

Submitted crossbow builds: ursacomputing/crossbow @ actions-ad58f7ee92

Task Status
wheel-windows-cp310-cp310-amd64 GitHub Actions
wheel-windows-cp311-cp311-amd64 GitHub Actions
wheel-windows-cp312-cp312-amd64 GitHub Actions
wheel-windows-cp313-cp313-amd64 GitHub Actions
wheel-windows-cp313-cp313t-amd64 GitHub Actions
wheel-windows-cp39-cp39-amd64 GitHub Actions

@pitrou
Copy link
Member

pitrou commented Jan 8, 2025

Ok, CI is green again. Do the comments look good to you @kou ?

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

@kou kou merged commit 4ede48c into apache:main Jan 8, 2025
36 of 37 checks passed
@kou kou removed the awaiting change review Awaiting change review label Jan 8, 2025
@github-actions github-actions bot added the awaiting merge Awaiting merge label Jan 8, 2025
@lysnikolaou lysnikolaou deleted the windows-free-threaded-wheels branch January 8, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting merge Awaiting merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants