[cmake-developers] Introduction and volunteering for the Matlab package

Brad King brad.king at kitware.com
Thu Feb 19 08:53:22 EST 2015


On 02/19/2015 08:39 AM, Raffi Enficiaud wrote:
> Please find attached the merge of the two previous patches, rebased on 5dae6cf.

Applied, thanks:

 FindMatlab: Further revisions
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=1416d214

>> Those issues are related to the space in the test folder in the dashboard

Quoting of arguments in the cmake language:

 http://www.cmake.org/cmake/help/v3.2/manual/cmake-language.7.html#command-arguments

is not necessary to deal with spaces that are not literally written in
the argument.  Separation of unquoted arguments after variable evaluation
only happens on ";".  However, any unnecessary quoting you added also
won't hurt anything and makes it easier to read anyway.

Returning to a previous comment:

> On 02/18/2015 09:13 AM, Brad King wrote:
>> Why do you need so many different find_program calls for matlab?
>> There should be exactly one for Matlab_MAIN_PROGRAM

I still see four places that try to find "matlab", quoted below.
It looks like you're trying to make Matlab_MAIN_PROGRAM defined
if the MAIN_PROGRAM component was requested.

>       find_program(
>         _matlab_main_tmp
>         NAMES matlab)
> 
>       if(NOT _matlab_main_tmp)
>         execute_process(
>           COMMAND "which" matlab
>           OUTPUT_VARIABLE _matlab_main_tmp
>           ERROR_VARIABLE _matlab_main_tmp_err)
>         # the output should be cleaned up
>         string(STRIP "${_matlab_main_tmp}" _matlab_main_tmp)
>       endif()

If find_program doesn't find it, "which" won't have better luck.

>   if(Matlab_MAIN_PROGRAM)
>     set(_matlab_main_tmp ${Matlab_MAIN_PROGRAM})
>   else()
>     find_program(
>       _matlab_main_tmp
>       matlab
>       PATHS ${Matlab_ROOT_DIR} ${Matlab_ROOT_DIR}/bin
>       DOC "Matlab main program"
>       NO_DEFAULT_PATH
>     )
>   endif()

We should not risk finding the wrong matlab here.  See below.

>   # todo cleanup with code above
>   if(NOT DEFINED Matlab_MAIN_PROGRAM)
>     find_program(
>       Matlab_MAIN_PROGRAM
>       matlab
>       PATHS ${Matlab_ROOT_DIR} ${Matlab_ROOT_DIR}/bin
>       DOC "Matlab main program"
>       NO_DEFAULT_PATH
>     )
>   endif()

This looks like the component-specific search.  If we are to present
a component-specific result variable name then it can simply be
populated from the "one true" location found.

If "matlab" is needed to compute information for other components
then finding it should not be optional.  There should be a single

 find_program(Matlab_EXECUTABLE ...)

whose result is cached and re-used everywhere that "matlab" is
needed.  Separate symlink-resolving on it can be done when needed
but does not need to be cached.

Thanks,
-Brad



More information about the cmake-developers mailing list