Skip to content

[vcpkg/toolchain] set CMAKE_SYSTEM_PROCESSOR for arm on windows#16111

Closed
Neumann-A wants to merge 13 commits intomicrosoft:masterfrom
Neumann-A:fix_arm_windows
Closed

[vcpkg/toolchain] set CMAKE_SYSTEM_PROCESSOR for arm on windows#16111
Neumann-A wants to merge 13 commits intomicrosoft:masterfrom
Neumann-A:fix_arm_windows

Conversation

@Neumann-A
Copy link
Copy Markdown
Contributor

@Neumann-A Neumann-A commented Feb 8, 2021

Correct the windows toolchain to correctly set CMAKE_SYSTEM_PROCESSOR so that CMake goes into CROSSCOMPILING mode.

closes #9390

@PhoebeHui PhoebeHui self-assigned this Feb 8, 2021
@PhoebeHui PhoebeHui added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Feb 9, 2021
@cenit
Copy link
Copy Markdown
Contributor

cenit commented Feb 9, 2021 •
edited
Loading

as a side note, opencv is already compatible with arm64/arm cross compiling without this PR in any case (#9390 is about opencv)

@Neumann-A
Copy link
Copy Markdown
Contributor Author

Neumann-A commented Feb 9, 2021

@cenit Did you patch it to mark it work or does it work by default? I interpreted #9390 as an request to correct the toolchain and obviously the toolchain is currently wrong for arm builds.

@PhoebeHui : I am not going to correct the baseline arm errors here. I just want to make it obvious that the arm setup is currently incorrect from a CMake based point of view.

@PhoebeHui
Copy link
Copy Markdown
Contributor

PhoebeHui commented Feb 9, 2021

@Neumann-A, I will take a look at the baseline issues.

@cenit
Copy link
Copy Markdown
Contributor

cenit commented Feb 9, 2021 •
edited
Loading

There are some specific lines in the portfile to deal with the issue: https://github.com/microsoft/vcpkg/blob/master/ports/opencv4/portfile.cmake#L27-L35

They are recent, since 4.5.0 PR. I forgot to link that issue to close it, but your solution is more general and more correct.

@PhoebeHui it's very probable that those libraries that now are failing before were built as PE binary files for x86 archs due to these missing flags....

ras0219-msft
ras0219-msft suggested changes Feb 9, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

This is an important fix, thank you!

@Neumann-A Neumann-A mentioned this pull request Feb 11, 2021
Closed
@rimrul rimrul mentioned this pull request Mar 7, 2021
Closed
1 task
@dscho
Copy link
Copy Markdown
Member

dscho commented Mar 8, 2021

@Neumann-A gentle ping: was there any progress?

@ras0219-msft
Copy link
Copy Markdown
Contributor

ras0219-msft commented Mar 25, 2021

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Mar 25, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@ras0219-msft
Copy link
Copy Markdown
Contributor

ras0219-msft commented Apr 19, 2021

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Apr 19, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@PhoebeHui
Copy link
Copy Markdown
Contributor

PhoebeHui commented Apr 20, 2021 •
edited
Loading

CI failures:

  • forest: known baseline issue, would be fixed by PR [vcpkg baseline] [forest] Remove forest #16836
  • vc:x64-uwp
  • kfr:x64-uwp
  • ampl-mp:arm64-windows
  • corrade:arm64-windows
  • gainput:arm64-windows
  • glog:arm64-windows
  • gppanel:arm64-windows
  • orc:arm64-windows
  • fltk:arm64-windows
  • halide:x86-windows
  • dcmtk:x86-windows
  • marble:x64-windows: has been fixed.

@PhoebeHui PhoebeHui mentioned this pull request Apr 25, 2021
Merged
@Neumann-A
Copy link
Copy Markdown
Contributor Author

Neumann-A commented May 10, 2021

Qt6 and the cmake test port require these changes to correctly compile for ARM

@strega-nil-ms
Copy link
Copy Markdown
Contributor

strega-nil-ms commented Jun 7, 2021

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Jun 7, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Jul 22, 2021

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Jul 22, 2021

Azure Pipelines successfully started running 1 pipeline(s).

strega-nil-ms
strega-nil-ms reviewed Jul 22, 2021
View reviewed changes
@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Sep 10, 2021

Ping for response.

cenit
cenit reviewed Oct 21, 2021
View reviewed changes
scripts/toolchains/windows.cmake Outdated
Copy link
Copy Markdown
Contributor

@cenit cenit Oct 21, 2021

Choose a reason for hiding this comment

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

ARM64 on win11 can run also AMD64 code

Copy link
Copy Markdown
Contributor

@cenit cenit Oct 21, 2021

Choose a reason for hiding this comment

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

(and yes, ARM64 can run ARM code)

Copy link
Copy Markdown
Contributor Author

@Neumann-A Neumann-A Oct 21, 2021

Choose a reason for hiding this comment

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

what does OS_VERSION return for windows 11 ?

@dg0yt dg0yt mentioned this pull request Dec 4, 2021
Merged
@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Dec 4, 2021

Note that the windows.cmake toolchain file is not loaded by vcpkg_cmake_configure at the moment, #21857.

This is an important fix, thank you!

And it still is, 10 months later. Some packages rely on CMAKE_SYSTEM_PROCESSOR being defined.(if(${CMAKE_SYSTEM_PROCESSOR} STREQUAL "x86") causes a cmake error.)

@dg0yt dg0yt mentioned this pull request Dec 5, 2021
Closed
@Neumann-A Neumann-A mentioned this pull request Feb 12, 2022
Merged
@BillyONeal BillyONeal mentioned this pull request Feb 17, 2022
Closed
@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Mar 18, 2022

Any progress here?

@Neumann-A Neumann-A mentioned this pull request Apr 8, 2022
Closed
12 tasks
@strega-nil-ms strega-nil-ms mentioned this pull request Apr 12, 2022
Merged
@Neumann-A
Copy link
Copy Markdown
Contributor Author

Neumann-A commented Apr 20, 2022

superseded by #24100

@Neumann-A Neumann-A closed this Apr 20, 2022
@WangWeiLin-MV WangWeiLin-MV mentioned this pull request Aug 16, 2024
Merged
@Neumann-A Neumann-A deleted the fix_arm_windows branch November 13, 2024 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] approved these changes

+6 more reviewers

@ras0219 ras0219 ras0219 left review comments

@cenit cenit cenit left review comments

@davidebeatrici davidebeatrici davidebeatrici left review comments

@dg0yt dg0yt dg0yt left review comments

@strega-nil-ms strega-nil-ms strega-nil-ms left review comments

@ras0219-msft ras0219-msft ras0219-msft requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@PhoebeHui PhoebeHui

@JackBoosY JackBoosY

Labels

category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Missing CMAKE_SYSTEM_PROCESSOR variable in cross-compiling