<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/125276/">https://git.reviewboard.kde.org/r/125276/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 17th, 2015, 12:11 p.m. UTC, <b>Kevin Funk</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm opposing. On quite a few distros (I know Debian/Ubuntu for sure), LLVMConfig.cmake files are not usable (missing CMake includes, etc.). I think the <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">upcoming</em> Ubuntu release finally(!) has this fixed, so we can use them (this corresponds to LLVM 3.8)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That's also why I did not drop my custom FindLLVM.cmake script until now. I'd suggest using the the original version first, and only when the lookup fails: fallback to CONFIG-based search.</p></pre>
</blockquote>
<p>On September 17th, 2015, 12:12 p.m. UTC, <b>Kevin Funk</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Feel free to make sure FindLLVM.cmake sets the same/similar variables like LLVMConfig.cmake in order to make CMakeLists.txt as simple as possible</p></pre>
</blockquote>
<p>On September 17th, 2015, 12:36 p.m. UTC, <b>Alex Richardson</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For my convert2qt5signalslot project I use a modified version of your FindClang.cmake that works fine with distro provided LLVMConfig.cmake: https://github.com/a-richardson/convert2qt5signalslot/blob/master/cmake/FindClang.cmake</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It works with openSUSE 13.2 which has LLVM 3.5 and Tumbleweed which currently has 3.6.1: https://build.opensuse.org/package/show/home:a_richardson/convert2qt5signalslot</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">However, that could be because openSUSE applies patches to the LLVMConfig.cmake so that it works correctly. If that is the case how about changing the variable names to match the ones in LLVMConfig.cmake and adding something like: </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%">if (Clang_FIND_VERSION VERSION_GREATER 3.7)
message(FATAL_ERROR "LLVMConfig.cmake is usable now -> remove FindLLVM.cmake. See https://git.reviewboard.kde.org/r/125276/")
endif()
</pre></div>
</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Any update on this? Can we discard this?</p></pre>
<br />
<p>- Milian</p>
<br />
<p>On September 17th, 2015, 12:04 p.m. UTC, Alex Richardson 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 KDevelop.</div>
<div>By Alex Richardson.</div>
<p style="color: grey;"><i>Updated Sept. 17, 2015, 12:04 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdev-clang
</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;">Use the upstream LLVMConfig.cmake instead of FindLLVM.cmake</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This has existed at least since 3.5 so we can make use of it</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;">Works for me with 3.7</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/FindClang.cmake <span style="color: grey">(6c9bd6ef0242319122dcc7e6fd6cea8d9f0cbfbb)</span></li>
<li>cmake/FindLLVM.cmake <span style="color: grey">(4441779f4baac17895914937da245da0d480d755)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/125276/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>