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

Suggestion for CMake statement ordering #1241

Closed
foundry-markf opened this issue Jun 11, 2020 · 1 comment
Closed

Suggestion for CMake statement ordering #1241

foundry-markf opened this issue Jun 11, 2020 · 1 comment

Comments

@foundry-markf
Copy link

Description of Issue

The top-level CMakeLists.txt (in the dev branch) currently begins with:

project(usd)

cmake_minimum_required(VERSION 3.12)

The documentation for CMake says that project should come after cmake_minimum_required. See https://cmake.org/cmake/help/latest/command/project.html#usage
and https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html#cmake-minimum-required

In switching these statements around, for macOS builds (albeit still experimental) at least, this proposal changes the policy that identifies the compiler as AppleClang rather than Clang. See https://cmake.org/cmake/help/latest/policy/CMP0025.html
So a change to cmake/defaults/CXXDefaults.cmake is also required, i.e.

if (CMAKE_COMPILER_IS_GNUCXX)
    include(gccdefaults)
elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
    include(clangdefaults)
elseif(MSVC)
    include(msvcdefaults)
endif()

to change the STREQUAL to MATCHES.
All the other cases I have found for clang already use MATCHES or explicitly check for AppleClang.

System Information (OS, Hardware)

I've tried this out on a macOS build on a much older version of USD, and has made my builds behave as I would expect when using CMake 3+.

I've not tried it on other platforms (yet), but it appears to be making the USD CMake scripts more inline with recommended practices.

Thanks

@jtran56
Copy link

jtran56 commented Jun 12, 2020

Filed as internal issue #USD-6120.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants