<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="https://git.reviewboard.kde.org/r/115506/">https://git.reviewboard.kde.org/r/115506/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 6th, 2014, 9:35 p.m. UTC, <b>Morten Volden</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;">Okay, I've had a chance to look at bit closer on the patch.
The assumption that source directory is the parent dir of the build directory is, I'll admit, pretty lame. So I'm pretty pleased that something is being done about it.
However, looking at the patch a little more closely I'm not sure I'm too keen on having the output filtering strategy know about projects.
First of all I'm not sure that the patch will solve bug #321982 in all cases. The issue specifies that during compilation the following line occurs:
CMake Error at foo/CMakeLists.txt:435 (ADD_LIBRARY):
If multiple projects are open with a subpath named 'foo' wouldn't it be random which CMakeLists.txt the user is forwarded to?
I think I would prefer it if the project path is inferred at the time the job is created and then passed to the CompilerFilter. That way it would also be easier to test the feature.
</pre>
</blockquote>
<p>On February 6th, 2014, 9:51 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;">By the way, I made some attempts at the CMake devel list in order to fix the root issue - CMake not printing absolute paths - a while ago: http://public.kitware.com/pipermail/cmake/2013-July/055361.html. Unfortunately no one replied at that point. I'll try to get this discussion running again.
Regarding the code: It's rather unfortunate that we have to put so much logic into the filter strategies just make this work with CMake. :/</pre>
</blockquote>
<p>On February 7th, 2014, 9:37 p.m. UTC, <b>Todd Nowacki</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;">I'm going to take a look into passing down the source directory along with the build directory; hopefully it won't get too crazy with the number of changes needed.
But yes, having absolute paths would be super helpful.</pre>
</blockquote>
<p>On February 7th, 2014, 10:30 p.m. UTC, <b>Todd Nowacki</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;">Taking a look at the into the other solution, it seems really odd to put the sourceDir in all of this just for cmake. I would have to add a sourceDir field to CompilerFilterStrategy, OutputModel, and OutputExecuteJob (with a setter, getter, and property for OutputExecuteJob) just so it can be used once by the CMake project manager in kdevelop.
Perhaps we should just ignore the bug and wait for CMake to fix the absolute path error?</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;">That was my thought too, although I doubt they consider it an error. Maybe you can ping them about the issue.</pre>
<br />
<p>- Aleix</p>
<br />
<p>On February 5th, 2014, 10:37 p.m. UTC, Todd Nowacki wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 KDevelop.</div>
<div>By Todd Nowacki.</div>
<p style="color: grey;"><i>Updated Feb. 5, 2014, 10:37 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=321982">321982</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevplatform
</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;">Before, it was assumed that the build directory was an immediate child of the source directory.
Now, we iterate through each project (via the project controller) to find a project with a matching build directory and grab the source directory from that project. </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;">The search works for kdevelop and kdevplatform.
It should ideally be tested on a project where he build directory is not an immediate child of the source directory, but I did not have such a project at the time of posting this patch. </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>outputview/CMakeLists.txt <span style="color: grey">(b15d640f4486e863dd90caa8560f9c332cc66810)</span></li>
<li>outputview/outputfilteringstrategies.cpp <span style="color: grey">(f2e65e115644d04d1bb362d31d30d149a8071d00)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115506/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>