[KDE/Mac] Review Request 122279: [OS X] make it build on OS X. Again ...

Tue Jan 27 22:26:11 UTC 2015

> On Jan. 27, 2015, 7:18 p.m., Yichao Yu wrote:
> > I've just pushed a slightly modified version. Can you check if it solves you problem.
> > 
> > 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 `fixx11h.h` 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.
> 
> Marko Käning wrote:
>     Yichao, the current state of qtcurve again builds fine on my OSX/CI system.
>     
>     Thanks to you and René for fixing this!!
> 
> René J.V. Bertin wrote:
>     Works for me too, in all test cases cited above.
>     
>     That means that this RR can be discarded, right?
> 
> Marko Käning wrote:
>     Yes, I suppose. :)
> 
> Marko Käning wrote:
>     Wait, it still fails on Linux/CI: http://build.kde.org/view/FAILED/job/qtcurve_master/28/console
> 
> René J.V. Bertin wrote:
>     Is that related to https://git.reviewboard.kde.org/r/121390/diff/# ?
> 
> Yichao Yu wrote:
>     @Rene No it shouldn't.
>     
>     From the output it looks like come kde(4) workspace headers are missing, e.g.
>     
>     ```
>     yuyichao% LANG=C yaourt -Qo /usr/include/kcommondecoration.h
>     /usr/include/kcommondecoration.h is owned by kdebase-workspace 4.11.14-1
>     ```
>     
>     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.
>     
>     Is that file installed in the CI?
> 
> Marko Käning wrote:
>     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!
> 
> René J.V. Bertin wrote:
>     Is this at all related to changes discussed in this RR?
> 
> Marko Käning wrote:
>     Oh, sorry, no, this relates to what Yichao has commmitted as a result of your suggested changes in this RR.
> 
> René J.V. Bertin wrote:
>     So it is related ;)
> 
> Marko Käning wrote:
>     Indirectly, yes. :)
> 
> Yichao Yu wrote:
>     Not really. The change in this RR should have nothing to do with the failure.
>     
>     The dependency list has kde-workspace 4.11 in it. Where can I find the list of file it installed?

Hi Yichao, this is what Ben said on IRC just now:
```
[23:23] <bcooksley> I haven't looked yet
[23:23] <bcooksley> but i'd take a pot shot guess
[23:23] <bcooksley> and say qtcurve isn't trying to find KDE4Workspace
[23:23] <bcooksley> thus doesn't have the right include paths
[23:23] <bcooksley> or if it does
[23:24] <mk_MP_> It does.
[23:24] <bcooksley> it isn't activating those include paths
[23:24] <bcooksley> using a include_path() statement
[23:24] <mk_MP_> 18:48:16 ==== kde-workspace - Branch KDE/4.11
[23:24] <bcooksley> er. include_directories()
[23:24] <bcooksley> that is the build metadata
[23:24] <bcooksley> CMake has to look as well
[23:24] <mk_MP_> OK, will let him know in the RR.
[23:24] <bcooksley> build metadata just says what is available
[23:24] <bcooksley> CMake has to make use of it
```

- Marko

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

On Jan. 27, 2015, 7:05 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122279/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2015, 7:05 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and Yichao Yu.
> 
> 
> Repository: qtcurve
> 
> 
> Description
> -------
> 
> Two regressions were exposed/introduced by the recent efforts to make the code build even on systems without X11, notably OS X.
> 
> - 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 `Display` type. Only it defines the type to be a `void*` 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 `x11base.h` that require it.
> This does not appear to be an issue when building for GTk2/X11 on OS X.
> 
> - 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 *not* when the user has set `QTC_ENABLE_X11=Off`.
> This patch adds the forgotten `includes_directories` statement, and adds the `SYSTEM` keyword in both cases because the xcb.h header is included as a system header using `#include <xcb/xcb.h>` (i.e. with sharp brackets and not double quotes).
> 
> I've also corrected a small typo in the CMake file.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 9bc4c3d 
>   lib/utils/x11base.h 7c41223 
> 
> Diff: https://git.reviewboard.kde.org/r/122279/diff/
> 
> 
> Testing
> -------
> 
> 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.
> 
> 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.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

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