<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/119302/">https://git.reviewboard.kde.org/r/119302/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 16th, 2014, 4:42 a.m. EEST, <b>Scott Kitterman</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;">This is the method used in qscintilla2's configure.py (which upstream has generally endorsed):</p>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">! /usr/bin/python</h1>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">import sys<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
import os</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">if sys.platform == 'win32':<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
data_dir = sys.prefix<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
else:<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
data_dir = sys.prefix + '/share'</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">py_sip_dir = os.path.join(data_dir, 'sip')</p>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Note: Set this by hand since the logic to figure out if we're using PyQt4 or</h1>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">PyQt5 isn't relevant to the question (QScintilla does do this, but it's not</h1>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">germane).</h1>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">pyqt = 'PyQt4'</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">if pyqt is not None:<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
pyqt_sip_dir = os.path.join(py_sip_dir, pyqt)<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
else:<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
pyqt_sip_dir = None</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">print(pyqt_sip_dir)</p>
<h1 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">prints /usr/share/sip/PyQt4</h1>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We should use something similar.</p></pre>
</blockquote>
<p>On July 16th, 2014, 4:43 a.m. EEST, <b>Scott Kitterman</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;">So markdown and python code comments don't mix. The bolded things all have a leading '#'.</p></pre>
</blockquote>
<p>On July 16th, 2014, 8:20 a.m. EEST, <b>Luca Beltrame</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;">Good idea. Can this be done?</p></pre>
</blockquote>
<p>On July 16th, 2014, 10:39 a.m. EEST, <b>Raphael Kubo da Costa</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 did look at QScintilla's build system when writing my patch, but chose not to follow this path: doing this only works for the default values (<code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">sys.prefix/sip</code> on Windows, <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">sys.prefix/share/sip</code> elsewhere), which in the worst case can be a different installation unrelated to the one used by the PyQt version we're usin
g. I didn't see much value in just working out of the box in some specific cases.</p></pre>
</blockquote>
<p>On July 16th, 2014, 3:15 p.m. EEST, <b>Scott Kitterman</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 think working out of the box in the standard, default case using the upstream recommended method is much better than requiring the value to be set by hand in all cases. This change set is about adjusting to the new upstream approach to things, so using the upstream recommended solution seems only logical.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If this doesn't get included upstream, I'll add it as a distro patch for Debian/Kubuntu as I think it's definitely a superior approach.</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;"><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;">I think working out of the box in the standard, default case using the upstream recommended method is much better than requiring the value to be set by hand in all cases. This change set is about adjusting to the new upstream approach to things, so using the upstream recommended solution seems only logical.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">shrug</em> Done in patch v2.</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;">If this doesn't get included upstream, I'll add it as a distro patch for Debian/Kubuntu</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">... ok?</p></pre>
<br />
<p>- Raphael</p>
<br />
<p>On July 16th, 2014, 4:21 p.m. EEST, Raphael Kubo da Costa 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 kdelibs, Alex Merry, Luca Beltrame, and Simon Edwards.</div>
<div>By Raphael Kubo da Costa.</div>
<p style="color: grey;"><i>Updated July 16, 2014, 4:21 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=337462">337462</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</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;">Since PyQt 4.10, PyQt.pyqtconfig is deprecated and not available unless<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
PyQt is built using the old configure script.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There is no direct replacement for it, as PyQt's new build system does<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
not provide as much information as before by design. Luckily, most of<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
the variables we are interested in can be obtained from PyQt's QtCore<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
module itself even if its old build system is used.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The only exception is pyqt_sip_dir, which cannot be determined at all if<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
pyqtconfig is not available. In this case, the most we can do is guess<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
the default path like QScintilla2 does, and fail if it does not exist.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
The user then needs to specify it manually via CMake with something like<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
-DPYQT4_SIP_DIR=/usr/share/sip/PyQt4. To this effect, all variables set<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
by FindPyQt4.cmake have been made cache variables, which means their<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
values can be overriden by the user, thus ignoring the contents read via<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
FindPyQt.py.</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;">I was able to make Kate find PyQt by passing <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">-DPYQT4_SIP_DIR=<...></code> with my PyQt installation without <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">pyqtconfig.py</code>, and calling <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">FindPyQt.py</code> by hand on a Debian sys
tem with <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">pyqtconfig.py</code> worked as before.</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/modules/FindPyQt.py <span style="color: grey">(5d2f9514d87553d5a16a95943618572316c92861)</span></li>
<li>cmake/modules/FindPyQt4.cmake <span style="color: grey">(b176b4f8cfee471a1b7aecdd2723d165b0496a85)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119302/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>