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