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

[Brad King]

Hi Raffi,

Thanks for your continuing work on this module.

I've made some whitespace and quoting tweaks to the files.  See attached
updated versions.  I also renamed the test helper to not start in "Find"
since no one should call find_package(Matlab_TestsRedirect).  See further
comments below.

On 07/04/2014 08:02 PM, Raffi Enficiaud wrote:
>> Many of our tests use "cmake -P" to run a cmake script that performs
>> the test inside.  You can use -Dvar=${val} before the -P option to
>> pass options to the script, such as $<TARGET_FILE_DIR:my_mex_target>.
> 
> Done: this is the add_matlab_unit_test function. In fact, I think I can
> remove the log files since they are redundant with the tests logs.

I see no problem with that, but I'm not familiar with the tools.

> I added a function add_matlab_mex that is like a add_library

Please make all APIs start in "matlab_".

The documentation blocks for each command also need to start in "#.rst:"

 #.rst:
 # .. command:: add_matlab_mex

or they will not be included in the processed documentation.

> for creating MEX (compiled) extensions. There is an option to this
> function called REDUCE_VISIBILITY

I see that implemented here:

>   if(HAS_VISIBILITY_INLINE_HIDDEN)
>     target_compile_options(${${prefix}_NAME} PRIVATE "-fvisibility-inlines-hidden")
>   endif()
>   if(HAS_VISIBILITY_HIDDEN)
>     target_compile_options(${${prefix}_NAME} PRIVATE "-fvisibility=hidden")
>   endif()

An upstream version of the module should use the builtin features
for visibility settings:

 http://www.cmake.org/cmake/help/v3.0/prop_tgt/LANG_VISIBILITY_PRESET.html
 http://www.cmake.org/cmake/help/v3.0/prop_tgt/VISIBILITY_INLINES_HIDDEN.html

> #    set(MATLAB_ADDITIONAL_VERSIONS
> #       "release_name1" "corresponding_version1"
> #       "release_name2" "corresponding_version2"
> #       ...
> #       )

Rather than relying on matched pairs, perhaps use "=" to separate:

#    set(MATLAB_ADDITIONAL_VERSIONS
#       "release_name1=corresponding_version1"
#       "release_name2=corresponding_version2"

?

> I am not sure how my implementation is compatible with the previous
> FindMatlab package. The requirements are to define the same variables,
> right? Is there anything else?

It needs to produce the same result variables and also respond to
the old "input" variables (like find_ command cached results) that
the old module did.  That way existing build scripts can continue
to work.

> +# * ``MATLAB_USER_ROOT`` the root of the Matlab installation. This is given by the user.

This should be documented in its own section of variables that the
user can set to control the search.  See FindZLIB for example.

> +  # 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 it gets the registry view of the calling CMake
unless you tell it what view to use.

> +  find_program(MATLAB_REG_EXE_LOCATION "reg")
> +  file(TO_NATIVE_PATH ${MATLAB_REG_EXE_LOCATION} MATLAB_REG_EXE_LOCATION)

The second line should not be needed.  The execute_process command
will take care of the path format.

> +  if((NOT DEFINED MATLAB_USER_ROOT) OR (NOT MATLAB_USER_ROOT))

This can be shortened to

 if(NOT MATLAB_USER_ROOT)

> +    if(NOT ${Matlab_PROGRAM})

and this to

 if(NOT Matlab_PROGRAM)

> BTW, is there any variable that tells the current cmake list,
> even in function in packages?

In the attached version I added

 set(_FindMatlab_SELF_DIR "${CMAKE_CURRENT_LIST_DIR}")

to the top of the file to save the location of the file for re-use
inside function calls later.

Thanks,
-Brad

-------------- next part --------------
A non-text attachment was scrubbed...
Name: MatlabTestsRedirect.cmake
Type: text/x-cmake
Size: 1739 bytes
Desc: not available
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20140707/b4e4fad8/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FindMatlab.cmake
Type: text/x-cmake
Size: 36956 bytes
Desc: not available
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20140707/b4e4fad8/attachment-0005.bin>