Inform user of missing dependencies instead #33
Conversation
Instead of failing with a cryptic error message like ``` CMake Error at /tmp/Arduino-CMake-NG/cmake/Platform/System/LinuxDistDetector.cmake:12 (string): string no output variable specified Call Stack (most recent call first): /tmp/Arduino-CMake-NG/cmake/Platform/System/BuildSystemInitializer.cmake:25 (detect_host_linux_distribution) /tmp/Arduino-CMake-NG/cmake/Platform/Arduino.cmake:34 (initialize_build_system) /usr/share/cmake-3.10/Modules/CMakeSystemSpecificInformation.cmake:26 (include) CMakeLists.txt:2 (project) ``` let's fail with a clean message about missing dependencies :-)
There was a problem hiding this comment.
@ooxi Welcome to the project and thank you very much for this contribution!
You've noticed a very subtle bug and I really appreciate the effort to fix it yourself!
It looks good, I just have a few notes for you to look at in the review comments 😄
| find_program(lsb_release_exec lsb_release) | ||
|
|
||
| if ("lsb_release_exec-NOTFOUND" STREQUAL "${lsb_release_exec}") | ||
| message(FATAL_ERROR "`lsb_release' not found") |
There was a problem hiding this comment.
FATAL_ERROR fails the whole CMake process, which is something we don't really want to do here since the dist-detection module is not part of the framework's core, just a nice-to-have feature 😃
Instead, use WARNING to inform the user that it won't benefit from this module until it installs missing dependencies.
Besides, I think it'd be more informative if the message also stated that "Linux distribution couldn't be detected", then the exact reason why.
There was a problem hiding this comment.
Thanks for the fast feedback, I'll update my pull request!
2 participants
Instead of failing with a cryptic error message like
let's fail with a clean message about missing dependencies :-)