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 -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
