<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/123874/">https://git.reviewboard.kde.org/r/123874/</a>
</td>
</tr>
</table>
<br />
<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;">Like Thiago already suggested on kde-core-devel, this might need additional discussion in a different venue.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As for the diff itself, I am not sure hitting the compile flags with everything Qt got is a very good idea. I have no examples of when this would be undesirable but considering the position in the past was to opt-in for specific flags, as necessary, it seems a bit over the top to now simply reuse all the flags.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I did some research though and I think I found why it fails. Apparently cmake implements fPIC and fPIE mutually exclusive (at least as far as CMAKE_POSITION_INDEPENDENT_CODE goes) depending on the actual target type
https://github.com/Kitware/CMake/blob/e54d2fdf50d7e1170f9e3dcbcb62ebacc7205de6/Source/cmLocalGenerator.cxx#L2412</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">And that really makes the problem greater in scope because more things use that flag to enable PIC (http://lxr.kde.org/search?<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">filestring=&_string=CMAKE_POSITION_INDEPENDENT_CODE) and it is in fact the recommended way to handle this right now (http://doc.qt.io/qt-5/cmake-manual.html). So yeah, I think this should probably be figured out with Qt/CMake people how to best enable position independency so both PIC and PIE are enabled at the same time.
That being said, the more accruate temporary fix probably would be to manually add CMAKE</em>${lang}<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">COMPILE_OPTIONS_PIC and CMAKE</em>${lang}_COMPILE_OPTIONS_PIE to the flags manually (i.e. replicate what ::AddPositionIndependentFlags does but using both PIC and PIE).</p></pre>
<br />
<p>- Harald Sitter</p>
<br />
<p>On May 22nd, 2015, 4:45 a.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 Build System, Phonon and Harald Sitter.</div>
<div>By Hrvoje Senjan.</div>
<p style="color: grey;"><i>Updated May 22, 2015, 4:45 a.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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Or to be more accurate, with commit 3eca75de67b3fd2c890715b30c7899cebc096fe9
I am not convinced this is best solution, but requires least changes to current cmake code.</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;">Builds with 5.4.2 branch, or mentioned commit backported.
Otherwise check_qt_visibility fails:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">[ 126s] -- Performing Test __KDE_HAVE_GCC_VISIBILITY - Success
[ 126s] Change Dir: /home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeFiles/CMakeTmp
[ 126s]
[ 126s] Run Build Command:"/usr/bin/gmake" "cmTryCompileExec3719375587/fast"
[ 126s] /usr/bin/gmake -f CMakeFiles/cmTryCompileExec3719375587.dir/build.make CMakeFiles/cmTryCompileExec3719375587.dir/build
[ 126s] gmake[1]: Entering directory '/home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeFiles/CMakeTmp'
[ 126s] /usr/bin/cmake -E cmake_progress_report /home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeFiles/CMakeTmp/CMakeFiles 1
[ 126s] Building CXX object CMakeFiles/cmTryCompileExec3719375587.dir/check_qt_visibility.cpp.o
[ 126s] /usr/bin/c++ -fmessage-length=0 -grecord-gcc-switches -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables -fasynchronous-unwind-tables -g -DNDEBUG -Wnon-virtual-dtor -Wno-long-long -Wundef -Wcast-align -Wchar-subscripts -Wall -W -Wpointer-arith -Wformat-security -fno-exceptions -DQT_NO_EXCEPTIONS -fno-check-new -fno-common -Woverloaded-virtual -fvisibility=hidden -fPIE -I/usr/include/qt5 -I/usr/include/qt5/QtCore -I/usr/lib64/qt5/mkspecs/linux-g++ -I/usr/include/qt5/QtWidgets -I/usr/include/qt5/QtGui -o CMakeFiles/cmTryCompileExec3719375587.dir/check_qt_visibility.cpp.o -c /home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeTmp/check_qt_visibility.cpp
[ 126s] In file included from /usr/include/qt5/QtCore/QtGlobal:1:0,
[ 126s] from /home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeTmp/check_qt_visibility.cpp:1:
[ 126s] /usr/include/qt5/QtCore/qglobal.h:1051:4: error: #error "You must build your code with position independent code if Qt was built with -reduce-relocations. " "Compile your code with -fPIC (-fPIE is not enough)."
[ 126s] # error "You must build your code with position independent code if Qt was built with -reduce-relocations. "\
[ 126s] ^
[ 126s] CMakeFiles/cmTryCompileExec3719375587.dir/build.make:57: recipe for target 'CMakeFiles/cmTryCompileExec3719375587.dir/check_qt_visibility.cpp.o' failed
[ 126s] gmake[1]: Leaving directory '/home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeFiles/CMakeTmp'
[ 126s] gmake[1]: *** [CMakeFiles/cmTryCompileExec3719375587.dir/check_qt_visibility.cpp.o] Error 1
[ 126s] Makefile:117: recipe for target 'cmTryCompileExec3719375587/fast' failed
[ 126s] gmake: *** [cmTryCompileExec3719375587/fast] Error 2
[ 126s]
[ 126s] CMake Error at cmake/FindPhononInternal.cmake:416 (message):
[ 126s] Qt compiled without support for -fvisibility=hidden. This will break
[ 126s] plugins and linking of some applications. Please fix your Qt installation
[ 126s] (try passing --reduce-exports to configure).
[ 126s] Call Stack (most recent call first):
[ 126s] CMakeLists.txt:47 (include)
</pre></div>
</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>cmake/FindPhononInternal.cmake <span style="color: grey">(44862b5)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123874/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>