Review Request: Set RPATH only when not installed into system dir

[Rex Dieter]

> On Dec. 22, 2011, 6:39 p.m., Rex Dieter wrote:
> > Looking good, a few comments:
> > 
> > I'd recommend using 
> > if("${_isSystem}" LESS 0)
> > instead of a strict -1 match.  Matches other cmake code that does the same thing.
> > 
> > Speaking of other cmake code doing similar things... I've never seen them check against anything other than CMAKE_PLATFORM_IMPLICIT_LINK_DIRECTORIES, what's the purpose of adding CMAKE_CXX_IMPLICIT_LINK_DIRECTORIES here too?
> 
> Harald Sitter wrote:
>     it be unacceptable anyway as this part is stolen from the empire of kdelibs, so the fix needs to be done there and pulled in
> 
> Rex Dieter wrote:
>     I take my first comment back, kdelibs' does indeed use STREQUAL "-1" , ok then.
>     
>     Given that, I'd support simply stealing the kdelibs' code in FindKDE4Internal.cmake , (that matches CMAKE_PLATFORM_IMPLICIT_LINK_DIRECTORIES only).
> 
> Felix Geyer wrote:
>     The review request for kdelibs is at https://git.reviewboard.kde.org/r/103422/
> 
> Rex Dieter wrote:
>     Imo, even without the CMAKE_CXX_IMPLICIT_LINK_DIRECTORIES bits, that's better than the status quo, which essentially forces everyone (who care about superfluous rpath'ing) to patch this or use some other hammer to strip out the rpath'ing going on.

in short, don't let 'perfect' be the enemy of the good.


- Rex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103423/#review9177
-----------------------------------------------------------


On Dec. 15, 2011, 10:17 p.m., Felix Geyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103423/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2011, 10:17 p.m.)
> 
> 
> Review request for Phonon.
> 
> 
> Description
> -------
> 
> Set CMAKE_INSTALL_RPATH only when phonon isn't installed into a system lib dir.
> 
> 
> Diffs
> -----
> 
>   cmake/FindPhononInternal.cmake 2bad949 
> 
> Diff: http://git.reviewboard.kde.org/r/103423/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Felix Geyer
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-multimedia/attachments/20120208/66ea3938/attachment.htm>
-------------- next part --------------
_______________________________________________
kde-multimedia mailing list
kde-multimedia at kde.org
https://mail.kde.org/mailman/listinfo/kde-multimedia