Skip to content

Conversation

@jgiannuzzi
Copy link
Contributor

@jgiannuzzi jgiannuzzi commented Oct 14, 2025

Rationale for this change

This PR adds a CI job to test building the C++ library on Windows ARM64 with MSVC, which will help ensuring that downstream projects like vcpkg can build Arrow C++ on that platform.

What changes are included in this PR?

  • The windows job from the cpp.yml workflow has been moved into a reusable workflow cpp_windows.yml
  • Use GitHub hosted runner windows-11-arm to build with MSVC ARM64 in cpp_extra.yml
  • simd-level is set NONE for now as xsimd does not yet support MSVC ARM64
  • msys2 is installed and used for the timezone data, as it is not guaranteed to exist on windows-11-arm images
  • A recent version of cmake is installed as support for finding Windows OpenSSL ARM64 builds was only added in 4.1.0 (see Kitware/CMake@bf52219)
  • The Boost context implementation is set to winfib when building with MSVC ARM64 as it's the only supported implementation for that platform (see Windows ARM64 Support boostorg/context#296 (comment) and Add Windows ARM64 support boostorg/context#315)

Are these changes tested?

Yes, in the CI itself

Are there any user-facing changes?

No

@github-actions
Copy link

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

@jgiannuzzi
Copy link
Contributor Author

jgiannuzzi commented Oct 14, 2025

This PR currently depends on #47779 for the new CI job to succeed, hence why I set it as draft.

@github-actions
Copy link

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

1 similar comment
@github-actions
Copy link

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

@pitrou
Copy link
Member

pitrou commented Oct 15, 2025

Can this be made part of the "C++ Extra" workflows? We probably don't want to support this officially for now, do we?
@kou

@jgiannuzzi
Copy link
Contributor Author

@pitrou @raulcd @kou if you are all fine with the duplication between the "C++" Windows MSVC x64 job and the "C++ Extra" Windows MSVC arm64 job (as they are more or less identical), I'm happy to move it there!

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.

Yes. Could you use cpp_extra.yml?

BTW, @pitrou could you start a discussion (and a vote?) for #46002 on [email protected] to clarify our support policy?

if(MSVC AND "${CMAKE_SYSTEM_PROCESSOR}" STREQUAL "ARM64")
set(BOOST_CONTEXT_IMPLEMENTATION
winfib
CACHE STRING "" FORCE)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to use CACHE variable here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, otherwise it will default to fcontext and the build will fail. If cmake is run a second time after the initial configuration failed, somehow it then works. I thus decided to use CACHE FORCE to make it more reliable.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm. Could you try CMP0126 https://cmake.org/cmake/help/latest/policy/CMP0126.html and normal (not cache) variable?

diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
index e805694f52..ca6b9e8746 100644
--- a/cpp/CMakeLists.txt
+++ b/cpp/CMakeLists.txt
@@ -71,6 +71,12 @@ cmake_policy(SET CMP0090 NEW)
 # MSVC runtime library flags are selected by an abstraction.
 cmake_policy(SET CMP0091 NEW)
 
+# https://cmake.org/cmake/help/latest/policy/CMP0126.html
+#
+# The set(CACHE) command does not remove any normal variable of the
+# same name from the current scope.
+cmake_policy(SET CMP0126 NEW)
+
 # https://cmake.org/cmake/help/latest/policy/CMP0135.html
 #
 # CMP0135 is for solving re-building and re-downloading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sadly it didn't work :/

Copy link
Member

Choose a reason for hiding this comment

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

Oh...

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Oct 16, 2025
@jgiannuzzi jgiannuzzi force-pushed the windows-arm64-cpp-ci branch from d2a3efd to e861fdc Compare October 16, 2025 14:11
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 16, 2025
if(MSVC AND "${CMAKE_SYSTEM_PROCESSOR}" STREQUAL "ARM64")
set(BOOST_CONTEXT_IMPLEMENTATION
winfib
CACHE STRING "" FORCE)
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. Could you try CMP0126 https://cmake.org/cmake/help/latest/policy/CMP0126.html and normal (not cache) variable?

diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
index e805694f52..ca6b9e8746 100644
--- a/cpp/CMakeLists.txt
+++ b/cpp/CMakeLists.txt
@@ -71,6 +71,12 @@ cmake_policy(SET CMP0090 NEW)
 # MSVC runtime library flags are selected by an abstraction.
 cmake_policy(SET CMP0091 NEW)
 
+# https://cmake.org/cmake/help/latest/policy/CMP0126.html
+#
+# The set(CACHE) command does not remove any normal variable of the
+# same name from the current scope.
+cmake_policy(SET CMP0126 NEW)
+
 # https://cmake.org/cmake/help/latest/policy/CMP0135.html
 #
 # CMP0135 is for solving re-building and re-downloading.

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Oct 17, 2025
@jgiannuzzi jgiannuzzi force-pushed the windows-arm64-cpp-ci branch from e861fdc to 4d7e0b3 Compare October 21, 2025 17:28
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 21, 2025
@jgiannuzzi
Copy link
Contributor Author

jgiannuzzi commented Oct 21, 2025

