[BUG] CMake 3.21 + NDK r23 sets MINGW=1 in CMake #1581

jheydebrand · 2021-09-09T13:01:43Z

Description

Observed behaviour:
When using CMake 3.21 with NDK r23 the compiler detection performed by CMake identifies NDK's Clang toolchain as MINGW.
This causes failures during the subsequent build because MINGW=1 implies that the Windows platform is targeted which means
Windows specific files and libraries might be included into the build.

$ cat CMakeLists.txt
cmake_minimum_required(VERSION 3.21.0)

project(NewAndroidCMakeRepro LANGUAGES C CXX)

if(DEFINED MINGW)
    message(STATUS "MINGW defined")
endif()
message(STATUS "MINGW=${MINGW}")
message(STATUS "CMAKE_SYSTEM_NAME=${CMAKE_SYSTEM_NAME}")

# find_program(git git REQUIRED)


$ cat tc.txt
set(ANDROID_CPP_FEATURES rtti exceptions)
set(ANDROID_ABI arm64-v8a)
set(ANDROID_STL c++_shared)
set(ANDROID_NATIVE_API_LEVEL 27)

# Needed for find_program(git) because Android is the only CMake platform which explicitly sets this to OFF
# Only required when ANDROID_USE_LEGACY_TOOLCHAIN_FILE=OFF
set(CMAKE_FIND_USE_SYSTEM_ENVIRONMENT_PATH 1)

# Force compiler ID detection to build for the target instead of the host and thus avoid MINGW=1 on Windows host
# Mismatching these flags with ANDROID_NATIVE_API_LEVEL and ANDROID_ABI might be really bad...
#set(CMAKE_CXX_FLAGS "--target=armv7-none-linux-androideabi16")
#set(CMAKE_C_FLAGS "--target=armv7-none-linux-androideabi16")

# Legacy toolchain does not cause MINGW=1 even in the absence of the CMAKE_<LANG>_FLAGS from above
#set(ANDROID_USE_LEGACY_TOOLCHAIN_FILE 1)

include("/path/to/ndk/23.0.7599858/build/cmake/android.toolchain.cmake")


$ cmake.exe  -GNinja -DCMAKE_TOOLCHAIN_FILE=tc.txt -S . -B .
-- Android: Targeting API '27' with architecture 'arm64', ABI 'arm64-v8a', and processor 'aarch64'
-- Android: Selected unified Clang toolchain
-- The C compiler identification is Clang 12.0.5
-- The CXX compiler identification is Clang 12.0.5
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /path/to/ndk/23.0.7599858/toolchains/llvm/prebuilt/windows-x86_64/bin/clang.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /path/to/ndk/23.0.7599858/toolchains/llvm/prebuilt/windows-x86_64/bin/clang++.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- MINGW defined
-- MINGW=1
-- CMAKE_SYSTEM_NAME=Android
-- Configuring done
-- Generating done
-- Build files have been written to: ./build

Expected behaviour:
The MINGW variable is not defined in CMake.

The expected behaviour can be restored either by setting ANDROID_USE_LEGACY_TOOLCHAIN_FILE=1 or by manipulating CMAKE_<LANG>_FLAGS in tc.txt.

Environment Details

cmake version 3.21.1; CMake suite maintained and supported by Kitware (kitware.com/cmake).
Windows 10 Host system (Build 19042.631)
NDK Version 23.0.7599858
ABI (arm64-v8a) and API level (27) do not seem to matter

The text was updated successfully, but these errors were encountered:

DanAlbert · 2021-09-14T20:59:09Z

@bradking hoping you can offer some advice here. It looks like the bug here is on the CMake side. If I've understood correctly, https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/CMakeDetermineCXXCompiler.cmake#L120 can only ever identify the default target of the compiler since the -target flag won't be in cxxflags until after the compiler has been proven to be clang. All this code is unfamiliar to me though, so there's a pretty good chance I'm wrong :) lmk if you agree and I'll file a bug over on the cmake side.

bradking · 2021-09-15T13:35:00Z

@DanAlbert you're correct. The logic here predates Clang or any notion of a compiler that supports more than one target. Therefore for Clang, it determines the default target. It can probably be updated to be aware of CMAKE_<LANG>_COMPILER_TARGET.

DanAlbert · 2021-09-15T19:09:01Z

Thanks for confirming. Filed https://gitlab.kitware.com/cmake/cmake/-/issues/22647

We might be able to forcibly add the target argument to the compiler name so that CMake can't avoid it in compiler ID, but idk if that'll cause other problems.

If that doesn't work we probably need to bump the CMake version that's required for using the non-legacy toolchain since this does render this path quite unusable on windows (and probably has subtle bugs elsewhere). Unfortunately that would mean effectively unshipping this feature for now since there obviously is no CMake version with the fix yet.

DanAlbert · 2021-09-15T19:54:07Z

Okay, looks like I can add this to the (new) toolchain file as a workaround:

if(CMAKE_ANDROID_ARCH_ABI STREQUAL armeabi-v7a)
  set(ANDROID_LLVM_TRIPLE armv7-none-linux-androideabi)
elseif(CMAKE_ANDROID_ARCH_ABI STREQUAL arm64-v8a)
  set(ANDROID_LLVM_TRIPLE aarch64-none-linux-android)
elseif(CMAKE_ANDROID_ARCH_ABI STREQUAL x86)
  set(ANDROID_LLVM_TRIPLE i686-none-linux-android)
