Review Request 126993: drkonqi : fix build errors and build nongui

Aleix Pol Gonzalez aleixpol at kde.org
Fri Feb 5 20:40:07 UTC 2016



> On Feb. 5, 2016, 7:58 p.m., Aleix Pol Gonzalez wrote:
> > It's only on your system because for some reason you are probably not including KDELibs4Support.
> > 
> > It should be something like this, but it isn't: https://paste.kde.org/pv12ioe89
> 
> René J.V. Bertin wrote:
>     I indeed removed the reference to KDELibs4Support from `find_package(KF5` in the toplevel CMake file, but only because I saw the framework isn't used by DrKonqi. I was under the impression it shouldn't thus change anything for DrK, does it? 
>     Of what is that paste snippet a diff?

It shouldn't, but it does.
The patch there is how it should have been, now it's quite impossible to apply it because half KDE would stop compiling.

We need to start dropping KDELibs4Support for real though... :/


- Aleix


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


On Feb. 5, 2016, 9:35 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126993/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2016, 9:35 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> While looking to build DrKonqi for OS X/MacPorts I came across a few issues that appear to be syntax/coding errors that, surprisingly, were never caught before given that they also caused compile failures when I used the same configure/build/packaging script on Linux.
> 
> The code uses methods that no longer exist in Qt 5.5+ : `QString::fromAscii()`, `QHeaderView::setResizeMod()` (replaced with `QHeaderView::setSectionResizeMode()` and `QInputDialog::getInteger()` (instead of `QInputDialog::getInt()`). Those are the easy and straightforward ones fixed by this patch.
> 
> The handling of file dialogs (in 2 locations) was more problematic. Both Apple's clang "6.0.0" and gcc 5.3 (on Linux) refused statements like `QWeakPointer<QFileDialog> dlg = new QFileDialog(parent,defname);` . My initial attempt was to follow the documentation for QWeakPointer, and do `QWeakPointer<QFileDialog> dlg = QSharedPointer<QFileDialog>(new QFileDialog(parent,defname));` but that caused a crash as soon as I clicked on the "save backtrace as" button - on both platforms.
> It turned out that `QSharedPointer<QFileDialog>()` returned a valid (non-null) shared pointer, but after creating a QWeakPointer from it, `dlg.data()` returned NULL -- immediately.
> 
> I had a look around in the 5.17.0 frameworks codebase, and could find no relevant examples of using QWeakPointer; some expressions appear to be equivalent to the original DrKonqi code, in other locations QWeakPointer instances are indeed created "going through" a QSharedPointer. In the end I tested using QFileDialog (and `QFileDialog::selectedUrls()` as kate5 does, and that works (on OS X and Linux).
> 
> That's what the attached patch does: no QFileDialog pointer at all, just an instance.
> This RR is intended to fuel a discussion on the subject, as suggested by Aleix; therefore I left the comment about the possibility of invalidating/deleting the file dialog e.g. through a DBus call. Is that actually a possibility, and why wouldn't it be in Kate? Are there no other ways to prevent it, or check for its occurrence?
> 
> This patch also marks drkonqi as a nongui executable, a requirement on OS X and which shouldn't cause any regressions elsewhere AFAIK.
> 
> 
> Diffs
> -----
> 
>   drkonqi/CMakeLists.txt deb8c40 
>   drkonqi/bugzillaintegration/bugzillalib.cpp 802c5fb 
>   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp e60fb14 
>   drkonqi/bugzillaintegration/reportassistantpages_bugzilla_duplicates.cpp 4f8f4ea 
>   drkonqi/drkonqi.cpp b12c118 
> 
> Diff: https://git.reviewboard.kde.org/r/126993/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.9.1 and Kubuntu 14.04, both with Qt 5.5.1 and KF5 5.17.0 installed into /opt/local
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160205/03bf25aa/attachment.html>


More information about the Plasma-devel mailing list