@pitrou @AntoinePrv #47573 has broken the Windows ARM64 MSVC build. I have addressed it in 3fb046c, but happy to create a separate PR for it if you want.

@github-actions
Copy link

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

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

It may be better that we split the bpacking_simd changes to a separated PR. @WillAyd may have a comment for the Meson part.

if(MSVC AND "${CMAKE_SYSTEM_PROCESSOR}" STREQUAL "ARM64")
set(BOOST_CONTEXT_IMPLEMENTATION
winfib
CACHE STRING "" FORCE)
Copy link
Member

Choose a reason for hiding this comment

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

Oh...

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Oct 22, 2025
@pitrou
Copy link
Member

pitrou commented Oct 22, 2025

It may be better that we split the bpacking_simd changes to a separated PR

+1 for splitting indeed

@jgiannuzzi
Copy link
Contributor Author

@pitrou @WillAyd @AntoinePrv split into #47910

@jgiannuzzi jgiannuzzi force-pushed the windows-arm64-cpp-ci branch from 4d7e0b3 to fc90543 Compare October 22, 2025 14:38
pitrou pushed a commit that referenced this pull request Oct 22, 2025
### Rationale for this change

#47573 broke the Windows ARM64 MSVC build by trying to compile xsimd even when `ARROW_SIMD_LEVEL` is set to `NONE`.

### What changes are included in this PR?

Make sure that xsimd is only included when SIMD is requested.

### Are these changes tested?

Yes, in the Windows ARM64 MSVC build pipeline that I was developing on #47811 when I noticed the change.

### Are there any user-facing changes?

No.
* GitHub Issue: #47909

Authored-by: Jonathan Giannuzzi <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
@jgiannuzzi jgiannuzzi force-pushed the windows-arm64-cpp-ci branch from fc90543 to 8f8edd7 Compare October 22, 2025 15:52
- Use GitHub hosted runner `windows-11-arm` to build with MSVC ARM64
- `simd-level` is set `NONE` for now as xsimd does not yet support MSVC ARM64
- `msys2` is installed and used for the timezone data, as it is not guaranteed to exist on windows-11-arm images
- A recent version of `cmake` is installed as support for finding Windows OpenSSL ARM64 builds was only added in 4.1.0 (see Kitware/CMake@bf52219)
- The Boost context implementation is set to `winfib` when building with MSVC ARM64 as it's the only supported implementation for that platform (see boostorg/context#296 (comment) and boostorg/context#315)
- Updated ccache installation script and version to enable a Windows ARM64 native version
@jgiannuzzi jgiannuzzi force-pushed the windows-arm64-cpp-ci branch from 8f8edd7 to e859997 Compare October 22, 2025 15:53
@jgiannuzzi
Copy link
Contributor Author

Rebased after the merge of #47910 and included @kou's latest comments around naming

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 8dd9eba into apache:main Oct 23, 2025
42 of 44 checks passed
@kou kou removed the awaiting merge Awaiting merge label Oct 23, 2025
@github-actions github-actions bot added the awaiting merge Awaiting merge label Oct 23, 2025
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 8dd9eba.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 57 possible false positives for unstable benchmarks that are known to sometimes produce them.

@jgiannuzzi jgiannuzzi deleted the windows-arm64-cpp-ci branch October 23, 2025 11:32
zanmato1984 pushed a commit to zanmato1984/arrow that referenced this pull request Nov 5, 2025
### Rationale for this change

apache#47573 broke the Windows ARM64 MSVC build by trying to compile xsimd even when `ARROW_SIMD_LEVEL` is set to `NONE`.

### What changes are included in this PR?

Make sure that xsimd is only included when SIMD is requested.

### Are these changes tested?

Yes, in the Windows ARM64 MSVC build pipeline that I was developing on apache#47811 when I noticed the change.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#47909

Authored-by: Jonathan Giannuzzi <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
zanmato1984 pushed a commit to zanmato1984/arrow that referenced this pull request Nov 5, 2025
### Rationale for this change

This PR adds a CI job to test building the C++ library on Windows ARM64 with MSVC, which will help ensuring that downstream projects like [`vcpkg`](https://github.com/microsoft/vcpkg) can build Arrow C++ on that platform.

### What changes are included in this PR?

- The `windows` job from the `cpp.yml` workflow has been moved into a reusable workflow `cpp_windows.yml`
- Use GitHub hosted runner `windows-11-arm` to build with MSVC ARM64 in `cpp_extra.yml`
- `simd-level` is set `NONE` for now as xsimd does not yet support MSVC ARM64
- `msys2` is installed and used for the timezone data, as it is not guaranteed to exist on `windows-11-arm` images
- A recent version of `cmake` is installed as support for finding Windows OpenSSL ARM64 builds was only added in 4.1.0 (see Kitware/CMake@bf52219)
- The Boost context implementation is set to `winfib` when building with MSVC ARM64 as it's the only supported implementation for that platform (see boostorg/context#296 (comment) and boostorg/context#315)

### Are these changes tested?

Yes, in the CI itself

### Are there any user-facing changes?

No
* GitHub Issue: apache#47196

Authored-by: Jonathan Giannuzzi <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants