[KDE/Mac] Review Request 120437: [OS X] patchset to make QtCurve 1.8.18+ build and work

Sat Oct 4 18:54:13 UTC 2014

> On 十月 4, 2014, 3:05 a.m., Ben Cooksley wrote:
> > lib/utils/CMakeLists.txt, line 20
> > <https://git.reviewboard.kde.org/r/120437/diff/3/?file=315842#file315842line20>
> >
> >     This CMake code is rather odd. I'd question why QtCurve is explicitly linked to libdl anyway...
> >     
> >     libm and librt are both fairly standard as well, so i'd question why Linux would need them explicitly linked as well...

It is linked to libdl to find the right symbol for determining the program name. (see `lib/utils/utils.c`) I know it's ugly to have application specific setting and I would like to remove them (and already have done for some of them) but those hacks have been in there for a long time and is hard to remove them all.

The current way of doing it should be more portable as it doesn't rely on if there's a procfs mounted on `/proc` and is using `dlsym` rather than directly linked to any of the platform specific symbols.

As for libm and librt, they are necessary because this is in `libqtcurve-utils` which doesn't link to any of the big libraries like Qt or Gtk. There will be unresolved symbols if they are not linked.

> On 十月 4, 2014, 3:05 a.m., Ben Cooksley wrote:
> > qt4/style/qtcurve.cpp, line 603
> > <https://git.reviewboard.kde.org/r/120437/diff/3/?file=315853#file315853line603>
> >
> >     Will this cause any behavioural issues?

No, see discussion about. This only has an effect if the qtcurve kwin theme is used.

> On 十月 4, 2014, 3:05 a.m., Ben Cooksley wrote:
> > qt4/style/qtcurve.cpp, line 13380
> > <https://git.reviewboard.kde.org/r/120437/diff/3/?file=315853#file315853line13380>
> >
> >     These should be limited based on the availability of the libraries needed to support these functions I guess, or use an abstraction layer which can no-op on OSX?

Hmmm, I'm a little bit surprised that QDBus can be found on OSX (see a few lines below). The `qtcX11*` is already a no-op unless x11 is used both at compile time and at runtime. Rene was in favor of also disabling them here and I'm fine with it (for Qt4 at least).

- Yichao

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

On 九月 30, 2014, 4: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 九月 30, 2014, 4: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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20141004/c9868168/attachment.html>