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

Unnecessary check for CMAKE_BUILD_TYPE #311

Closed
kevinbeck76 opened this issue Jun 11, 2021 · 1 comment
Closed

Unnecessary check for CMAKE_BUILD_TYPE #311

kevinbeck76 opened this issue Jun 11, 2021 · 1 comment

Comments

@kevinbeck76
Copy link

The check in the top-level CMakeLists.txt:

elseif(NOT (${CMAKE_BUILD_TYPE} STREQUAL "Release" OR ${CMAKE_BUILD_TYPE} STREQUAL "Debug" ))MESSAGE(SEND_ERROR "CMAKE_BUILD_TYPE must be either Release or Debug")

is unnecessary and breaks builds that use CMake build types other than Release or Debug (RelWithDebInfo, release, RELEASE, etc.). The code builds just fine without this check.

According to the documentation CMAKE_BUILD_TYPE names are case-insensitive. We use release or debug in our environment and the check above fails. This is the only package of the 20+ packages that we use that has any checking of the CMAKE_BUILD_TYPE.

CMAKE_BUILD_TYPE
Specifies the build type on single-configuration generators.

This statically specifies what build type (configuration) will be built in this build tree. Possible values are empty, Debug, Release, RelWithDebInfo, MinSizeRel, ... This variable is only meaningful to single-configuration generators (such as Makefile Generators and Ninja) i.e. those which choose a single configuration when CMake runs to generate a build tree as opposed to multi-configuration generators which offer selection of the build configuration within the generated build environment. There are many per-config properties and variables (usually following clean SOME_VAR_ order conventions), such as CMAKE_C_FLAGS_, specified as uppercase: CMAKE_C_FLAGS_[DEBUG|RELEASE|RELWITHDEBINFO|MINSIZEREL|...]. For example, in a build tree configured to build type Debug, CMake will see to having CMAKE_C_FLAGS_DEBUG settings get added to the CMAKE_C_FLAGS settings. See also CMAKE_CONFIGURATION_TYPES.

<Note that configuration names are case-insensitive. The value of this variable will be the same as it is specified when invoking CMake. For instance, if -DCMAKE_BUILD_TYPE=ReLeAsE is specified, then the value of CMAKE_BUILD_TYPE will be ReLeAsE.

studiofuga added a commit to studiofuga/socket.io-client-cpp that referenced this issue Jan 13, 2022
The check for Release or Debug in CMAKE_BUILD_TYPE breaks some assumption in Debian packaging.
Since the check is useless, it has been removed.
studiofuga added a commit to studiofuga/socket.io-client-cpp that referenced this issue Jan 13, 2022
The check for Release or Debug in CMAKE_BUILD_TYPE breaks some assumption in Debian packaging.
Since the check is useless, it has been removed.
studiofuga added a commit to studiofuga/socket.io-client-cpp that referenced this issue Jan 13, 2022
studiofuga added a commit to studiofuga/socket.io-client-cpp that referenced this issue Feb 25, 2022
The check for Release or Debug in CMAKE_BUILD_TYPE breaks some assumption in Debian packaging.
Since the check is useless, it has been removed.

Also fix some PUBLIC/PRIVATE mistake in target_include_directories to correctly pass the
properties to client code.
studiofuga added a commit to studiofuga/socket.io-client-cpp that referenced this issue Feb 25, 2022
@jmigual jmigual linked a pull request Mar 1, 2022 that will close this issue
jmigual pushed a commit to jmigual/socket.io-client-cpp that referenced this issue Mar 8, 2022
@jmigual
Copy link
Collaborator

jmigual commented Mar 9, 2022

Fixed by 9173946

@jmigual jmigual closed this as completed Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants