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: Remove remaining checks for CMAKE_VERSION #4293

Merged
merged 1 commit into from
Jun 21, 2023
Merged

cmake: Remove remaining checks for CMAKE_VERSION #4293

merged 1 commit into from
Jun 21, 2023

Conversation

juan-lunarg
Copy link
Contributor

Remove conditional code that doesn't need to exist anymore.

Remove conditional code that doesn't need to exist anymore.
if (CMAKE_VERSION VERSION_LESS "2.8.12")
add_compile_options("-Wa,-mbig-obj")
else()
add_definitions("-Wa,-mbig-obj")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add_definitions is used to add defines to the code like NOMINMAX or WIN32_LEAN_AND_MEAN using it for compiler options isn't correct.

Also add_definitions isn't recommended for usage anymore:
https://cmake.org/cmake/help/latest/command/add_definitions.html#command:add_definitions

return()
endif()

get_cmake_property(is_multi "GENERATOR_IS_MULTI_CONFIG")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note GENERATOR_IS_MULTI_CONFIG was added in 3.9 and should be used here instead of CMAKE_CONFIGURATION_TYPES.

https://cmake.org/cmake/help/latest/prop_gbl/GENERATOR_IS_MULTI_CONFIG.html

@juan-lunarg
Copy link
Contributor Author

There are now no more instances of CMAKE_VERSION in the codebase 💯

@dinord dinord self-assigned this Jun 21, 2023
@dinord
Copy link
Collaborator

dinord commented Jun 21, 2023

Thanks for the cleanup! Just for posterity, explicitly running python3 in cmake is based on the fact that googletest's python tests are actually written in python3 (as of a few years ago).

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

Successfully merging this pull request may close these issues.

2 participants