onion
onion copied to clipboard
davidmoreno
Change the way CMakeLists.txt deals with optional dependencies
Right now, CMakeLists.txt will check if the user setted some dependency for TRUE (all dependencies are true by default), and if it is TRUE CMakeLists.txt will search the dependency and if it is not found will issue a warning.
My suggestion is, if the user specified that he wants some dependency, it is a fatal error not to find it. Also, if some dependencie's variable is unsetted, Cmake will search the dependency and if it is found the variable is default to true, if it is not found, the variable is default to false.
Hi,
I thought a bit about it, and maybe it's better to be more assertive and do not offer a 'maybe' option. If set to yes, and not found, give error, explaining both how to install missing dependency and how to disable manually that functionality.
This way we can solve the 'compiled on another system and everything looked alright' problems.
What do you think? On 3 Feb 2014 00:31, "André Puel" [email protected] wrote:
@davidmoreno https://github.com/davidmoreno Do you have any thoughs on this?
Reply to this email directly or view it on GitHubhttps://github.com/davidmoreno/onion/issues/55#issuecomment-33917029 .
https://github.com/Andrepuel/onion/commit/aff94dae31e91300fd5804c8ea9b1bb6b24b73fa
Note how cmake first searches for GNUTLS and only then defines ONION_USE_SSL for true or false. If the user had already defined it, the SET() does nothing. Also, if GNUTLS was not found but the variable was already setted to true, a SEND_ERROR is issued.
It only bothers me that the SET() is duplicated.
Hi,
somehow I prefer to have all the optionals at the beginning, so it is easier to know what can you tweak. Also as I commented now I think that might be a good idea to do not have optional dependencies, or at least some of them should be not optional in the sense of autodetected.
So the idea should be that if ONION_USE_SSL is set to True, but no gnutls is installed, fail. But not all the libraries have this level of required. For example cairo is only used in an example, so if not installed, no big problem.
The idea is that for the same flags all users should get the same library.
I will try to implement it ASAP.
Anyway about your patch, although it sparkled this line of work, I don't want to apply it to onion as it is now.
I would use True as default value.
Actually I would use True for all values.
The only problem that I see is that onion would be hard to compile for newcomers, which would have to install dependencies or choose the right flags for their system. The "auto-setup" approach would compile with what is available and set the flags according, so the flags still tell exactly which "type" of onion was compiled.
Yet I dont want you to think that I am insisting on my proposed approach, I am just showing my thoughs on this topic.
CMake files really needs revamp. At most of the places there are confusing statements when a library is missing or one haven't chosen a particular part to compile.
I agree with @Andrepuel that onion is hard to compile for new comers (developers mainly) as there are dependencies (such as GNUTLS, GCrypt etc.) that are not easily available on systems. But on the other hand not including them as well, can lead to certain features getting missed out.
If set to yes, and not found, give error, explaining both how to install missing dependency and how to disable manually that functionality.
I really like this idea but we need to be clear in messages.
Also these dependencies are being externally linked with their own names and not by the variables provided by CMake Modules which might not be extensible in future.
Certain places in the CMake files can be refactored or sub divided into different files to make them easily understandable to new developers.
