<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/107148/">http://git.reviewboard.kde.org/r/107148/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 3rd, 2012, 4:59 p.m., <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;">FindPythonLibrary.cmake: are all these changes source compatible ? Where is PYTHON_INCLUDE_PATH set now ? Does this come from FindPythonLibs.cmake ?
PythonMacros.cmake: this is mostly coding style changes, right ? Why did you remove that install() command at the end ?</pre>
</blockquote>
<p>On November 3rd, 2012, 5:24 p.m., <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;">> FindPythonLibrary.cmake: are all these changes source compatible ? Where is
> PYTHON_INCLUDE_PATH set now ? Does this come from FindPythonLibs.cmake ?
PYTHON_INCLUDE_PATH is indeed set by FindPythonLibs. Should I write that in the header?
> PythonMacros.cmake: this is mostly coding style changes, right ? Why did you
> remove that install() command at the end ?
It was removed because the destination directory is set in the if() block above to the _py_install variable, which is then used in the final install() call.</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;">So no objections from my side.
Go ahead.
</pre>
<br />
<p>- Alexander</p>
<br />
<p>On November 2nd, 2012, 9:16 a.m., Luca Beltrame wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Build System.</div>
<div>By Luca Beltrame.</div>
<p style="color: grey;"><i>Updated Nov. 2, 2012, 9:16 a.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;">These adjustments ensure that the macros work with newer Python versions (which changed their behavior with byte-compiled files) and allow proper installation in systems where the name of the Python library is changed (see openSUSE). Additionally, all the finding logic is now delegated to the proper Find* packages in CMake. The variables are set to ensure source compatibility: those not set are directly taken from FindPythonInterp and FindPythonLibs.
Additionally, PythonMacros is adjusted to ensure that Python files are properly byte-compiled and messages routed to the right place.
This change will be committed only to the 4.10 branch, as it relies on some CMake functionality (VERSION_LESS, VERSION_GREATER, Python_ADDITIONAL_VERSIONS) only available in 2.8+.
Note: I'm aware of the whitespace issues, will clean up before committing.</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;">Ran CMake in the PyKDE4 checkout, testing different interpreters and site-packages directories, no issues.
Tested with a custom PyKDE4 project, no issues with file compilation.</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/FindPythonLibrary.cmake <span style="color: grey">(60567e297f686ecc1a1c1f4bcebfd94181bfe116)</span></li>
<li>cmake/modules/PythonMacros.cmake <span style="color: grey">(661e32d0c8fbc5a083fd7e8271557ae51c5e12d1)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107148/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>