elseif(CMAKE_ANDROID_ARCH_ABI STREQUAL x86_64)
  set(ANDROID_LLVM_TRIPLE x86_64-none-linux-android)
else()
  message(FATAL_ERROR "Invalid Android ABI: ${ANDROID_ABI}.")
endif()
set(CMAKE_C_COMPILER_TARGET "${ANDROID_LLVM_TRIPLE}${CMAKE_SYSTEM_VERSION}")
set(CMAKE_CXX_COMPILER_TARGET "${ANDROID_LLVM_TRIPLE}${CMAKE_SYSTEM_VERSION}")

(thanks Brad for the hint on the other bug)

It won't help anyone not using our toolchain file, but I wasn't having any luck finding another way to do it, and our toolchain file is the workflow with the regression, so I think that's probably fine?

DanAlbert · 2021-09-15T21:17:40Z

https://android-review.googlesource.com/c/platform/ndk/+/1827522 adds a workaround.

hhb · 2021-09-16T03:10:58Z

CMAKE_CXX_COMPILER_TARGET is used at CMakeDetermineCXXCompiler.cmake.
It then includes Android-Determine-CXX.cmake.
And then includes Android/Determine-Compiler.cmake.
We have a hook at the beginning of Android/Determine-Compiler.cmake.

So we can set CMAKE_CXX_COMPILER_TARGET at ${CMAKE_ANDROID_NDK}/build/cmake/hooks/pre/Determine-Compiler.cmake. It will be included by Android/Determine-Compiler.cmake. This will work for both cmake users and toolchain file users.

DanAlbert · 2021-09-16T04:37:10Z

Aha, thanks! I'll try that change tomorrow. I'd poked around in a few of the hooks but whatever I'd tried didn't work. Haven't really gotten familiar with the new system yet.

hhb · 2021-09-16T05:29:38Z

Aha, thanks! I'll try that change tomorrow. I'd poked around in a few of the hooks but whatever I'd tried didn't work. Haven't really gotten familiar with the new system yet.

Yeah.. sorry for all the mess I left here...😭

DanAlbert · 2021-09-16T08:12:46Z

Not so much a mess as just I haven't taken the time to learn it yet :)

DanAlbert · 2021-09-16T22:39:55Z

Uploaded https://android-review.googlesource.com/c/platform/ndk/+/1829052 to apply the workaround better and https://android-review.googlesource.com/c/platform/ndk/+/1829092 to make it non-permanent, since Brad already has a fix up targeted at 3.22. We shouldn't submit the latter until the CMake fix actually lands so not on autosubmit.

DanAlbert · 2021-09-16T23:27:20Z

Hmm, although that breaks the legacy toolchain because apparently the hooks are loaded even in that case (where CMAKE_SYSTEM_VERSION is always 1 because that's how we tell CMake to disable all its own behavior). I thought the hooks were only supposed to be used for the non-legacy toolchain? Am I remembering wrong, or is that a bug? If the hooks need to support both workflows that's going to get messy fast.

DanAlbert · 2021-09-16T23:33:07Z

For now I've just opted that hook out. We may want to copy/paste that into all the hooks.

bradking · 2021-09-17T13:17:45Z

The hooks are there so that the NDK can add logic before/after CMake's logic. The hooks themselves can start with some kind of if(legacy) => return condition if they don't want to work for the legacy toolchain.

DanAlbert · 2021-09-17T19:30:13Z

Gotcha. That's what I've done, but wasn't sure if that was the intent. Thanks for confirming.

If we don't explicitly set the target CMake will ID the compiler using the default target, causing MINGW to be defined when a Windows host is used. Test: added Bug: android/ndk#1581 Bug: https://gitlab.kitware.com/cmake/cmake/-/issues/22647 Change-Id: I3c4829eefd4fad6fbe85709c0d045b5e595b7b8e (cherry picked from commit 063d2b8)

Move this into the hook so it works for non-toolchain file users too. Thanks hhb for the pointer! Test: re-tested this on Windows Bug: android/ndk#1581 Change-Id: I39a1c8646d5853eea4bf557c6f6efa2ca5244cf2 (cherry picked from commit 02dfb02)

Test: added a FATAL_ERROR log and checked with a TOT CMake Bug: android/ndk#1581 Change-Id: Ic57323b1a4ac83ab02d29768932e67105c1fc2ed

jheydebrand added the bug label Sep 9, 2021

DanAlbert added this to Triaged in r23b via automation Sep 9, 2021

DanAlbert moved this from Triaged to Needs cherry-pick in r23b Sep 17, 2021

DanAlbert moved this from Needs cherry-pick to Merged in r23b Dec 14, 2021

DanAlbert closed this as completed Oct 19, 2022

osspop pushed a commit to osspop/android-ndk that referenced this issue Jan 17, 2023

Disable CMake workaround when using 3.22+.

d8fcfb2

Test: added a FATAL_ERROR log and checked with a TOT CMake Bug: android/ndk#1581 Change-Id: Ic57323b1a4ac83ab02d29768932e67105c1fc2ed

prince-chrismc mentioned this issue Aug 27, 2023

Add example for cross-build to android with CCI recipe conan-io/examples2#113

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG] CMake 3.21 + NDK r23 sets MINGW=1 in CMake #1581

[BUG] CMake 3.21 + NDK r23 sets MINGW=1 in CMake #1581

[BUG] CMake 3.21 + NDK r23 sets MINGW=1 in CMake #1581

[BUG] CMake 3.21 + NDK r23 sets MINGW=1 in CMake #1581

Comments

Description

Environment Details