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
Description: Binary data
Thanks,
Raffi
> On 18 Feb 2015, at 23:11, Raffi Enficiaud <raffi.enfici...@free.fr> 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 <brad.k...@kitware.com> 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
