[cmake-developers] install bug on windows

[Eric Noulard]

2010/9/20 Bill Hoffman <bill.hoffman at kitware.com>:
> On 9/20/2010 1:39 PM, Eric Noulard wrote:
>>
>> 2010/9/20 Eric Noulard<eric.noulard at gmail.com>:
>>>
>>> 2010/9/20 Bill Hoffman<bill.hoffman at kitware.com>:
>>>>
>>>> Eric,
>>>>
>>>> This commit is breaking windows installs:
>>>>
>>>> commit 6a521f8604ee4e6a757109e731a36fdc5575f6c8
>>>> Author: Eric NOULARD<eric.noulard at gmail.com>
>>>> Date:   Mon Aug 23 17:38:33 2010 +0200
>>>>
>>>>    CPack   Enable better handling of absolute installed files
>>>>
>>>>    The idea of the patch is to let the install generator define
>>>>    CPACK_ABSOLUTE_INSTALL_FILES then when CMake is installing
>>>>    project he will concatenate the list of files and give
>>>>    it to specific CPack Generator by defining
>>>> CPACK_ABSOLUTE_INSTALL_FILES
>>>>    to be the list of ALL files that were installed using absolute
>>>> destination.
>>>>    An example of use has been applied to RPM generator which now
>>>>    tries to automatically build a relocatable package.
>>>>
>>
>> [...]
>>
>>>> Why do you convert the / to an output path?
>>>
>>> I do not remember ... :-(
>>>
>>> I'm looking into this just now, give half an hour and I'll come
>>> with some answer.
>>
>> Now I know: this is a plain mistake :-).
>>
>>>> 6a521f86 (Eric NOULARD       2010-08-23 17:38:33 +0200  71)    os<<
>>>>  dest<<
>>>> cmSystemTools::ConvertToOutputPath("/");
>>>> 6a521f86 (Eric NOULARD       2010-08-23 17:38:33 +0200  72)    if
>>>> (rename&&
>>>> *rename)
>>>>
>>>> All paths in the cmake_install file should be cmake paths as cmake is
>>>> going
>>>> to consume them.
>>>
>>> Off course, you are right.
>>
>> Considering this is plain mistake, the fix is obvious
>>
>> os<<  dest<<  "/";
>>
>> corresponding patch is attached.
>>
>> Would you like me to apply it to next or would you
>> prefer to do it yourself?
>>
>>>> All that said, I am not sure why no test failed from this ????
>>>
>>> May be because there is no test with absolutely installed file
>>> in the test suite?
>>>
>>> I'll have a look too.
>>
>> OK I think I know.
>> The provided example from Micha Renner is a little awkward
>> it uses "unix-like" absolute install path "/usr/local"
>> on a Windows hosts :-)
>>
>> May be it's supposed to work but I wonder if
>> it's a common usage pattern.
>
> The path he used does not really mater, it should have worked.

OK.

>>
>>  What do you think?
>
> Use the absolute path to the binary directory for the project, it should
> always be writable.

Off course, you are right again. I didn't thought of that one.

>  The test should not try to write into system areas even if it can...

Yeah sure, in fact this is the main issue with absolute file install.

> For example this change:
> INSTALL(FILES ${CMAKE_SOURCE_DIR}/CTest.c
>  DESTINATION ${T_BINARY_DIR}/usr/local)
>
>
> Still gives the same error:
>
>>CMake Warning (dev) at cmake_install.cmake:31 (list):
> 1>  Syntax error in cmake code at
> 1>    C:/Users/hoffman/Max/tm/src/b/cmake_install.cmake:32
> 1>  when parsing string
> 1>    C:/Users/hoffman/Max/tm/src/b/usr/local\CTest.c
> 1>  Invalid escape sequence \C
>
>
> So, if you could create a patch with a test that would be great.

I'll try that tomorrow night or wednesday night.
Unfortunately I am on tight schedule for my "usual" work
but I'll try to make it done by wednesday night.


-- 
Erk
Membre de l'April - « promouvoir et défendre le logiciel libre » -
http://www.april.org