<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/122907/">https://git.reviewboard.kde.org/r/122907/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 11th, 2015, 7:03 p.m. UTC, <b>Andreas Pakulat</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;">No, thats the wrong fix. This requires that when building kdevplatform the top-level source dir is in the include path which is just wrong and it can break in cases where there are multiple vcs directories in the various include paths passed to the compiler via -I (and a vcsexport.h in both). The proper fix is to keep the "" so the header is searched first based on the path of the file including it and using a path that is relative to the file including it. So in vcs/models/foo.cpp use #include "../vcsexport.h" instead of #include "vcsexport.h".</p></pre>
</blockquote>
<p>On March 12th, 2015, 5:27 p.m. UTC, <b>Giorgos Tsiapaliokas</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 understand the problem that this patch creates, but I don't understand how to solve it.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So in vcs/models/foo.cpp use #include "../vcsexport.h" instead of #include "vcsexport.h".</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Do you mean in vcs/models/foo.h ?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I used #include "../vcsexport.h" in vcs/models/foo.[h, cpp] but the build fails.</p></pre>
</blockquote>
<p>On March 12th, 2015, 5:59 p.m. UTC, <b>Aleix Pol Gonzalez</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;">Maybe it will be just easier if we use the same directory structure both in-source and installed. i.e. installing models into vcs/models/</p></pre>
</blockquote>
<p>On March 12th, 2015, 6:39 p.m. UTC, <b>Andreas Pakulat</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;">Oh, wasn't aware of that. Yeah, keeping the directory structure is definetly needed for my proposed fix.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As for the build issue, is vcsexport.h in the builddir not the source dir? Thats the only way I could see the build to fail. Ah, yeah kf5 generates the header. So using "" with a relative path is not an easy fix either. That means there's no way to fix this properly. As far as I can see at least for the language library a <language/languageexport.h> is used already, so there's a precedent.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So I withdraw my objection, just use <vcs/vcsexport.h>. The clash with -I is relatively unlikely anyway since its generally not advisable to have system-provided kdevplatform headers along with a self-compiled version.</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;">+1.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There's no easy way around it (for the exact reasons you mention: a) project layout is different from the install layout, b) headers in the build dir).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So let's just use <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;"><mylib/mylibexport.h></code>.</p></pre>
<br />
<p>- Kevin</p>
<br />
<p>On March 11th, 2015, 4:30 p.m. UTC, Giorgos Tsiapaliokas 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 Giorgos Tsiapaliokas.</div>
<p style="color: grey;"><i>Updated March 11, 2015, 4:30 p.m.</i></p>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Replacing "vcsexport.h" with <vcs/vcsexport.h> allows other projects to use KDev::Vcs</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In another project if your try to use a header which isn't located in the root vcs directory
like $KF5/include/kdevplatform/vcs the build will fail, </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">because the files located in the subdirectories of $KF5/include/kdevplatform/vcs
will try to do #include "vcsexport.h" which it doesn't exist. For example
$KF5/include/kdevplatform/vcs/models/vcseventmodel.h so the build fails.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Instead <vcs/vcsexport.h> which expands to something like
$KF5/include/kdevplatform/vcs/vcsexport.h exists.</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;"><ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Try to build a project which is using KDev::Vcs, the build will fail</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">uninstall kdevplatform and kdevelop</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">make clean in kdevplatform and kdevelop</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">apply this patch</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">build kdevplatform, it builds normally</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">build kdevelop, it builds normally</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">build the other project, it builds normally</li>
</ol></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>vcs/interfaces/icontentawareversioncontrol.h <span style="color: grey">(c938468)</span></li>
<li>vcs/interfaces/ipatchdocument.h <span style="color: grey">(998c184)</span></li>
<li>vcs/interfaces/ipatchexporter.h <span style="color: grey">(1a9cce0)</span></li>
<li>vcs/interfaces/ipatchsource.h <span style="color: grey">(eaaf5a1)</span></li>
<li>vcs/models/vcsannotationmodel.h <span style="color: grey">(b034729)</span></li>
<li>vcs/models/vcseventmodel.h <span style="color: grey">(5ef69f0)</span></li>
<li>vcs/models/vcsfilechangesmodel.h <span style="color: grey">(3d87180)</span></li>
<li>vcs/models/vcsitemeventmodel.h <span style="color: grey">(19ee034)</span></li>
<li>vcs/widgets/standardvcslocationwidget.h <span style="color: grey">(0ad1844)</span></li>
<li>vcs/widgets/vcscommitdialog.h <span style="color: grey">(a1cf712)</span></li>
<li>vcs/widgets/vcsdiffpatchsources.h <span style="color: grey">(601e37a)</span></li>
<li>vcs/widgets/vcsdiffwidget.h <span style="color: grey">(c2c60fd)</span></li>
<li>vcs/widgets/vcseventwidget.h <span style="color: grey">(336d975)</span></li>
<li>vcs/widgets/vcsimportmetadatawidget.h <span style="color: grey">(4363df9)</span></li>
<li>vcs/widgets/vcslocationwidget.h <span style="color: grey">(d1cf35f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122907/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>