Please find attached the merge of the two previous patches, rebased on 5dae6cf.

Thanks,
Raffi Enficiaud

Attachment: 0001-Adding-R2014a.patch
Description: Binary data


> On 19 Feb 2015, at 12:49, Raffi Enficiaud <[email protected]> wrote:
> 
> Dear Brad,
> 
> Apparently there are some issues when things are running with the dashboard, 
> which I did not observed yesterday.
> Those issues are related to the space in the test folder in the dashboard, 
> which I did not see on my local computer.
> 
> The attached patch (based on 5dae6cf) should solve those issues (tested only 
> in the dashboard folder of the ubuntu version). 
> 
> <0001-Fixing-problems-related-to-spaces-in-directory-names.patch>
> 
> Thanks,
> Raffi
> 
> 
> 
>> On 18 Feb 2015, at 23:11, Raffi Enficiaud <[email protected]> wrote:
>> 
>> Please find attached the patch addressing the issues + some others, rebased 
>> against 5dae6cf. 
>> I tested it on the 3 target platforms.
>> 
>> <patch.diff>
>> 
>>> On 18 Feb 2015, at 15:13, Brad King <[email protected]> wrote:
>>> 
>>> 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 restored the quotes. Maybe I experienced a caching issue: I run ctest with 
>> "FindMatlab" regex, and from time to time the cache is messed while I am 
>> working and I do not clean the folders systematically. 
>> 
>> 
>>> 
>>>> 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.
>> 
>> Couple of questions:
>> - does the script of the dashboard clean the folders? Or I have to do that 
>> manually? (cf caching issues above)
>> - is it the "next" branch that is tested on the "nightly" section of the 
>> dashboard? 
>> 
>>> 
>>> 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.
>> 
>> I wanted to separate the parts in some kind of modules.
>> 
>> - The first part is supposed to output the Matlab_ROOT and nothing else, and 
>> the other parts are relying on that. Finding a matlab_program is an 
>> implementation "detail", which is not cross platform. Yet, the method is 
>> kind of robust to find a proper installation ROOT. 
>> 
>> - The second part deals with the version, in case we have no other way than 
>> from asking Matlab. Since at this point, we have a ROOT, either given by the 
>> user or from the first part, we search for the matlab program using this 
>> information alone. In case the user gave the ROOT but not the version, we 
>> still have to find the program under this ROOT. In case the user gave 
>> nothing, we have to find the ROOT and the version, the former maybe implying 
>> a matlab_program search. Again, I think this is an implementation detail 
>> that the second part should not rely on.
>> 
>> - The third part is the user facing matlab_program, that we find on demand.
>> 
>> I agree this can be "optimized" in terms of find_program calls, but I would 
>> like to keep this structure for finding in the appropriate sequence all the 
>> information needed by the module. 
>> 
>> The symlink resolutions are made on the appropriate places now.
>> 
>>> 
>>> 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.
>> 
>> I did not understood that from the documentation ("the program in filename 
>> will be found in the system search path"): I thought it was another way of 
>> finding programs. I removed the corresponding lines.
>> 
>> 
>>> 
>>>> # 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.
>> 
>> Yes, we exchanged on this point already. I removed the comments. Basically, 
>> at some point I thought it would have been useful to use cmake as a make 
>> that can run matlab commands through the matlab_program (and not obliged to 
>> link anything to it). This is not possible in the current state of the 
>> module, but would be possible readily in the future. BTW, I volunteered for 
>> the maintenance of the module, so I guess these would be future extensions.
>> 
>> 
>>> 
>>>> 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.
>> 
>> Ok, I made the change. I thought it would be the "proper" method to find 
>> things first. 
>> 
>>> 
>>>> 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.
>> 
>> Done (+ all other calls).
>> 
>> 
>> -- 
>> 
>> 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
> 
> -- 
> 
> 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

-- 

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