Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

User-specified compilation flags are ignored #164

Open
numberZero opened this issue Feb 21, 2023 · 4 comments
Open

User-specified compilation flags are ignored #164

numberZero opened this issue Feb 21, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@numberZero
Copy link
Contributor

numberZero commented Feb 21, 2023

set(CMAKE_CXX_FLAGS_RELEASE "-O3")
set(CMAKE_CXX_FLAGS_DEBUG "-g")

This was probably intended to provide default values for CMAKE_CXX_FLAGS_RELEASE and (for whatever reason) CMAKE_CXX_FLAGS_DEBUG. Instead, it sets regular variables which hide the user-specified (cache) variables with hard-coded values.

E.g. I have this in my CMakeCache:
CMAKE_BUILD_TYPE:STRING=Release
CMAKE_CXX_FLAGS_RELEASE:STRING=-O3 -DNDEBUG
But actual compilation command doesn’t contain -DNDEBUG.

@sfan5 sfan5 added the bug Something isn't working label Feb 21, 2023
@sfan5
Copy link
Member

sfan5 commented Sep 13, 2023

Minetest does this too:
https://github.com/minetest/minetest/blob/master/src/CMakeLists.txt#L791-L801

aren't users supposed to put their own wishes into CMAKE_CXX_FLAGS?

@JosiahWI
Copy link
Contributor

JosiahWI commented Sep 13, 2023

CMake has variables by these names since version 3.11 as documented.

Our variables by the same names are, to my knowledge, internal. This is very confusing, and I suggest we change the names of our variables to something like IRR_CXX_FLAGS_RELEASE and IRR_CXX_FLAGS_DEBUG so they won't be confused with the built-in variables in 3.11 and onward. Alternatively, we could increase our minimum CMake version to 3.11, and use list(APPEND ...) instead of set(...) so that the cache variables are not shadowed. Yet again, we could set them as CACHE variables, so that the user provided values override them. The latter two solutions would need additional documentation, of course.

Also note that there's a documented variable that can be used to initialize the value of CMAKE_<LANG>_FLAGS_<CONFIG>. This offers a fourth solution.

The last link explicitly states that CMAKE_<LANG>_FLAGS_<CONFIG> is a cache variable, which means it should be possible for the user to override it from the command line. Our behavior is contradictory to the official CMake documentation for CMake version 3.11 and onwards.

@numberZero
Copy link
Contributor Author

CMake has variables by these names since version 3.11 as documented.

CMake 2.6 has these already, just documented separately:
https://cmake.org/cmake/help/cmake2.6docs.html#variable:CMAKE_LANG_FLAGS_DEBUG
https://cmake.org/cmake/help/cmake2.6docs.html#variable:CMAKE_LANG_FLAGS_RELEASE

@sfan5
Copy link
Member

sfan5 commented Sep 17, 2023

Sounds like we need to fix it in MT and Irrlicht then, PRs welcome.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants