<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/122279/">https://git.reviewboard.kde.org/r/122279/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 27th, 2015, 7:18 p.m. CET, <b>Yichao Yu</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've just pushed a slightly modified version. Can you check if it solves you problem.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">NOTE: one issue with include X11 header in Qt project is that it might interfere with some qt headers (in a way that I don't understand and don't want to figure out), which is what <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">fixx11h.h</code> is for. Also, please don't use any Qt macro in the lib folder unless it is only meant to be use for Qt (and there's no way around that), in which case, it should be wrapped qt detection macros.</p></pre>
</blockquote>
<p>On January 27th, 2015, 7:47 p.m. CET, <b>Marko Käning</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;">Yichao, the current state of qtcurve again builds fine on my OSX/CI system.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks to you and René for fixing this!!</p></pre>
</blockquote>
<p>On January 27th, 2015, 9:10 p.m. CET, <b>René J.V. Bertin</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;">Works for me too, in all test cases cited above.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That means that this RR can be discarded, right?</p></pre>
</blockquote>
<p>On January 27th, 2015, 9:13 p.m. CET, <b>Marko Käning</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;">Yes, I suppose. :)</p></pre>
</blockquote>
<p>On January 27th, 2015, 9:19 p.m. CET, <b>Marko Käning</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;">Wait, it still fails on Linux/CI: http://build.kde.org/view/FAILED/job/qtcurve_master/28/console</p></pre>
</blockquote>
<p>On January 27th, 2015, 9:33 p.m. CET, <b>René J.V. Bertin</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;">Is that related to https://git.reviewboard.kde.org/r/121390/diff/# ?</p></pre>
</blockquote>
<p>On January 27th, 2015, 10:20 p.m. CET, <b>Yichao Yu</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;">@Rene No it shouldn't.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">From the output it looks like come kde(4) workspace headers are missing, e.g.</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%">yuyichao<span style="color: #666666">%</span> LANG<span style="color: #666666">=</span>C yaourt <span style="color: #666666">-</span>Qo <span style="color: #666666">/</span>usr<span style="color: #666666">/</span>include<span style="color: #666666">/</span>kcommondecoration.h
<span style="color: #666666">/</span>usr<span style="color: #666666">/</span>include<span style="color: #666666">/</span>kcommondecoration.h is owned by kdebase<span style="color: #666666">-</span>workspace <span style="color: #666666">4.11.14-1</span>
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I just made sure kwin is disabled if kde is disabled (even if kwin is enabled in the command line) but I don't really think it's the problem.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is that file installed in the CI?</p></pre>
</blockquote>
<p>On January 27th, 2015, 10:33 p.m. CET, <b>Marko Käning</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;">Well, the CI system lists all dependencies in the build log (above), which will be installed for this project before building: kwin does NOT appear in there!</p></pre>
</blockquote>
<p>On January 27th, 2015, 10:34 p.m. CET, <b>René J.V. Bertin</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;">Is this at all related to changes discussed in this RR?</p></pre>
</blockquote>
<p>On January 27th, 2015, 10:37 p.m. CET, <b>Marko Käning</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, sorry, no, this relates to what Yichao has commmitted as a result of your suggested changes in this RR.</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;">So it is related ;)</p></pre>
<br />
<p>- René J.V.</p>
<br />
<p>On January 27th, 2015, 7:05 p.m. CET, René J.V. Bertin 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 KDE Software on Mac OS X and Yichao Yu.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Jan. 27, 2015, 7:05 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
qtcurve
</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;">Two regressions were exposed/introduced by the recent efforts to make the code build even on systems without X11, notably OS X.</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">When KDE4 is installed (for instance through MacPorts) and KDE4 support is activated, a KDE header is included (kapplication.h) that has its own "stub" typedef for the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Display</code> type. Only it defines the type to be a <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">void*</code> rather than using the definition XOrg uses.
I have proposed to change that in kdelibs, but in the meantime it is safer to avoid typedef'ing DISPLAY on OS X, and use a different function prototype for the 2 function prototypes in <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">x11base.h</code> that require it.
This does not appear to be an issue when building for GTk2/X11 on OS X.</p>
</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">When building against a non-MacPorts (or Fink, or HomeBrew) Qt install, X headers will typically not be available. The toplevel CMake file catches that and includes a minimal stub from the QtCurve source tree, but <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">not</em> when the user has set <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QTC_ENABLE_X11=Off</code>.
This patch adds the forgotten <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">includes_directories</code> statement, and adds the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">SYSTEM</code> keyword in both cases because the xcb.h header is included as a system header using <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">#include <xcb/xcb.h></code> (i.e. with sharp brackets and not double quotes).</p>
</li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I've also corrected a small typo in the CMake file.</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;">On OS X against Qt4+KDE 4.14.4, GTk2/X11 and Qt 5.4.0 ("Qt only") all from MacPorts.
On OS X against Qt 5.4.0 installed through Digia's installer.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">FWIW, my Qt 5.4.0/MacPorts install has both the regular "Cocoa" platform plugin and the xcb platform plugin. QtCurve works with both, and with the xcb plugin on local and remote X11 displays.</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>CMakeLists.txt <span style="color: grey">(9bc4c3d)</span></li>
<li>lib/utils/x11base.h <span style="color: grey">(7c41223)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122279/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>