Add MSW VS ARM64 and ARM64EC infra (PR #24615)
Jon Kunkee
This change will, when it's done, add:
- ARM64 and ARM64EC Platform entries to all VS Project files
- ARM64 Platform entries to
msw_vs15.sln,msw_vs16.sln, andmsw_vs17.sln - ARM64EC Platform entries to
msw_vs17.sln - ARM64X enablement (
BuildAsX) as appropriate - CI changes to validate and deliver these changes
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/24615
Commit Summary
- a3dd79e Add ARM64 and ARM64EC Platforms to projects and vc17 solution
- d5a06e7 Start doc update for ARM64 Windows support
File Changes
(28 files)
- M README.md (2)
- M build/msw/wx_adv.vcxproj (436)
- M build/msw/wx_aui.vcxproj (438)
- M build/msw/wx_base.vcxproj (486)
- M build/msw/wx_core.vcxproj (682)
- M build/msw/wx_custom_build.vcxproj (152)
- M build/msw/wx_gl.vcxproj (438)
- M build/msw/wx_html.vcxproj (436)
- M build/msw/wx_media.vcxproj (436)
- M build/msw/wx_net.vcxproj (436)
- M build/msw/wx_propgrid.vcxproj (438)
- M build/msw/wx_qa.vcxproj (436)
- M build/msw/wx_ribbon.vcxproj (438)
- M build/msw/wx_richtext.vcxproj (438)
- M build/msw/wx_stc.vcxproj (436)
- M build/msw/wx_vc17.sln (408)
- M build/msw/wx_webview.vcxproj (446)
- M build/msw/wx_wxexpat.vcxproj (366)
- M build/msw/wx_wxjpeg.vcxproj (364)
- M build/msw/wx_wxlexilla.vcxproj (380)
- M build/msw/wx_wxpng.vcxproj (366)
- M build/msw/wx_wxregex.vcxproj (374)
- M build/msw/wx_wxregex.vcxproj.filters (77)
- M build/msw/wx_wxscintilla.vcxproj (380)
- M build/msw/wx_wxtiff.vcxproj (364)
- M build/msw/wx_wxzlib.vcxproj (364)
- M build/msw/wx_xml.vcxproj (0)
- M build/msw/wx_xrc.vcxproj (0)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/24615.patch
- https://github.com/wxWidgets/wxWidgets/pull/24615.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Thanks, I didn't look at all changes, but those that I did look at seem pretty straightforward to me.
I'd extract the changes to wxregex.vcxproj.filters in a separate commit as they're not really ARM-related, but otherwise no real comments — please just remove the "Draft" status when you'd like this to get merged.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
I have a few more edits for the docs, then I want to re-scope the description to match, then yes, let's get it in soon. (.filters changes are a matter of preference, so I drop that and review the other files.)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
No, I the .filters change is definitely correct, we (I, probably) just forgot to update it when switching from the old regex library to PCRE2, and it needs to be done, but it would just be better to do it in a separate commit. TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee pushed 1 commit.
- e2ee6e3 Revert .filters change to be done later
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee pushed 1 commit.
- 6d464a7 Add MSW-ARM64-specific documentation notes
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
After some tweaks, this should be ready to merge.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
Hang on, part of this should be quick...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
It looks like we need to add ARM platform to the minimal sample projects. This shouldn't be difficult to do but I've been thinking about adding project files for all the other samples too (as part of migration from using makefile.vc) and updating all of those risks being painful. I wonder if there is some way to centralize the common part of all the samples, somehow, e.g. include some common fragment defining the supported platforms or something?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
I poked around with this a bit. Adding the minimal configuration is hitting an error around not being able to find wx/setup.h. As noted with ARM64X build, this bit of infra needs a little love.
- I can add the
iflogic to the minimal example and test jobs along with the run jobs. This just punts on the configuration problem. I see that Win32 is also missing from the minimal example. - I can work through adding the
minimalconfigurations now.
Note that NMake already supports ARM64, so that should be usable in the interim.
As for factoring out common parts of project/vcxproj files, it's only a little more effort. My personal preference is using the Visual Studio GUI to make a change, then looking at git diff to move the common changes into Directory.Build.props and Directory.Build.targets files (docs link). (I've seen industrial-scale work with lots of recursive property sheets/.props files--as long as things are Import'd in the correct order, you can do anything--but more diversion from the normal path is more work.)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee pushed 1 commit.
- 4bb178a Add OutDir and IntDir entries for ARM64 and ARM64EC
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee pushed 1 commit.
- 6d058cf Add ARM64 and ARM64EC cases to wxArchSuffix logic
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
I poked around with this a bit. Adding the minimal configuration is hitting an error around not being able to find
wx/setup.h. As noted with ARM64X build, this bit of infra needs a little love.
I was going to write that this needs to be addressed by just adding another arch to build/msw/wx_setup.props, but I think you've already done it, so is this still a problem?
1. I can add the `if` logic to the minimal example and test jobs along with the run jobs. This just punts on the configuration problem. I see that Win32 is also missing from the minimal example.
Sorry, I don't understand this at all, samples/minimal/minimal.vcxproj clearly defines Win32 configurations and I'm pretty sure building them works (even though I stopped using Win32 myself since a couple of years, so I didn't test this recently).
As for factoring out common parts of project/
vcxprojfiles, it's only a little more effort.
Yes, this is definitely doable, but it "just" needs to be done. To give some context, we've been wanting to automate the generation of these project files and add them for all the samples, but I've never found a good enough way to do it, so I've finally started just adding them by hand, but it's a laborious process (there are ~100 of them) and I didn't finish yet.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee pushed 5 commits.
- 5bb546e Add ARM64 entries to minimal sample MSW
- cbcff79 Add ARM64 entries to test and test_gui vc17 solutions
- 46db218 Add ARM64 entries to VC15 and VC16 solutions
- 4d633e7 Add ARM64EC configs to test and test_gui solutions
- f8c60bb Add ARM64EC to minimal
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee pushed 12 commits.
- 230f5e0 Add ARM64 and ARM64EC Platforms to projects and vc17 solution
- c2f295c Start doc update for ARM64 Windows support
- e9f4cd8 Revert .filters change to be done later
- 5d64cb1 Add MSW-ARM64-specific documentation notes
- 62b5b01 Add MSW vc15 and vc16 .sln entries
- 8881484 Attempt to add CI
- 8d2f038 Add ARM64 and ARM64EC cases to wxArchSuffix logic
- 9720a0e Add ARM64 entries to minimal sample MSW
- cba70f3 Add ARM64 entries to test and test_gui vc17 solutions
- e542c10 Add ARM64 entries to VC15 and VC16 solutions
- a263b63 Add ARM64EC configs to test and test_gui solutions
- ef1496a Add ARM64EC to minimal
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
OK, I finished getting the minimal sample, tests, and GUI tests building, got it rebased onto the merged changes from the other PR, and pushed it. Here's hoping CI is passing now--the tasks pass locally. :)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee pushed 1 commit.
- 4769bab ARM64 for minimal vc15 and vc16
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
I poked around with this a bit. Adding the minimal configuration is hitting an error around not being able to find
wx/setup.h. As noted with ARM64X build, this bit of infra needs a little love.I was going to write that this needs to be addressed by just adding another arch to
build/msw/wx_setup.props, but I think you've already done it, so is this still a problem?
Nope, and it needed to be done anyhow.
- I can add the
iflogic to the minimal example and test jobs along with the run jobs. This just punts on the configuration problem. I see that Win32 is also missing from the minimal example.Sorry, I don't understand this at all,
samples/minimal/minimal.vcxprojclearly defines Win32 configurations and I'm pretty sure building them works (even though I stopped using Win32 myself since a couple of years, so I didn't test this recently).
I think I misread something in haste (VS isn't consistent about how it orders sections) and yes, it's fine.
As for factoring out common parts of project/
vcxprojfiles, it's only a little more effort.Yes, this is definitely doable, but it "just" needs to be done. To give some context, we've been wanting to automate the generation of these project files and add them for all the samples, but I've never found a good enough way to do it, so I've finally started just adding them by hand, but it's a laborious process (there are ~100 of them) and I didn't finish yet.
This is a hard one. Directory.Build.props and .targets mean no manual Imports and can mean the individual project files can be very small, bit it's still one each and...100 is a lot.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
In .github/workflows/ci_msw.yml:
> @@ -113,6 +116,7 @@ jobs:
msbuild /noLogo /m '/p:Configuration=${{ matrix.configuration }}' /p:Platform=${{ matrix.platform }} tests\test_vc16.sln
- name: Run tests
+ if: ${{ !contains(matrix.platform, 'arm64') }}
Do the tests fail under ARM or is there some other reason to exclude them?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee commented on this pull request.
In .github/workflows/ci_msw.yml:
> @@ -113,6 +116,7 @@ jobs:
msbuild /noLogo /m '/p:Configuration=${{ matrix.configuration }}' /p:Platform=${{ matrix.platform }} tests\test_vc16.sln
- name: Run tests
+ if: ${{ !contains(matrix.platform, 'arm64') }}
The ARM64 tests need to run on ARM64 hardware, and GitHub-hosted ARM64 runners are currently in public beta. Since it sounds like setting those up currently involves a GitHub Enterprise admin, I opted to skip running them this round. What do you think?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee commented on this pull request.
In .github/workflows/ci_msw.yml:
> @@ -113,6 +116,7 @@ jobs:
msbuild /noLogo /m '/p:Configuration=${{ matrix.configuration }}' /p:Platform=${{ matrix.platform }} tests\test_vc16.sln
- name: Run tests
+ if: ${{ !contains(matrix.platform, 'arm64') }}
(I'm working through setting up and running the tests now, as yes, they should be passing for check-in.)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee commented on this pull request.
In .github/workflows/ci_msw.yml:
> @@ -113,6 +116,7 @@ jobs:
msbuild /noLogo /m '/p:Configuration=${{ matrix.configuration }}' /p:Platform=${{ matrix.platform }} tests\test_vc16.sln
- name: Run tests
+ if: ${{ !contains(matrix.platform, 'arm64') }}
With the tests folder as the working directory and DPI set to 100%, all tests pass for ARM64, ARM64EC, and x64 on ARM64 Windows 11 (26100). (Since the _gui tests rely on pixel counts, non-unity DPI breaks a number of them--but that isn't ARM64-specific and is easy to control for.)
vc_x64_mswu_test.txt
vc_x64_mswu_test_gui.txt
vc_arm64_mswu_test.txt
vc_arm64_mswu_test_gui.txt
vc_arm64ec_mswu_test.txt
vc_arm64ec_mswu_test_gui.txt
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
In .github/workflows/ci_msw.yml:
> @@ -113,6 +116,7 @@ jobs:
msbuild /noLogo /m '/p:Configuration=${{ matrix.configuration }}' /p:Platform=${{ matrix.platform }} tests\test_vc16.sln
- name: Run tests
+ if: ${{ !contains(matrix.platform, 'arm64') }}
Thanks! Do you plan updating this PR to run the tests in the CI builds?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee commented on this pull request.
In .github/workflows/ci_msw.yml:
> @@ -113,6 +116,7 @@ jobs:
msbuild /noLogo /m '/p:Configuration=${{ matrix.configuration }}' /p:Platform=${{ matrix.platform }} tests\test_vc16.sln
- name: Run tests
+ if: ${{ !contains(matrix.platform, 'arm64') }}
Currently, no. It would require setting up and manually managing runners for the wxWidgets org and moving the tests to a second runner to execute--or just building ARM64 on an ARM64 runner then running the tests locally.
I'm happy to either a) adjust this PR to not touch CI at all until that's possible (or whatever scope you feel is appropriate) or b) wait until GH-hosted ARM64 Windows runners are available. (Technically they're available now in beta, but I'm not sure how much you'd have to do from the wxWidgets org perspective to set them up.)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee pushed 1 commit.
- b7286d3 Merge branch 'wxWidgets:master' into jkunkee-arm64-arm64ec
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
In .github/workflows/ci_msw.yml:
> @@ -113,6 +116,7 @@ jobs:
msbuild /noLogo /m '/p:Configuration=${{ matrix.configuration }}' /p:Platform=${{ matrix.platform }} tests\test_vc16.sln
- name: Run tests
+ if: ${{ !contains(matrix.platform, 'arm64') }}
Thanks, I'll just (squash) merge this for now then (unless you prefer to rebase it to keep your commits).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jon Kunkee
@jkunkee commented on this pull request.
In .github/workflows/ci_msw.yml:
> @@ -113,6 +116,7 @@ jobs:
msbuild /noLogo /m '/p:Configuration=${{ matrix.configuration }}' /p:Platform=${{ matrix.platform }} tests\test_vc16.sln
- name: Run tests
+ if: ${{ !contains(matrix.platform, 'arm64') }}
Sounds good to me; I prefer squash merges myself.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.