On 02/17/2015 07:28 PM, Raffi Enficiaud wrote:
> The tests were failing because of the following modification:
> 
> -      matlab_get_version_from_matlab_run(${Matlab_MAIN_PROGRAM} 
> matlab_list_of_all_versions)
> +      matlab_get_version_from_matlab_run("${Matlab_MAIN_PROGRAM}" 
> matlab_list_of_all_versions)
> 
> Apparently the quotes here are interpreted as part of the binary name,
> which prevents the proper call to matlab using the execute_process function.

That should not be possible.  The quotes are needed in case the variable
value is an empty string.  They will not be treated as part of the value
passed to the function argument.

> I kept the symlink resolution, but I narrowed the case those should be
> resolved. I added a variable pointing to the (symlink resolved) location
> of the binary from which the version is retrieved.

Yes, thanks.

I squashed the changes into 9d414ca2 and rebased again.  Everything
so far is now in:

 FindMatlab: Rewrite module and provide a usage API
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=5dae6cfc

and merged to 'next' for testing.  Please base fixes for the below
on that.

More comments:

Why do you need so many different find_program calls for matlab?
There should be exactly one for Matlab_MAIN_PROGRAM, and it does
not need to be guarded by if(NOT Matlab_MAIN_PROGRAM) because
find_program does that already.  Any symlink resolution can be
done on the result.

The get_filename_component(PROGRAM) mode is intended to take a
command line string and figure out which leading piece is an
existing program in case it is an unquoted path with spaces.
While it may be a bug that this can return a directory, there
should be no use case for this functionality in FindMatlab.

>   # list the keys under HKEY_LOCAL_MACHINE\SOFTWARE\mathworks but the call to
>   # reg does not work from cmake, curiously, as is. The command provides the
>   # desired result under the command line though.
>   # Fix: this is because "/reg:64" should appended to the command, otherwise
>   # it gets on the 32 bits software key (curiously again)

This is because the default registry view depends on which "reg"
tool gets executed.  These comments do not belong in the final
version of the module.

>   find_program(MATLAB_REG_EXE_LOCATION "reg")

Many projects just execute_process() the "reg" tool directly
without finding it first.  It is reliably available on Windows.

>   execute_process(
>     COMMAND ${matlab_binary_program} -nosplash -nojvm 
> ${_matlab_additional_commands} -logfile 
> ${_matlab_temporary_folder}/matlabVersionLog.cmaketmp -nodesktop -nodisplay 
> -r "version, exit"
>     OUTPUT_VARIABLE _matlab_version_from_cmd_dummy
>     RESULT_VARIABLE _matlab_result_version_call
>     TIMEOUT 30
>     )

This should quote "${matlab_binary_program}" in case it is
empty for some reason.  Also you should capture the stderr
output with ERROR_VARIABLE so it does not leak to the output
of the CMake configuration process.

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

Reply via email to