FindR.cmake needs some work
Alexander Neundorf
neundorf at kde.org
Tue Apr 27 21:13:38 CEST 2010
Hi Alex,
somebody reported a problem with FindR.cmake, so I had a look at it.
It has some issues and needs some work.
Can you please have a look at them ?
I'll happily provide guidance on how to fix them if you have questions.
So here we go (numbered so it's easier to refer to them):
1) The check IF(R_EXECUTABLE-NOTFOUND) is wrong.
I guess you want to check whether R_EXECUTABLE has been set to something
useful. To do this, do IF(R_EXECUTABLE).
2) A big part of the code below uses R_EXECUTABLE and is executed also if
R_EXECUTABLE has not been found. So all that code below should be put into a
IF(R_EXECUTABLE)
3) Is there a reason why you do
FIND_FILE(R_H
R.h
PATHS ${R_INCLUDEDIR}
NO_DEFAULT_PATH)
...
GET_FILENAME_COMPONENT(R_INCLUDEDIR ${R_H}
PATH)
instead of
FIND_PATH(R_INCLUDEDIR R.h PATHS ${R_INCLUDEDIR} NO_DEFAULT_PATH)
?
This should do the same and save code, i.e. make the file easier to read.
4) Why do you use the NO_DEFAULT_PATH as last argument ? Is this to make sure
that cmake first looks in R_INCLUDEDIR ?
A better way to do this is to use the "HINTS" keyword instead the "PATHS"
keyword. The directories listed after HINTS are searched before the default
directories, not after them. So then there should be no reason to use
NO_DEFAULT_PATH anymore.
5) The documentation at the top says that R_INCLUDE_DIR is provided by the
file, but it seems that instead R_INCLUDEDIR is set. This should be changed
so that indeed R_INCLUDE_DIR is provided.
6) R_USED_LIBS should be named R_LIBRARIES.
7) All the other library variables should be named "R_FOO_LIBRARY", e.g.
R_R_LIBRARY, R_LAPACK_LIBRARY etc.
8) All variables which hold the result of a find_file/path/library/program()
call should be marked as advanced at the bottom of the file (using
MARK_AS_ADVANCED() ) The only exception may be R_EXECUTABLE itself.
9) To check whether FindR.cmake was successful, you should use the macro
find_package_handle_standard_args, provided by
FindPackageHandleStandardArgs.cmake, something like
include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(R DEFAULT_MSG
R_EXECUTABLE R_INCLUDE_DIR R_R_LIBRARY)
Alex
More information about the Kde-buildsystem
mailing list