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

Broken comparison of boolean values in CMakeLists.txt #1101

Closed
mojca opened this Issue Jan 6, 2019 · 4 comments

Comments

Projects
None yet
3 participants
@mojca
Copy link

mojca commented Jan 6, 2019

The boolean options in CMake should not be compared with STREQUAL, but as boolean values instead, so that even -DUSE_LIBWRAP=yes would work, for example, see the CMake documentation.

For example, the following

if (${USE_LIBWRAP} STREQUAL ON)
	set (MOSQ_LIBS ${MOSQ_LIBS} wrap)
	add_definitions("-DWITH_WRAP")
endif (${USE_LIBWRAP} STREQUAL ON)

would probably better be written as

if (USE_LIBWRAP)
	set (MOSQ_LIBS ${MOSQ_LIBS} wrap)
	add_definitions("-DWITH_WRAP")
endif (USE_LIBWRAP)
@slewsys

This comment has been minimized.

Copy link

slewsys commented Jan 7, 2019

The problem is aggravated by the fact that the Build Dependencies section of Mosquitto's readme.md explicitly uses "yes" and "no" boolean values, contrary to the tests in CMakeLists.txt as per Mojca's comment.

@mojca

This comment has been minimized.

Copy link
Author

mojca commented Jan 8, 2019

Of course those examples are for the regular Makefiles, not for CMake. The only way to kind of clarify that would be to repeat the same options as they were supposed to be used in CMake. I admit that I was never aware of the validity of yes/no until @slewsys pointed it out.

@ralight ralight added this to the 1.5.6 milestone Jan 10, 2019

ralight added a commit that referenced this issue Jan 10, 2019

Fix comparison of boolean values in CMake build.
Closes #1101. Thanks to Mojca Miklavec and Andrew L. Moore.
@ralight

This comment has been minimized.

Copy link
Contributor

ralight commented Jan 10, 2019

Thanks for the report and example. It's also nice to have a change which brings a functional improvement and makes it more readable all at the same time.

Closed in 8fce261 , which will be part of 1.5.6 soon-ish.

@ralight ralight closed this Jan 10, 2019

@mojca

This comment has been minimized.

Copy link
Author

mojca commented Jan 10, 2019

Thank you very much for the super quick fix.

ralight added a commit that referenced this issue Feb 8, 2019

Fix comparison of boolean values in CMake build.
Closes #1101. Thanks to Mojca Miklavec and Andrew L. Moore.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment