<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/121544/">https://git.reviewboard.kde.org/r/121544/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/121544/diff/1/?file=333443#file333443line155" style="color: black; font-weight: bold; text-decoration: underline;">cmake/modules/TelepathyDefaults.cmake</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">155</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">set</span><span class="p">(</span><span class="s">LIB_SUFFIX</span> <span class="s2">""</span> <span class="s">CACHE</span> <span class="s">STRING</span> <span class="s2">"Define suffix of library directory name (32/64)"</span> <span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">155</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">set</span><span class="p">(</span><span class="s">LIB_DESTINATION</span> <span class="s2">"${CMAKE_INSTALL_FULL_LIBDIR}"</span> <span class="s">CACHE</span> <span class="s">PATH</span> <span class="s2">"The subdirectory where libraries will be installed (default is ${CMAKE_INSTALL_PREFIX}/lib${LIB_SUFFIX})"</span> <span class="s">FORCE</span><span class="p">)</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">156</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">set</span><span class="p">(</span><span class="s"><span class="hl">LIB</span>_INSTALL_DIR</span><span class="hl"> </span><span class="s2"><span class="hl">"lib${LIB_SUFFIX}"</span></span><span class="hl"> </span> <span class="s">CACHE</span> <span class="s">PATH</span> <span class="s2">"The subdirectory where <span class="hl">librari</span>es will be installed (default is ${CMAKE_INSTALL_PREFIX}/<span class="hl">lib${LIB_SUFFIX}</span>)"</span> <span class="s">FORCE</span><span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">156</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">set</span><span class="p">(</span><span class="s"><span class="hl">INCLUDE_DESTINATION</span></span><span class="hl"> </span><span class="s2"><span class="hl">"${CMAKE</span>_INSTALL_<span class="hl">FULL_INCLUDE</span>DIR<span class="hl">}"</span></span> <span class="s">CACHE</span> <span class="s">PATH</span> <span class="s2">"The subdirectory where <span class="hl">header fil</span>es will be installed (default is ${CMAKE_INSTALL_PREFIX}/<span class="hl">include</span>)"</span> <span class="s">FORCE</span><span class="p">)</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">157</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">set</span><span class="p">(</span><span class="s"><span class="hl">INCLUD</span>E_INSTALL_DIR</span> <span class="s2"><span class="hl">"include"</span></span><span class="hl"> </span> <span class="s">CACHE</span> <span class="s">PATH</span> <span class="s2">"The subdirectory where <span class="hl">header</span> files will be installed (default is ${CMAKE_INSTALL_PREFIX}/<span class="hl">include)"</span></span><span class="hl"> </span><span class="s"><span class="hl">FORCE</span></span><span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">157</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">set</span><span class="p">(</span><span class="s"><span class="hl">DATA_DESTINATION</span></span><span class="hl"> </span><span class="s2"><span class="hl">"${CMAK</span>E_INSTALL_<span class="hl">FULL_DATA</span>DIR<span class="hl">}/telepathy"</span></span> <span class="s">CACHE</span> <span class="s">PATH</span> <span class="s2">"The subdirectory where <span class="hl">data</span> files will be installed (default is ${CMAKE_INSTALL_PREFIX}/<span class="hl">share/telepathy)"</span></span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You should not offer these <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">XXX_DESTINATION</code> variables to the user. <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">CMAKE_INSTALL_XXXDIR</code> are already cached, modifying these variables will modify the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">DESTINATION</code> ones too.
In this way you will just get extra cmake cached variables in GUIs, and you won't be able to modify them, since they are forced every time.
You can either make them <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">INTERNAL</code> instead of <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">PATH</code> (<code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">FORCE</code> is implied in this case), or you can just set them as normal variables since you are including the file in the main <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">CMakeLists.txt</code> (therefore you don't have problems with variable scope), and since it does not make much sense to cache them.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also I think I remember that there is some reason to use the <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">relative</strong> path instead of <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">absolute</strong> when doing <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">install()</code> (therefore <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">CMAKE_INSTALL_XXXDIR</code> instead of <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">CMAKE_INSTALL_FULL_XXXDIR</code>), but I might be wrong here...</p></pre>
</div>
</div>
<br />
<p>- Daniele E. Domenichelli</p>
<br />
<p>On December 15th, 2014, 11:30 p.m. UTC, Hrvoje Senjan wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Telepathy and Aleix Pol Gonzalez.</div>
<div>By Hrvoje Senjan.</div>
<p style="color: grey;"><i>Updated Dec. 15, 2014, 11:30 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
telepathy-logger-qt
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">this is less fragile in abs. vs. relative paths, etc..</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">so far tried only cmake sets correct vars...</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(0022600)</span></li>
<li>TelepathyLoggerQt/CMakeLists.txt <span style="color: grey">(125b56b)</span></li>
<li>TelepathyLoggerQt/TelepathyLoggerQt.pc.in <span style="color: grey">(508fe25)</span></li>
<li>TelepathyLoggerQt/TelepathyLoggerQtConfig.cmake.in <span style="color: grey">(0a4eef9)</span></li>
<li>cmake/modules/TelepathyDefaults.cmake <span style="color: grey">(ec091e2)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121544/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>