Skip to content

Commit

Permalink
Don't manually set CMAKE_C_COMPILER_ID in toolchain file
Browse files Browse the repository at this point in the history
Bug: android/ndk#222 : CMake toolchain
file's `CMAKE_CXX_COMPILER_ID_RUN=true` breaks compiler feature
detection
Bug: android/ndk#253 : cmake toolchain
file does not set CMAKE_CXX_COMPILER_VERSION
Bug: https://gitlab.kitware.com/cmake/cmake/issues/16587 : Adding
"-march=" to flags breaks determining compiler ID when cross-compiling
with Clang

The current comment reads:

> Clang can fail to compile if CMake doesn't correctly supply the target
> and external toolchain, but to do so, CMake needs to already know
> that the compiler is clang. Tell CMake that the compiler is really
> clang, but don't use CMakeForceCompiler, since we still want
> compile checks. We only want to skip the compiler ID detection
> step.

The issue arises in CMakeDetermineCompilerId.cmake, which passes along
user-specified flags like `-march=armv5te` but not the specified
compiler target. On Clang, this fails, since the validity of `-march`
depends on the current target.

I've filed a bug for CMake to fix this (allow determining the compiler
ID without using user flags). In the meantime, I think a more robust
solution than overriding CMAKE_CXX_COMPILER_ID_RUN is to run the
compiler ID checks before passing along any flags. The
CMakeDetermineCCompiler.cmake and CMakeDetermineCXXCompiler.cmake
modules are not marked for internal-use-only, and this method takes a
more proactive approach to the problem.

Change-Id: I2c250ebc2999b3f251417a9e3730b5e093e446d1
(cherry picked from commit 15909f2)
  • Loading branch information
ligfx authored and DanAlbert committed Jan 24, 2017
1 parent 5f80002 commit f1fef4d
Showing 1 changed file with 9 additions and 13 deletions.
22 changes: 9 additions & 13 deletions build/cmake/android.toolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -322,19 +322,6 @@ if(ANDROID_TOOLCHAIN STREQUAL clang)
set(ANDROID_C_COMPILER "${ANDROID_LLVM_TOOLCHAIN_PREFIX}clang${ANDROID_TOOLCHAIN_SUFFIX}")
set(ANDROID_CXX_COMPILER "${ANDROID_LLVM_TOOLCHAIN_PREFIX}clang++${ANDROID_TOOLCHAIN_SUFFIX}")
set(ANDROID_ASM_COMPILER "${ANDROID_LLVM_TOOLCHAIN_PREFIX}clang${ANDROID_TOOLCHAIN_SUFFIX}")
# Clang can fail to compile if CMake doesn't correctly supply the target and
# external toolchain, but to do so, CMake needs to already know that the
# compiler is clang. Tell CMake that the compiler is really clang, but don't
# use CMakeForceCompiler, since we still want compile checks. We only want
# to skip the compiler ID detection step.
set(CMAKE_C_COMPILER_ID_RUN TRUE)
set(CMAKE_CXX_COMPILER_ID_RUN TRUE)
set(CMAKE_C_COMPILER_ID Clang)
set(CMAKE_CXX_COMPILER_ID Clang)
set(CMAKE_C_COMPILER_VERSION 3.8)
set(CMAKE_CXX_COMPILER_VERSION 3.8)
set(CMAKE_C_STANDARD_COMPUTED_DEFAULT 11)
set(CMAKE_CXX_STANDARD_COMPUTED_DEFAULT 98)
set(CMAKE_C_COMPILER_TARGET ${ANDROID_LLVM_TRIPLE})
set(CMAKE_CXX_COMPILER_TARGET ${ANDROID_LLVM_TRIPLE})
set(CMAKE_ASM_COMPILER_TARGET ${ANDROID_LLVM_TRIPLE})
Expand Down Expand Up @@ -576,6 +563,15 @@ set(CMAKE_AR "${ANDROID_AR}" CACHE FILEPATH "Archiver")
set(CMAKE_RANLIB "${ANDROID_RANLIB}" CACHE FILEPATH "Ranlib")
set(_CMAKE_TOOLCHAIN_PREFIX "${ANDROID_TOOLCHAIN_PREFIX}")

# Run the compiler ID checks before we set flags.
# When passed the `-march=` flag, Clang can fail to compile if CMake doesn't
# correctly supply the target and external toolchain, but to do so, CMake
# needs to already know that the compiler is Clang. Tell CMake to run the
# compiler ID detection step before we set Android flags.
# See https://gitlab.kitware.com/cmake/cmake/issues/16587
include(CMakeDetermineCCompiler)
include(CMakeDetermineCXXCompiler)

# Set or retrieve the cached flags.
# This is necessary in case the user sets/changes flags in subsequent
# configures. If we included the Android flags in here, they would get
Expand Down

0 comments on commit f1fef4d

Please sign in to comment.