On 9/20/2010 1:39 PM, Eric Noulard wrote:
2010/9/20 Eric Noulard<[email protected]>:
2010/9/20 Bill Hoffman<[email protected]>:
Eric,
This commit is breaking windows installs:
commit 6a521f8604ee4e6a757109e731a36fdc5575f6c8
Author: Eric NOULARD<[email protected]>
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 << ";";
}
_______________________________________________
cmake-developers mailing list
[email protected]
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
