<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><br><br>——————<br><br><blockquote type="cite">Hi Raffi,<br><br>Thanks for your continuing work on this module.<br><br>I've made some whitespace and quoting tweaks to the files. See attached<br>updated versions. I also renamed the test helper to not start in "Find"<br>since no one should call find_package(Matlab_TestsRedirect). See further<br>comments below.<br><br>On 07/04/2014 08:02 PM, Raffi Enficiaud wrote:<br><blockquote type="cite"><blockquote type="cite">Many of our tests use "cmake -P" to run a cmake script that performs<br>the test inside. You can use -Dvar=${val} before the -P option to<br>pass options to the script, such as $<TARGET_FILE_DIR:my_mex_target>.<br></blockquote><br>Done: this is the add_matlab_unit_test function. In fact, I think I can<br>remove the log files since they are redundant with the tests logs.<br></blockquote><br>I see no problem with that, but I'm not familiar with the tools.<br><br><blockquote type="cite">I added a function add_matlab_mex that is like a add_library<br></blockquote><br>Please make all APIs start in "matlab_”.<br></blockquote><br>Done<br><br><blockquote type="cite"><br>The documentation blocks for each command also need to start in "#.rst:"<br><br>#.rst:<br># .. command:: add_matlab_mex<br><br>or they will not be included in the processed documentation.<br></blockquote><br>Ok. I checked the documentation again, and I think it contains now all the necessary information, plus it is now visually more appealing. <br><br><br><blockquote type="cite"><br><blockquote type="cite">for creating MEX (compiled) extensions. There is an option to this<br>function called REDUCE_VISIBILITY<br></blockquote><br>I see that implemented here:<br><br><blockquote type="cite">if(HAS_VISIBILITY_INLINE_HIDDEN)<br> target_compile_options(${${prefix}_NAME} PRIVATE "-fvisibility-inlines-hidden")<br>endif()<br>if(HAS_VISIBILITY_HIDDEN)<br> target_compile_options(${${prefix}_NAME} PRIVATE "-fvisibility=hidden")<br>endif()<br></blockquote><br>An upstream version of the module should use the builtin features<br>for visibility settings:<br><br><br><a href="http://www.cmake.org/cmake/help/v3.0/prop_tgt/LANG_VISIBILITY_PRESET.html">http://www.cmake.org/cmake/help/v3.0/prop_tgt/LANG_VISIBILITY_PRESET.html</a><br><br><br><a href="http://www.cmake.org/cmake/help/v3.0/prop_tgt/VISIBILITY_INLINES_HIDDEN.html">http://www.cmake.org/cmake/help/v3.0/prop_tgt/VISIBILITY_INLINES_HIDDEN.html</a><br></blockquote><br>I am not sure how to do that, please have a look at my changes. It looks ok to me (generated compilation command include the visibility element) but maybe there is something better.<br><br><br><br><blockquote type="cite"><br><br><blockquote type="cite"># set(MATLAB_ADDITIONAL_VERSIONS<br># "release_name1" "corresponding_version1"<br># "release_name2" "corresponding_version2"<br># ...<br># )<br></blockquote><br>Rather than relying on matched pairs, perhaps use "=" to separate:<br><br># set(MATLAB_ADDITIONAL_VERSIONS<br># "release_name1=corresponding_version1"<br># "release_name2=corresponding_version2"<br><br>?<br></blockquote><br>Right, it is better.<br><br><br><blockquote type="cite"><br><blockquote type="cite">I am not sure how my implementation is compatible with the previous<br>FindMatlab package. The requirements are to define the same variables,<br>right? Is there anything else?<br></blockquote><br>It needs to produce the same result variables and also respond to<br>the old "input" variables (like find_ command cached results) that<br>the old module did. That way existing build scripts can continue<br>to work.<br></blockquote><br>This is something I should now check deeper. Is it an option to call it FindMatlab2 ?<br><br><br><br><blockquote type="cite"><br><blockquote type="cite">+# * ``MATLAB_USER_ROOT`` the root of the Matlab installation. This is given by the user.<br></blockquote><br>This should be documented in its own section of variables that the<br>user can set to control the search. See FindZLIB for example.<br><br><blockquote type="cite">+ # list the keys under HKEY_LOCAL_MACHINE\SOFTWARE\mathworks but the call to reg does not work<br>+ # from cmake, curiously, as is. The command provides the desired result under the command line though.<br>+ # Fix: this is because "/reg:64" should appended to the command, otherwise it gets on the 32 bits software<br></blockquote>key (curiously again)<br><br>This is because it gets the registry view of the calling CMake<br>unless you tell it what view to use.<br></blockquote><br>Ok<br><br><blockquote type="cite"><br><blockquote type="cite">+ find_program(MATLAB_REG_EXE_LOCATION "reg")<br>+ file(TO_NATIVE_PATH ${MATLAB_REG_EXE_LOCATION} MATLAB_REG_EXE_LOCATION)<br></blockquote><br>The second line should not be needed. The execute_process command<br>will take care of the path format.<br></blockquote><br>Done<br><br><blockquote type="cite"><br><blockquote type="cite">+ if((NOT DEFINED MATLAB_USER_ROOT) OR (NOT MATLAB_USER_ROOT))<br></blockquote><br>This can be shortened to<br><br>if(NOT MATLAB_USER_ROOT)<br></blockquote><br>Done<br><br><blockquote type="cite"><br><blockquote type="cite">+ if(NOT ${Matlab_PROGRAM})<br></blockquote><br>and this to<br><br>if(NOT Matlab_PROGRAM)<br></blockquote><br>Done<br><br><blockquote type="cite"><br><blockquote type="cite">BTW, is there any variable that tells the current cmake list,<br>even in function in packages?<br></blockquote><br>In the attached version I added<br><br>set(_FindMatlab_SELF_DIR "${CMAKE_CURRENT_LIST_DIR}")<br><br>to the top of the file to save the location of the file for re-use<br>inside function calls later.<br></blockquote><br>Good. <br><br><blockquote type="cite"><br>Thanks,<br>-Brad</blockquote></body></html>