-
Notifications
You must be signed in to change notification settings - Fork 252
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
Various tidy-ups (2) #581
Various tidy-ups (2) #581
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the tidy-up!
conan_provider.cmake
Outdated
@@ -130,7 +132,7 @@ endmacro() | |||
|
|||
|
|||
function(detect_lib_cxx OS LIB_CXX) | |||
if(${OS} STREQUAL "Android") | |||
if(OS STREQUAL "Android") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one here is tricky - with the function being invoked like this:
detect_lib_cxx(MYOS MYLIB_CXX)
the value of OS
inside this function is simply the string MYOS
- so removing the reference (${OS}), causes this to compare the string MYOS
which Android
, which obviously fails.
This is the only of the detect_*
functions that takes an input that is not the name of a variable to write to - so it makes it confusing.
I've refactored this to simply check in CMake for the current system name being Android, I think that's more in line with the rest of the file.
This PR supercedes #561 . It has been rebased on top of
develop2
, and excludes the debated variable name harmonization patch.