<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/111661/">http://git.reviewboard.kde.org/r/111661/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 23rd, 2013, 7:02 p.m. UTC, <b>Alexander Neundorf</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="http://git.reviewboard.kde.org/r/111661/diff/1/?file=173101#file173101line239" style="color: black; font-weight: bold; text-decoration: underline;">kde-modules/KDECompilerSettings.cmake</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">238</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">if</span><span class="p">(</span><span class="s2">"${CMAKE_CXX_COMPILER_ID}"</span> <span class="s">STREQUAL</span> <span class="s2">"GNU"</span> <span class="s">OR</span> <span class="s2">"${CMAKE_CXX_COMPILER_ID}"</span> <span class="s">STREQUAL</span> <span class="s2">"Clang"</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Is this patch complete ?
This part looks wrong.
This is for the lines where the version is checked, right ?
I guess there should be a check in line 219 to accept also Clang, and in line 221 to accept only GNU (for the version check).
I also don't see why you need the endif() and opening if() in line 235.</pre>
</blockquote>
<p>On July 23rd, 2013, 10:45 p.m. UTC, <b>Milian Wolff</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;">Well, both achieve the same result - i.e. keep the version check GNU only. I just splitted the conditionals, into a GNU specific part and a shared GNU/Clang part. So which one do you prefer, I really don't care.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">so, do you want it changed, or is it OK like it is now?</pre>
<br />
<p>- Milian</p>
<br />
<p>On July 23rd, 2013, 6:53 p.m. UTC, Milian Wolff 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, Andreas Pakulat and Alexander Neundorf.</div>
<div>By Milian Wolff.</div>
<p style="color: grey;"><i>Updated July 23, 2013, 6:53 p.m.</i></p>
<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;">Add support for Clang in KDECompilerSettings.cmake.
This more or less copies what was done for KDE4 in https://git.reviewboard.kde.org/r/111612/ .
</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;">With the changes applied I successfully installed a hello world application. The linker and compiler command lines seem to be correct and include all extended features.
I also checked and debugfull also works as expected (-g3 is added).
milian@minime:~/projects/kde4/test-ecm/build$ cmake ..
-- The C compiler identification is Clang 3.3.0
-- The CXX compiler identification is Clang 3.3.0
-- Check for working C compiler: /usr/bin/ccache
-- Check for working C compiler: /usr/bin/ccache -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/ccache
-- Check for working CXX compiler: /usr/bin/ccache -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Performing Test __KDE_HAVE_W_OVERLOADED_VIRTUAL
-- Performing Test __KDE_HAVE_W_OVERLOADED_VIRTUAL - Success
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY - Success
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY - Success
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Success
-- Configuring done
-- Generating done
-- Build files have been written to: /home/milian/projects/kde4/test-ecm/build
milian@minime:~/projects/kde4/test-ecm/build$ make VERBOSE=1
makeobj[0]: Entering directory `/home/milian/projects/kde4/test-ecm/build'
/usr/bin/cmake -H/home/milian/projects/kde4/test-ecm -B/home/milian/projects/kde4/test-ecm/build --check-build-system CMakeFiles/Makefile.cmake 0
/usr/bin/cmake -E cmake_progress_start /home/milian/projects/kde4/test-ecm/build/CMakeFiles /home/milian/projects/kde4/test-ecm/build/CMakeFiles/progress.marks
/usr/bin/make -f CMakeFiles/Makefile2 all
make[1]: Entering directory `/home/milian/projects/kde4/test-ecm/build'
/usr/bin/make -f CMakeFiles/hello.dir/build.make CMakeFiles/hello.dir/depend
make[2]: Entering directory `/home/milian/projects/kde4/test-ecm/build'
cd /home/milian/projects/kde4/test-ecm/build && /usr/bin/cmake -E cmake_depends "Unix Makefiles" /home/milian/projects/kde4/test-ecm /home/milian/projects/kde4/test-ecm /home/milian/projects/kde4/test-ecm/build /home/milian/projects/kde4/test-ecm/build /home/milian/projects/kde4/test-ecm/build/CMakeFiles/hello.dir/DependInfo.cmake --color=
Dependee "/home/milian/projects/kde4/test-ecm/build/CMakeFiles/hello.dir/DependInfo.cmake" is newer than depender "/home/milian/projects/kde4/test-ecm/build/CMakeFiles/hello.dir/depend.internal".
Dependee "/home/milian/projects/kde4/test-ecm/build/CMakeFiles/CMakeDirectoryInformation.cmake" is newer than depender "/home/milian/projects/kde4/test-ecm/build/CMakeFiles/hello.dir/depend.internal".
Scanning dependencies of target hello
make[2]: Leaving directory `/home/milian/projects/kde4/test-ecm/build'
/usr/bin/make -f CMakeFiles/hello.dir/build.make CMakeFiles/hello.dir/build
make[2]: Entering directory `/home/milian/projects/kde4/test-ecm/build'
/usr/bin/cmake -E cmake_progress_report /home/milian/projects/kde4/test-ecm/build/CMakeFiles 1
[100%] Building CXX object CMakeFiles/hello.dir/main.cpp.o
/usr/bin/ccache /usr/bin/clang++ -Qunused-arguments -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_FROM_BYTEARRAY -DQT_NO_CAST_TO_ASCII -DQT_NO_SIGNALS_SLOTS_KEYWORDS -DQT_NO_URL_CAST_FROM_STRING -DQT_STRICT_ITERATORS -DQT_USE_FAST_CONCATENATION -DQT_USE_FAST_OPERATOR_PLUS -D_BSD_SOURCE -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -D_XOPEN_SOURCE=500 -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 -Werror=return-type -fvisibility=hidden -fvisibility-inlines-hidden -O2 -g -DNDEBUG -DQT_NO_DEBUG -o CMakeFiles/hello.dir/main.cpp.o -c /home/milian/projects/kde4/test-ecm/main.cpp
Linking CXX executable hello
/usr/bin/cmake -E cmake_link_script CMakeFiles/hello.dir/link.txt --verbose=1
/usr/bin/ccache /usr/bin/clang++ -Qunused-arguments -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 -Werror=return-type -fvisibility=hidden -fvisibility-inlines-hidden -O2 -g -DNDEBUG -DQT_NO_DEBUG -Wl,--enable-new-dtags CMakeFiles/hello.dir/main.cpp.o -o hello -rdynamic
make[2]: Leaving directory `/home/milian/projects/kde4/test-ecm/build'
/usr/bin/cmake -E cmake_progress_report /home/milian/projects/kde4/test-ecm/build/CMakeFiles 1
[100%] Built target hello
make[1]: Leaving directory `/home/milian/projects/kde4/test-ecm/build'
/usr/bin/cmake -E cmake_progress_start /home/milian/projects/kde4/test-ecm/build/CMakeFiles 0
makeobj[0]: Leaving directory `/home/milian/projects/kde4/test-ecm/build'</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>kde-modules/KDECompilerSettings.cmake <span style="color: grey">(d72bd9e)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111661/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>