-
Notifications
You must be signed in to change notification settings - Fork 6.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix CMake detection of cross-compilers on arm/arm64 build arch #35084
Conversation
Signed-off-by: Vasyl Gello <vasek.gello@gmail.com>
Rustdesk is built not only on amd64 hosts but aarch64 builders. CMake detects Debian/Ubuntu cross-compilers only for x86_64 architecture. With this patch, aarch64 builders produce arm-linux builds. |
@dg0yt please have a look here :) |
Is the use of prefixed tool names the common solution to target From a more general perspective, the toolchain should probably look for |
Yes, and that's not only in Debian (where I test it all). Unfortunately -m32 / -m64 is x86 specific - other architectures in GCC have this convention. Clang is different - see |
@@ -13,7 +13,7 @@ elseif(VCPKG_TARGET_ARCHITECTURE STREQUAL "x86") | |||
string(APPEND VCPKG_LINKER_FLAGS " -m32") | |||
elseif(VCPKG_TARGET_ARCHITECTURE STREQUAL "arm") | |||
set(CMAKE_SYSTEM_PROCESSOR armv7l CACHE STRING "") | |||
if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux" AND CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64") | |||
if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the test entirely seems wrong as that will do the wrong thing when arm is the native architecture?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong, yes. But not worse than current support for modern arm64 hosts.
How much of vcpkg works out of the box on 32-bit arm hosts? The user must provide cmake and ninja, so maybe it is fair to ask for providing the compiler variables, too.
Couldn't we do find_program(var NAMES <prefix>-tool tool ...)
? This would consider user input, prefix, default. Hard-coded values belong into custom toolchain files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I declare the table of possible combinations of host & target instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't we do
find_program(var NAMES <prefix>-tool tool ...)
? This would consider user input, prefix, default. Hard-coded values belong into custom toolchain files.
FTR find_program
is what scripts/cmake/mingw.cmake
does:
vcpkg/scripts/toolchains/mingw.cmake
Lines 24 to 25 in 23ceb9c
find_program(CMAKE_C_COMPILER "${CMAKE_SYSTEM_PROCESSOR}-w64-mingw32-gcc") | |
find_program(CMAKE_CXX_COMPILER "${CMAKE_SYSTEM_PROCESSOR}-w64-mingw32-g++") |
(However, I noticed this now due to insufficient initalization of CMAKE_SYSTEM_PROCESSOR
in a test project.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong, yes. But not worse than current support for modern arm64 hosts.
OK, I'm sold by 'not worse than the status quo'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user must provide cmake and ninja, so maybe it is fair to ask for providing the compiler variables, too.
Sadly that doesn't seem to be so easy, overriding the variables from the command-line doesn't work... maybe via a triplet, but that would introduce a fair bit of hassle for anyone who wants to perform a native build on an ARM machine. Ideally, we should just make the defaults work (see my comment below and #38113).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, we should just make the defaults work
Yes, we can agree on that, and #38113 is a good direction. It is just not the whole discussion from the last days.
Thanks! |
Unfortunately, this did actually break things. Most arm64 Linux hosts report as aarch64, causing vcpkg to think that it cross-compiles, which then results in very obscure errors such as:
with
with
From my experimentation, the issue can be solved either
|
#35084 introduced a regression for native ARM Linux builds where the toolchain would mistakenly try to reference a cross-compiler (e.g. `aarch64-linux-gnu-gcc`) despite building natively because vcpkg spells the architecture differently from what the system reports (`arm64` vs. `aarch64`). This fixes the issue by checking whether the `CMAKE_HOST_SYSTEM_PROCESSOR` matches the target (in CMake's spelling of the architecture). --------- Co-authored-by: Kai Pastor <dg0yt@darc.de>
Fixes cross-building arm-linux / arm64-linux hon non-x86_64 hosts