Skip to content
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

CMake: Use CMAKE_CXX_STANDARD instead of passing -std=c++14 #4703

Merged
merged 2 commits into from Mar 16, 2017

Conversation

ligfx
Copy link
Contributor

@ligfx ligfx commented Jan 21, 2017

Available since CMake 3.1

@lioncash
Copy link
Member

set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

will likely be preferable to have set as well.

@ligfx
Copy link
Contributor Author

ligfx commented Jan 21, 2017

set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

will likely be preferable to have set as well.

Sure, I'll add those in and see what happens.

@ligfx
Copy link
Contributor Author

ligfx commented Jan 22, 2017

The Android build seems to be running into android/ndk#222, where something in the NDK's toolchain file prevents CMAKE_CXX_STANDARD from working.

@Orphis
Copy link
Member

Orphis commented Jan 22, 2017

Yes, and I knew of the issue, which is why I didn't use the flag.

@Orphis
Copy link
Member

Orphis commented Jan 22, 2017

On a side note, the Android support in 3.7 (independent from Google's Gradle integration) works fine with it. Not sure we want to use it though...

@ligfx
Copy link
Contributor Author

ligfx commented Jan 23, 2017

@Orphis: there are two proposed fixes up for AOSP (https://android-review.googlesource.com/#/c/326383/ (mine) and https://android-review.googlesource.com/#/c/324419/). Hopefully something will get merged for r14, and then CMAKE_CXX_STANDARD will work as intended.

In the meantime... what do you think about something like

# The Android NDK toolchain file is broken.
# See https://github.com/android-ndk/ndk/issues/222
if(CMAKE_TOOLCHAIN_FILE MATCHES "android.toolchain.cmake$")
  set(_new_toolchain_file "${CMAKE_CURRENT_BINARY_DIR}/android.toolchain.cmake")
  if(NOT EXISTS "${_new_toolchain_file}")
    file(TO_CMAKE_PATH "${CMAKE_TOOLCHAIN_FILE}" CMAKE_TOOLCHAIN_FILE)
    file(WRITE "${_new_toolchain_file}" "
      include(\"${CMAKE_TOOLCHAIN_FILE}\")
      set(CMAKE_C_COMPILER_ID_RUN FALSE)
      set(CMAKE_CXX_COMPILER_ID_RUN FALSE)
    ")
    set(CMAKE_TOOLCHAIN_FILE "${_new_toolchain_file}")
  endif()
endif()

? Too much?

@Orphis
Copy link
Member

Orphis commented Jan 23, 2017

Something will be merged for r14, the issue is tagged with r14 in Github after all.
Yours look much better and I wonder why CMake would have issues detecting the compiler at all.

@degasus
Copy link
Member

degasus commented Mar 14, 2017

The android buildbot was updated a few weeks ago, may you rebase and ask anyone to merge it? LGTM.

@Helios747 Helios747 merged commit 3b1dae5 into dolphin-emu:master Mar 16, 2017
@ligfx ligfx deleted the cmake_cxx_standard branch May 24, 2017 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants