<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/113649/">http://git.reviewboard.kde.org/r/113649/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 5th, 2013, 4:53 p.m. UTC, <b>Alexander Neundorf</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">If LIB_INSTALL_DIR is a cache variable, i.e. if it can be set from the outside (I guess it can), you should test before whether it is an
absolute path or not, and make it absolute if it isn't:
set(_abs_LIB_INSTALL_DIR "${LIB_INSTALL_DIR}")
if (NOT IS_ABSOLUTE "${_abs_LIB_INSTALL_DIR}")
set(_abs_LIB_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/${LIB_INSTALL_DIR}")
endif()
</pre>
</blockquote>
<p>On November 5th, 2013, 5:02 p.m. UTC, <b>Rex Dieter</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">someone just commented in #kde-devel that even CMAKE_INSTALL_PREFIX can be relative? then what? test that for absolute-ness too?</pre>
</blockquote>
<p>On November 5th, 2013, 5:05 p.m. UTC, <b>Rex Dieter</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Looking at FindPhononInternal.cmake, I see a comment there that LIB_INSTALL_DIR is expected to be relative, so does checking if it isn't even make sense?</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I have never seen a relative CMAKE_INSTALL_PREFIX.
Probably it is possible to set it to a relative path, but I have never seen somebody doing that nor do I know why somebody should do that. I'd say, it is expected that CMAKE_INSTALL_PREFIX is absolute. So for now, I would ignore the case of a non-absolute CMAKE_INSTALL_PREFIX.
I just had a look at FindPhononInternal.
include(GNUInstallDirs)
...
set(LIB_INSTALL_DIR "${CMAKE_INSTALL_LIBDIR}" )
CMAKE_INSTALL_DIR comes from GNUInstallDirs.cmake:
if(NOT DEFINED CMAKE_INSTALL_LIBDIR)
...
set(CMAKE_INSTALL_LIBDIR "${_LIBDIR_DEFAULT}" CACHE PATH "object code libraries (${_LIBDIR_DEFAULT})")
endif()
So CMAKE_INSTALL_LIBDIR is a cache variable and can be edited by the user, and is not guaranteed to be an absolute path, but can be.
So, if I see things correctly, LIB_INSTALL_DIR can be both a relative or an absolute path and I would recommend to test for that.
</pre>
<br />
<p>- Alexander</p>
<br />
<p>On November 5th, 2013, 4:38 p.m. UTC, Rex Dieter wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Build System and Phonon.</div>
<div>By Rex Dieter.</div>
<p style="color: grey;"><i>Updated Nov. 5, 2013, 4:38 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
phonon
</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;">Prior to this patch, the cmake code queries/uses ${LIB_INSTALL_DIR} which is a relative path, so an unconditional (relative) rpath gets added.</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;">build/install phonon-4.7.0 to system path on fedora 20 without rpath.</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>cmake/FindPhononInternal.cmake <span style="color: grey">(b48bfaf)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113649/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>