<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/105863/">http://git.reviewboard.kde.org/r/105863/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 5th, 2012, 9:12 a.m. UTC, <b>David Faure</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/105863/diff/1/?file=76038#file76038line7" style="color: black; font-weight: bold; text-decoration: underline;">tier2/kconfig/CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">7</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">set</span><span class="p">(</span><span class="s">CMAKE_MODULE_PATH</span> <span class="o">${</span><span class="nv">ECM_MODULE_PATH</span><span class="o">}</span> <span class="s">cmake/</span><span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">set</span><span class="p">(</span><span class="s">CMAKE_MODULE_PATH</span><span class="hl"> </span><span class="o"><span class="hl">${</span></span><span class="nv"><span class="hl">CMAKE_MODULE_PATH</span></span><span class="o"><span class="hl">}</span></span> <span class="o">${</span><span class="nv">ECM_MODULE_PATH</span><span class="o">}</span> <span class="s">cmake/</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;">Ah, damn, there are conflicting goals here.
We removed ${CMAKE_MODULE_PATH} on purpose so that each framework (in this case, tier2/kconfig) builds standalone, i.e. doesn't use any cmake file from the rest of kdelibs. As long as we have independent frameworks being built together in one big kdelibs (which is a temporary situation), this is a way to ensure that each module is self-contained in terms of cmake files.
Now let's talk about FindKDEWin.cmake: can't we get rid of that? Try to think of kconfig as "a pure Qt-based library", like say, soprano, qca, or qjson. These libs don't use and don't need FindKDEWin.cmake, right? So why would kconfig need that additional layer? I know it was a necessary layer to get KDE4 code to compile on Windows, but the goal with KF5 is to remove layers and ensure that Qt and cmake have everything we need to compile our standalone libs on top of them.
Please evaluate what needs the "kdewin" (library, right?), and whether that code can't be ported to "pure Qt" instead.</pre>
</blockquote>
<p>On August 5th, 2012, 9:32 a.m. UTC, <b>Patrick Spendrin</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;">Yes, when I started with kf5 I already tried to put the easy parts into frameworks itself. Some stuff will be really hard, especially where "excessive" use of posix functions is used. Iirc kconfig was one of these, so it might be needed/wanted to keep the dependency for that part. One issue I wanted to address is that kdewin should ship a cmake Config.cmake so that we at least do not need the FindKDEWin.cmake anymore.
The question for KDE actually is how much code from kdewin can/should go into those libraries before it is better to keep kdewin.</pre>
</blockquote>
<p>On August 5th, 2012, 9:41 a.m. UTC, <b>David Faure</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 said "port to pure qt", not "duplicate kdewin code everywhere" :-)
I think the right approach is really: "hmm, wait, shouldn't Qt offer a way to do this, cross-platform"? If it does, port to that. If it doesn't, add it to Qt5 and submit it to gerrit. (Then we can look into what this means for the Qt4-based build: ifdef'ing out the code, i.e. breaking behavior with qt4, is a valid option).
But to discuss this further we need actual details of the "excessive use of posix functions" in kconfig.
A quick grep made me find only one, in KConfigIniBackend::isWritable() :
if (::access(QFile::encodeName(filePath).constData(), W_OK) == 0) {
I added some time ago a comment that says
// Qt 5 TODO: QFileInfo::canBeCreated or something.
But actually we can implement this one already on top of Qt, using QFileInfo::isWritable() on the parent directory.
(There's an issue if the parent dir doesn't exist yet, but that issue was there already in the ::access() call, AFAICS).</pre>
</blockquote>
<p>On August 5th, 2012, 3 p.m. UTC, <b>Alexander Neundorf</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 agree with David.
A possible alternative would be to put FindKDEWIN.cmake into e-c-m, and then use it from there.
</pre>
</blockquote>
<p>On August 6th, 2012, 11:20 a.m. UTC, <b>Patrick Spendrin</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 added a KDEWinConfig.cmake as a first step so that the FindKDEWin.cmake is not needed anymore.
I just looked into my local patches, and the question about having kdewin happens again in karchive, of course also in kdecore, kjs, kde4support and likely some others.</pre>
</blockquote>
<p>On May 6th, 2013, 11:33 a.m. UTC, <b>Kevin Ottens</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;">Is this discussion still relevant? If not this entry should probably be closed.</pre>
</blockquote>
<p>On May 6th, 2013, 7:23 p.m. UTC, <b>Alexander Neundorf</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;">Whether a FindKDEWin.cmake is used or whether kdewin installs a KDEWinConfig.cmake file does not change the fact that this means that these tier1-libs have a dependency additionally to Qt (... doesn't that make them tier2 ?).
</pre>
</blockquote>
<p>On May 6th, 2013, 8:25 p.m. UTC, <b>Kevin Ottens</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;">Yes it does. I in fact raises the question of the validity of that approach... We're basically trying to recreate UNIX syscalls on top of Windows where it's behavior differ. I doubt we need to have that much of low level code and should encourage using higher level constructs instead (probably from the lower parts of Qt itself).
I'd be interested in a list of the places where kdewin is used and why. I think most of those calls are just very old code not ported to more modern APIs. We should get rid of those instead of nurturing a kdewin IMO.</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;">Please discard this review request and help with porting KF5 away from unix syscalls instead :)
We made much progress in that direction, I believe, someone should try again on windows to see where it first breaks.</pre>
<br />
<p>- David</p>
<br />
<p>On August 4th, 2012, 9:11 p.m. UTC, Andrius da Costa Ribas 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 KDE Frameworks and Patrick Spendrin.</div>
<div>By Andrius da Costa Ribas.</div>
<p style="color: grey;"><i>Updated Aug. 4, 2012, 9:11 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;">Keep the paths already in CMAKE_MODULE_PATH when adding ECM_MODULE_PATH and other search-paths into it.</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;">Before this adjustment it was not possible to proceed with the build due to missing Find*.cmake (e.g.: FindKDEWin.cmake) files.</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>CMakeLists.txt <span style="color: grey">(f20069c)</span></li>
<li>tier2/kconfig/CMakeLists.txt <span style="color: grey">(c4b2cf6)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105863/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>