[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-mac
Subject:    Re: [KDE/Mac] Review Request 120437: [OS X] patchset to make QtCurve 1.8.18+ build and work
From:       "Ben Cooksley" <bcooksley () kde ! org>
Date:       2014-10-04 18:49:23
Message-ID: 20141004184923.15676.75031 () probe ! kde ! org
[Download RAW message or body]

--===============7365234675106269980==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit



> On Oct. 4, 2014, 7:05 a.m., Ben Cooksley wrote:
> > qt4/config/exportthemedialog.h, line 28
> > <https://git.reviewboard.kde.org/r/120437/diff/3/?file=315848#file315848line28>
> > 
> > Likewise with the header clash issue
> 
> René J.V. Bertin wrote:
> Not a clash, but a failure to find the header in one of the OS X configurations I \
> tested. It's not completely impossible that this is due to the fact that we have \
> cmake 3.0.2 in MacPorts, while most Linux distros are probably still with CMake \
> 2.8.x . 
> Ben Cooksley wrote:
> The CI system runs CMake 3.x from what I can remember.
> 
> Marko Käning wrote:
> I've 3.0.2 running on OSX/CI.
> (Don't know what's present on Jenkins master ATM.)

Sorry - should have been more clear that I was referring to the Linux system there.


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120437/#review67882
-----------------------------------------------------------


On Sept. 30, 2014, 8:01 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120437/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2014, 8:01 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, André Marcelo Alvarenga, Yuri \
> Chornoivan, and Yichao Yu. 
> 
> Repository: qtcurve
> 
> 
> Description
> -------
> 
> KDE is built on a cross-platform framework, and as such most applications (not \
> directly linked to the Plasma desktop or X11) function or can be made to function \
> fine. Qt will use a native style by default, but supports the same style plugins as \
> it does on other platforms, which bring the advantage of better (more precise) and \
> more compact layout, without by definition reducing integration with the OS X \
> desktop. 
> As such, QtCurve 1.8.14 worked "out of the box" \
> (http://kde-look.org/content/show.php?content=40492, source from \
> http://craigd.wikispaces.com/file/view) and I created a MacPorts port for it \
> (https://trac.macports.org/ticket/44527). With the `kde4-workspace` port installed, \
> one has almost the full customisation experience, minus everything window-manager \
> related of course, nor full control of window backgrounds (not allowed by Qt and/or \
> OS X). 
> The current version, which has seen many changes that require X11, was less \
> straightforward to get to work, requiring a considerable collection of small \
> changes. 
> The brunt of the present patchset consists of making the code in question \
> conditional on the presence of `Q_WS_X11`, `Q_OS_MAC`, `__MACH__` or `__APPLE__` in \
> non Qt code and `APPLE` in the cmake files. A few patches introduce functions not \
> available on OS X (getline) or replace them with OS X specifics (`clock_gettime` -> \
> `mach_absolute_timer`, nicely initialised with a dylib constructor function :) ) \
> while others simply ensure that header files are found (or inexistent ones \
> ignored). The only functional guess/change I made is in `setOpacityProp()`, where I \
> added a `w->setWindowOpacity( 0.01 * opacity )` for non Q_WS_X11 code, presuming \
> that `prop` is simply on the 0-100 percentage as exposed in the preferences dialog. \
>  
> Diffs
> -----
> 
> CMakeLists.txt f76fd1b 
> gtk2/common/config_file.c d732ca9 
> gtk2/style/CMakeLists.txt 01e8891 
> gtk2/style/qt_settings.c f5a5c98 
> lib/cairo/CMakeLists.txt c66c63c 
> lib/utils/CMakeLists.txt 15757ed 
> lib/utils/color.h 2c7081f 
> lib/utils/map.c a829e9e 
> lib/utils/process.c f2490ef 
> lib/utils/timer.c 879451e 
> qt4/config/CMakeLists.txt 15454e6 
> qt4/config/exportthemedialog.h 42590ec 
> qt4/config/exportthemedialog.cpp f39b86d 
> qt4/kwin/CMakeLists.txt 654604b 
> qt4/kwinconfig/CMakeLists.txt cbd8b62 
> qt4/style/CMakeLists.txt f38d029 
> qt4/style/qtcurve.cpp 7346c2f 
> qt4/style/qtcurve_plugin.cpp f390da4 
> qt5/CMakeLists.txt 1d0359e 
> qt5/style/CMakeLists.txt b6cb222 
> 
> Diff: https://git.reviewboard.kde.org/r/120437/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.6.8 against kdelibs 4.14.1 and Qt 4.8.6 . This OS requires to build \
> v1.8.18 with a gcc version from MacPorts in order to get the necessary C++11 \
> support; newer OS versions will use a recent clang version (system compiler). 
> - Qt4/KDE4 support: OK (see screenshot)
> - GTk2 support: OK
> - X11 support: builds but I have no idea what it's supposed to do
> - Qt5 support: OK (against Qt 5.3.1 obtained with Digia's installer)
> 
> I tested building on Linux (Kubuntu 14.04, KDE SC 4.13.3, using clang 3.4) after \
> the 2nd update to the patchset and that worked fine. One issue that could use \
> attention is the Qt5 detection: Qt5.3 is apparently required and `ENABLE_QT5` is \
> not unset when an earlier Qt5 version is found instead. 
> 
> File Attachments
> ----------------
> 
> sample showing a native file dialog for comparison
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/30/02545bef-04d2-4a45-8955-e13bf7d063a0__Screen_shot_2014-09-30_at_14.05.54.png
>  KDE4 systemsettings and the QtCurve configuration dialog
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/30/fe5564f2-5e4e-485a-96d7-d192ff104261__QtCurve-1818.png
>  KDE4 systemsettings and QtCurve config dialog with QtCuve 1.8.14, for comparison
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/30/6bd8dd5a-9b94-43c3-88d9-c9be32a56b72__QtCurve-1814.png
>  a little convenience file for uploading to RB from KDevelop
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/30/39207a85-4092-4ea1-a181-051d0db5bb96__.reviewboardrc
>  
> 
> Thanks,
> 
> René J.V. Bertin
> 
> 


--===============7365234675106269980==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit




<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/120437/">https://git.reviewboard.kde.org/r/120437/</a>
  </td>
    </tr>
   </table>
   <br />










<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <p style="margin-top: 0;">On October 4th, 2014, 7:05 a.m. UTC, <b>Ben \
Cooksley</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="https://git.reviewboard.kde.org/r/120437/diff/3/?file=315848#file315848line28" \
style="color: black; font-weight: bold; text-decoration: \
underline;">qt4/config/exportthemedialog.h</a>  <span style="font-weight: normal;">

     (Diff revision 3)

    </span>
   </th>
  </tr>
 </thead>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">28</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="cp">#include &quot;common/common.h&quot;</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;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Likewise with the header clash issue</p></pre>  </blockquote>



 <p>On October 4th, 2014, 9:08 a.m. UTC, <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;">Not a \
clash, but a failure to find the header in one of the OS X configurations I \
tested.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> It's not completely impossible that this is due to \
the fact that we have cmake 3.0.2 in MacPorts, while most Linux distros are probably \
still with CMake 2.8.x .</p></pre>  </blockquote>





 <p>On October 4th, 2014, 11:40 a.m. UTC, <b>Ben Cooksley</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;">The \
CI system runs CMake 3.x from what I can remember.</p></pre>  </blockquote>





 <p>On October 4th, 2014, 12:09 p.m. UTC, <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;">I've \
3.0.2 running on OSX/CI.<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> (Don't know what's present on Jenkins \
master ATM.)</p></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;"><p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Sorry - should have been more clear that I was referring to the Linux \
system there.</p></pre> <br />




<p>- Ben</p>


<br />
<p>On September 30th, 2014, 8:01 p.m. UTC, 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, André Marcelo Alvarenga, Yuri \
Chornoivan, and Yichao Yu.</div> <div>By René J.V. Bertin.</div>


<p style="color: grey;"><i>Updated Sept. 30, 2014, 8:01 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;">KDE is built on a cross-platform framework, and as \
such most applications (not directly linked to the Plasma desktop or X11) function or \
can be made to function fine.<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> Qt will use a native style by \
default, but supports the same style plugins as it does on other platforms, which \
bring the advantage of better (more precise) and more compact layout, without by \
definition reducing integration with the OS X desktop.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As \
such, QtCurve 1.8.14 worked "out of the box" \
(http://kde-look.org/content/show.php?content=40492, source from \
http://craigd.wikispaces.com/file/view) and I created a MacPorts port for it \
(https://trac.macports.org/ticket/44527). With the <code style="text-rendering: \
inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">kde4-workspace</code> port installed, one has almost the full customisation \
experience, minus everything window-manager related of course, nor full control of \
window backgrounds (not allowed by Qt and/or OS X).</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The \
current version, which has seen many changes that require X11, was less \
straightforward to get to work, requiring a considerable collection of small \
changes.</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The brunt of the present patchset consists of making \
the code in question conditional on the presence of <code style="text-rendering: \
inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">Q_WS_X11</code>, <code style="text-rendering: inherit;color: \
#4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">Q_OS_MAC</code>, <code style="text-rendering: inherit;color: \
#4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">__MACH__</code> or <code style="text-rendering: inherit;color: \
#4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">__APPLE__</code> in non Qt code and <code style="text-rendering: \
inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">APPLE</code> in the cmake files. A few patches introduce functions not \
available on OS X (getline) or replace them wi  th OS X specifics (<code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">clock_gettime</code> -&gt; <code style="text-rendering: \
inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">mach_absolute_timer</code>, nicely initialised with a dylib constructor \
function :) ) while others simply ensure that header files are found (or inexistent \
ones ignored).<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> The only functional guess/change I made is in <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">setOpacityProp()</code>, where I added a <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">w-&gt;setWindowOpacity( 0.01 * opacity )</code> for non \
Q_WS_X11 code, presuming that <code style="text-rendering: inherit;color: \
#4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">prop</code> \
is simply on the 0-100 percentage as exposed in the preferences dialog.</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 10.6.8 against kdelibs 4.14.1 and Qt 4.8.6 . \
This OS requires to build v1.8.18 with a gcc version from MacPorts in order to get \
the necessary C++11 support; newer OS versions will use a recent clang version \
(system compiler).</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;">Qt4/KDE4 support: OK \
(see screenshot)</li> <li style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;">GTk2 support: OK</li> <li \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;">X11 support: builds but I have no idea what it's supposed to do</li> <li \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;">Qt5 support: OK (against Qt 5.3.1 obtained with Digia's installer)</li> \
</ul> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I tested building on Linux (Kubuntu 14.04, KDE SC \
4.13.3, using clang 3.4) after the 2nd update to the patchset and that worked \
fine.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> One issue that could use attention is the Qt5 \
detection: Qt5.3 is apparently required and <code style="text-rendering: \
inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">ENABLE_QT5</code> is not unset when an earlier Qt5 version is found \
instead.</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">(f76fd1b)</span></li>

 <li>gtk2/common/config_file.c <span style="color: grey">(d732ca9)</span></li>

 <li>gtk2/style/CMakeLists.txt <span style="color: grey">(01e8891)</span></li>

 <li>gtk2/style/qt_settings.c <span style="color: grey">(f5a5c98)</span></li>

 <li>lib/cairo/CMakeLists.txt <span style="color: grey">(c66c63c)</span></li>

 <li>lib/utils/CMakeLists.txt <span style="color: grey">(15757ed)</span></li>

 <li>lib/utils/color.h <span style="color: grey">(2c7081f)</span></li>

 <li>lib/utils/map.c <span style="color: grey">(a829e9e)</span></li>

 <li>lib/utils/process.c <span style="color: grey">(f2490ef)</span></li>

 <li>lib/utils/timer.c <span style="color: grey">(879451e)</span></li>

 <li>qt4/config/CMakeLists.txt <span style="color: grey">(15454e6)</span></li>

 <li>qt4/config/exportthemedialog.h <span style="color: grey">(42590ec)</span></li>

 <li>qt4/config/exportthemedialog.cpp <span style="color: grey">(f39b86d)</span></li>

 <li>qt4/kwin/CMakeLists.txt <span style="color: grey">(654604b)</span></li>

 <li>qt4/kwinconfig/CMakeLists.txt <span style="color: grey">(cbd8b62)</span></li>

 <li>qt4/style/CMakeLists.txt <span style="color: grey">(f38d029)</span></li>

 <li>qt4/style/qtcurve.cpp <span style="color: grey">(7346c2f)</span></li>

 <li>qt4/style/qtcurve_plugin.cpp <span style="color: grey">(f390da4)</span></li>

 <li>qt5/CMakeLists.txt <span style="color: grey">(1d0359e)</span></li>

 <li>qt5/style/CMakeLists.txt <span style="color: grey">(b6cb222)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/120437/diff/" style="margin-left: \
3em;">View Diff</a></p>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments \
</h1>


 <li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/09/30/02545bef-04d2-4a45-8955-e13bf7d063a0__Screen_shot_2014-09-30_at_14.05.54.png">sample \
showing a native file dialog for comparison</a></li>

 <li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/09/30/fe5564f2-5e4e-485a-96d7-d192ff104261__QtCurve-1818.png">KDE4 \
systemsettings and the QtCurve configuration dialog</a></li>

 <li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/09/30/6bd8dd5a-9b94-43c3-88d9-c9be32a56b72__QtCurve-1814.png">KDE4 \
systemsettings and QtCurve config dialog with QtCuve 1.8.14, for comparison</a></li>

 <li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/09/30/39207a85-4092-4ea1-a181-051d0db5bb96__.reviewboardrc">a \
little convenience file for uploading to RB from KDevelop</a></li>

</ul>




  </td>
 </tr>
</table>








  </div>
 </body>
</html>


--===============7365234675106269980==--



_______________________________________________
kde-mac@kde.org
List Information: https://mail.kde.org/mailman/listinfo/kde-mac
KDE/Mac Information: http://community.kde.org/Mac

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic