[cmake-developers] install bug on windows

Bill Hoffman bill.hoffman at kitware.com
Mon Sep 20 16:20:43 EDT 2010


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 bare reality is twofold:
>
>    a) I do not use windows platform myself so I rely
>        on dashboard for regression.
>
>    b) I did not add a [cross platform] test for this change
>
> I shall try to write one test for this, but this may not be easy
> to write a cross-platform one because I'll have to find a "writable"
> "absolute destination" for each platform.
>
> On Unix platforms "/tmp" should be ok
> On WIndows I really don't know.
>
>   What do you think?

Seems to be covered:
(but I am guessing the install is not actually being called in that case...)

http://www.cdash.org/CDash/viewCoverageFile.php?buildid=726294&fileid=11009794
  TF       63    if (cmSystemTools::FileIsFullPath(dest.c_str()))
    54      |          64       {
    55      |          65       os << "list(APPEND 
CPACK_ABSOLUTE_DESTINATION_FILES\n";
    56      |          66       os << indent << " \"";
    57      |          67 
for(std::vector<std::string>::const_iterator fi = files.begin
    58      | TF       68                 fi != files.end(); ++fi)
    59      |          69               {
    60      | -->F     70                 if (fi!=files.begin()) os << ";";
    61      |          71                 os << dest << 
cmSystemTools::ConvertToOutputPath("/
    62      | -->F     72a                if (
    63      |   -->t   72b                    rename &&
    64      |   -->f   72c                              *rename)
    65      |          73                   {
    66      |          74                   os << rename;
    67      |          75                   }
    68      |   ...
    69      |          81       os << "\")\n";
    70      |          82       }


BTW, this type of code is not allowed in CMake:

    if (fi!=files.begin()) os << ";";

Needs to be:

if((fi!=files.begin())
{
os << ";";
}





More information about the cmake-developers mailing list